diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 69a154b46f..e72a7f6d11 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -28,7 +28,8 @@ from courseware.access_utils import ( adjust_start_date, check_start_date, debug, - in_preview_mode + in_preview_mode, + check_course_open_for_learner, ) from courseware.masquerade import get_masquerade_role, is_masquerading_as_student from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException @@ -173,31 +174,6 @@ def has_staff_access_to_preview_mode(user, course_key): return has_admin_access_to_course or is_masquerading_as_student(user, course_key) -def _can_access_descriptor_with_start_date(user, descriptor, course_key): # pylint: disable=invalid-name - """ - Checks if a user has access to a descriptor based on its start date. - - If there is no start date specified, grant access. - Else, check if we're past the start date. - - Note: - We do NOT check whether the user is staff or if the descriptor - is detached... it is assumed both of these are checked by the caller. - - Arguments: - user (User): the user whose descriptor access we are checking. - descriptor (AType): the descriptor for which we are checking access, - where AType is CourseDescriptor, CourseOverview, or any other class - that represents a descriptor and has the attributes .location, .id, - .start, and .days_early_for_beta. - - Returns: - AccessResponse: The result of this access check. Possible results are - ACCESS_GRANTED or a StartDateError. - """ - return check_start_date(user, descriptor.days_early_for_beta, descriptor.start, course_key) - - def _can_view_courseware_with_prerequisites(user, course): # pylint: disable=invalid-name """ Checks if a user has access to a course based on its prerequisites. @@ -333,7 +309,7 @@ def _has_access_course(user, action, courselike): """ response = ( _visible_to_nonstaff_users(courselike) and - _can_access_descriptor_with_start_date(user, courselike, courselike.id) + check_course_open_for_learner(user, courselike) ) return ( @@ -519,7 +495,7 @@ def _has_access_descriptor(user, action, descriptor, course_key=None): _can_access_descriptor_with_milestones(user, descriptor, course_key) and ( _has_detached_class_tag(descriptor) or - _can_access_descriptor_with_start_date(user, descriptor, course_key) + check_start_date(user, descriptor.days_early_for_beta, descriptor.start, course_key) ) ) diff --git a/lms/djangoapps/courseware/access_utils.py b/lms/djangoapps/courseware/access_utils.py index 77cc9cbc1d..7dc3a01b4a 100644 --- a/lms/djangoapps/courseware/access_utils.py +++ b/lms/djangoapps/courseware/access_utils.py @@ -11,6 +11,7 @@ from django.utils.timezone import UTC from courseware.access_response import AccessResponse, StartDateError from courseware.masquerade import is_masquerading_as_student +from openedx.features.course_experience import COURSE_PRE_START_ACCESS_FLAG from student.roles import CourseBetaTesterRole from xmodule.util.django import get_current_request_hostname @@ -83,10 +84,13 @@ def in_preview_mode(): return bool(preview_lms_base and hostname and hostname.split(':')[0] == preview_lms_base.split(':')[0]) -def is_course_open_for_learner(user, course): +def check_course_open_for_learner(user, course): """ Check if the course is open for learners based on the start date. + + Returns: + AccessResponse: Either ACCESS_GRANTED or StartDateError. """ - now = datetime.now(UTC()) - effective_start = adjust_start_date(user, course.days_early_for_beta, course.start, course.id) - return not(not in_preview_mode() and now < effective_start) + if COURSE_PRE_START_ACCESS_FLAG.is_enabled(course.id): + return ACCESS_GRANTED + return check_start_date(user, course.days_early_for_beta, course.start, course.id) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index a98d229fbe..02e1671124 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -19,7 +19,6 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey import courseware.access as access import courseware.access_response as access_response -from ccx.tests.factories import CcxFactory from courseware.masquerade import CourseMasquerade from courseware.tests.factories import ( BetaTesterFactory, @@ -31,6 +30,7 @@ from courseware.tests.factories import ( from courseware.tests.helpers import LoginEnrollmentTestCase, masquerade_as_group_member from lms.djangoapps.ccx.models import CustomCourseForEdX from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from student.models import CourseEnrollment from student.roles import CourseCcxCoachRole, CourseStaffRole from student.tests.factories import ( @@ -45,7 +45,6 @@ from xmodule.course_module import ( CATALOG_VISIBILITY_CATALOG_AND_ABOUT, CATALOG_VISIBILITY_NONE ) -from xmodule.error_module import ErrorDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( @@ -54,10 +53,9 @@ from xmodule.modulestore.tests.django_utils import ( SharedModuleStoreTestCase ) from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.modulestore.xml import CourseLocationManager from xmodule.partitions.partitions import MINIMUM_STATIC_PARTITION_ID, Group, UserPartition -from xmodule.tests import get_test_system +QUERY_COUNT_TABLE_BLACKLIST = WAFFLE_TABLES # pylint: disable=protected-access @@ -847,5 +845,5 @@ class CourseOverviewAccessTestCase(ModuleStoreTestCase): num_queries = 0 course_overview = CourseOverview.get_from_id(course.id) - with self.assertNumQueries(num_queries): + with self.assertNumQueries(num_queries, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): bool(access.has_access(user, action, course_overview, course_key=course.id)) diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py index 99cc43187d..11e0289acb 100644 --- a/lms/djangoapps/courseware/tests/test_course_info.py +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -11,6 +11,7 @@ from django.test.utils import override_settings from lms.djangoapps.ccx.tests.factories import CcxFactory from nose.plugins.attrib import attr from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from pyquery import PyQuery as pq from student.models import CourseEnrollment from student.tests.factories import AdminFactory @@ -27,6 +28,8 @@ from xmodule.modulestore.xml_importer import import_course_from_xml from .helpers import LoginEnrollmentTestCase +QUERY_COUNT_TABLE_BLACKLIST = WAFFLE_TABLES + @attr(shard=1) class CourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTestCase): @@ -378,14 +381,14 @@ class SelfPacedCourseInfoTestCase(LoginEnrollmentTestCase, SharedModuleStoreTest and Mongo queries. """ url = reverse('info', args=[unicode(course.id)]) - with self.assertNumQueries(sql_queries): + with self.assertNumQueries(sql_queries, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(mongo_queries): with mock.patch("openedx.core.djangoapps.theming.helpers.get_current_site", return_value=None): resp = self.client.get(url) self.assertEqual(resp.status_code, 200) def test_num_queries_instructor_paced(self): - self.fetch_course_info_with_queries(self.instructor_paced_course, 26, 3) + self.fetch_course_info_with_queries(self.instructor_paced_course, 22, 3) def test_num_queries_self_paced(self): - self.fetch_course_info_with_queries(self.self_paced_course, 26, 3) + self.fetch_course_info_with_queries(self.self_paced_course, 22, 3) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index c8c60e6fc2..4508797807 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -20,7 +20,7 @@ from certificates.tests.factories import CertificateInvalidationFactory, Generat from commerce.models import CommerceConfiguration from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory -from courseware.access_utils import is_course_open_for_learner +from courseware.access_utils import check_course_open_for_learner from courseware.model_data import FieldDataCache, set_score from courseware.module_render import get_module from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory @@ -2567,9 +2567,10 @@ class AccessUtilsTestCase(ModuleStoreTestCase): (-1, True) ) @ddt.unpack + @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) def test_is_course_open_for_learner(self, start_date_modifier, expected_value): staff_user = AdminFactory() start_date = datetime.now(UTC) + timedelta(days=start_date_modifier) course = CourseFactory.create(start=start_date) - self.assertEqual(is_course_open_for_learner(staff_user, course), expected_value) + self.assertEqual(bool(check_course_open_for_learner(staff_user, course)), expected_value) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 0639c3452f..3dafd532a4 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -39,7 +39,7 @@ from xmodule.modulestore.django import modulestore from xmodule.x_module import STUDENT_VIEW from ..access import has_access -from ..access_utils import in_preview_mode, is_course_open_for_learner +from ..access_utils import in_preview_mode, check_course_open_for_learner from ..courses import get_course_with_access, get_current_child, get_studio_url from ..entrance_exams import ( course_has_entrance_exam, @@ -372,7 +372,7 @@ class CoursewareIndex(View): self._add_entrance_exam_to_context(courseware_context) # staff masquerading data - if not is_course_open_for_learner(self.effective_user, self.course): + if not check_course_open_for_learner(self.effective_user, self.course): # Disable student view button if user is staff and # course is not yet visible to students. courseware_context['disable_student_access'] = True diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index ec3d5e4e1f..2a682bb288 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -18,7 +18,7 @@ from commerce.utils import EcommerceService from course_modes.models import CourseMode from courseware.access import has_access, has_ccx_coach_role from courseware.access_response import StartDateError -from courseware.access_utils import in_preview_mode, is_course_open_for_learner +from courseware.access_utils import in_preview_mode, check_course_open_for_learner from courseware.courses import ( can_self_enroll_in_course, get_course, @@ -350,8 +350,7 @@ def course_info(request, course_id): if SelfPacedConfiguration.current().enable_course_home_improvements: context['resume_course_url'] = get_last_accessed_courseware(course, request, user) - # LEARNER-981/LEARNER-837: Hide masquerade as necessary in Course Home (DONE) - if not is_course_open_for_learner(user, course): + if not check_course_open_for_learner(user, course): # Disable student view button if user is staff and # course is not yet visible to students. context['disable_student_access'] = True @@ -486,7 +485,7 @@ class CourseTabView(EdxFragmentView): else: masquerade = None - if course and not is_course_open_for_learner(request.user, course): + if course and not check_course_open_for_learner(request.user, course): # Disable student view button if user is staff and # course is not yet visible to students. supports_preview_menu = False diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 78c997a1dd..f3be484977 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -4,19 +4,15 @@ from datetime import datetime import ddt from django.core.urlresolvers import reverse -from django.http import HttpResponse, Http404 +from django.http import Http404 from django.test.client import Client, RequestFactory from django.test.utils import override_settings from django.utils import translation from mock import ANY, Mock, call, patch from nose.tools import assert_true -from rest_framework.test import APIRequestFactory -from common.test.utils import MockSignalHandlerMixin, disable_signal from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory -from discussion_api import api -from discussion_api.tests.utils import CommentsServiceMockMixin, make_minimal_cs_thread from django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY from django_comment_client.permissions import get_team from django_comment_client.tests.group_id import ( @@ -24,7 +20,6 @@ from django_comment_client.tests.group_id import ( GroupIdAssertionMixin, NonCohortedTopicGroupIdTestMixin ) -from django_comment_client.base.views import create_thread from django_comment_client.tests.unicode import UnicodeTestMixin from django_comment_client.tests.utils import ( CohortedTestCase, @@ -37,7 +32,6 @@ from django_comment_common.models import ( CourseDiscussionSettings, ForumsConfig, FORUM_ROLE_STUDENT, - Role ) from django_comment_common.utils import ThreadContext, seed_permissions_roles from lms.djangoapps.courseware.exceptions import CourseAccessRedirect @@ -50,9 +44,10 @@ from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts from openedx.core.djangoapps.course_groups.tests.test_views import CohortViewsTestCase from openedx.core.djangoapps.util.testing import ContentGroupTestCase +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired from student.roles import CourseStaffRole, UserBasedRole -from student.tests.factories import CourseAccessRoleFactory, CourseEnrollmentFactory, UserFactory +from student.tests.factories import CourseEnrollmentFactory, UserFactory from util.testing import EventTestMixin, UrlResetMixin from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -65,6 +60,8 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, chec log = logging.getLogger(__name__) +QUERY_COUNT_TABLE_BLACKLIST = WAFFLE_TABLES + # pylint: disable=missing-docstring @@ -467,7 +464,7 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): [num_cached_mongo_calls, num_cached_sql_queries], ] for expected_mongo_calls, expected_sql_queries in cached_calls: - with self.assertNumQueries(expected_sql_queries): + with self.assertNumQueries(expected_sql_queries, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(expected_mongo_calls): call_single_thread() diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 75c0323e0b..3c13e5c650 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -39,6 +39,7 @@ from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMe from lms.lib.comment_client import Thread from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory +from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES from student.roles import CourseStaffRole, UserBasedRole from student.tests.factories import CourseAccessRoleFactory, CourseEnrollmentFactory, UserFactory from util.testing import UrlResetMixin @@ -60,6 +61,8 @@ log = logging.getLogger(__name__) CS_PREFIX = "http://localhost:4567/api/v1" +QUERY_COUNT_TABLE_BLACKLIST = WAFFLE_TABLES + # pylint: disable=missing-docstring @@ -395,7 +398,7 @@ class ViewsQueryCountTestCase( with modulestore().default_store(default_store): self.set_up_course(module_count=module_count) self.clear_caches() - with self.assertNumQueries(sql_queries): + with self.assertNumQueries(sql_queries, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(mongo_calls): func(self, *args, **kwargs) return inner diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index bcae90833c..96060e22a5 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -247,6 +247,13 @@ class WaffleFlag(object): self.flag_name = flag_name self.flag_undefined_default = flag_undefined_default + @property + def namespaced_flag_name(self): + """ + Returns the fully namespaced flag name. + """ + return self.waffle_namespace._namespaced_name(self.flag_name) + def is_enabled(self): """ Returns whether or not the flag is enabled. diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index ebe026711a..578c005e00 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -18,6 +18,9 @@ UNIFIED_COURSE_TAB_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'unified_cours # Waffle flag to enable the sock on the footer of the home and courseware pages DISPLAY_COURSE_SOCK_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'display_course_sock') +# Waffle flag to let learners access a course before its start date +COURSE_PRE_START_ACCESS_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'pre_start_access') + # Waffle flag to enable a review page link from the unified home page SHOW_REVIEWS_TOOL_FLAG = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'show_reviews_tool') diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 10607783fd..23e424e3ac 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -2,8 +2,12 @@ """ Tests for the course home page. """ +import datetime import ddt import mock +import pytz +from waffle.testutils import override_flag + from courseware.tests.factories import StaffFactory from django.conf import settings from django.core.urlresolvers import reverse @@ -18,6 +22,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import CourseUserType, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls +from ... import COURSE_PRE_START_ACCESS_FLAG from .helpers import add_course_mode from .test_course_updates import create_course_update @@ -142,6 +147,26 @@ class TestCourseHomePage(CourseHomePageTestCase): url = course_home_url(self.course) self.client.get(url) + @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False}) + def test_start_date_handling(self): + """ + Verify that the course home page handles start dates correctly. + """ + now = datetime.datetime.now(pytz.UTC) + tomorrow = now + datetime.timedelta(days=1) + self.course.start = tomorrow + + # The course home page should 404 for a course starting in the future + url = course_home_url(self.course) + response = self.client.get(url) + self.assertRedirects(response, '/dashboard?notlive=Jan+01%2C+2030') + + # With the Waffle flag enabled, the course should be visible + with override_flag(COURSE_PRE_START_ACCESS_FLAG.namespaced_flag_name, True): + url = course_home_url(self.course) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + @ddt.ddt class TestCourseHomePageAccess(CourseHomePageTestCase): diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index 14f07d479e..ab9cad1303 100644 --- a/openedx/features/course_experience/utils.py +++ b/openedx/features/course_experience/utils.py @@ -71,9 +71,9 @@ def get_course_outline_block_tree(request, course_id): block_types_filter=['course', 'chapter', 'sequential'] ) - course_outline_root_block = all_blocks['blocks'][all_blocks['root']] - populate_children(course_outline_root_block, all_blocks['blocks']) - set_last_accessed_default(course_outline_root_block) - mark_last_accessed(request.user, course_key, course_outline_root_block) - + course_outline_root_block = all_blocks['blocks'].get(all_blocks['root'], None) + if course_outline_root_block: + populate_children(course_outline_root_block, all_blocks['blocks']) + set_last_accessed_default(course_outline_root_block) + mark_last_accessed(request.user, course_key, course_outline_root_block) return course_outline_root_block diff --git a/openedx/features/course_experience/views/course_home.py b/openedx/features/course_experience/views/course_home.py index 14658b344b..b1cd16b562 100644 --- a/openedx/features/course_experience/views/course_home.py +++ b/openedx/features/course_experience/views/course_home.py @@ -83,14 +83,14 @@ class CourseHomeFragmentView(EdxFragmentView): return block course_outline_root_block = get_course_outline_block_tree(request, course_id) - last_accessed_block = get_last_accessed_block(course_outline_root_block) + last_accessed_block = get_last_accessed_block(course_outline_root_block) if course_outline_root_block else None has_visited_course = bool(last_accessed_block) if last_accessed_block: resume_course_url = last_accessed_block['lms_web_url'] else: - resume_course_url = course_outline_root_block['lms_web_url'] + resume_course_url = course_outline_root_block['lms_web_url'] if course_outline_root_block else None - return (has_visited_course, resume_course_url) + return has_visited_course, resume_course_url def _get_course_handouts(self, request, course): """ @@ -141,9 +141,6 @@ class CourseHomeFragmentView(EdxFragmentView): # Get the course tools enabled for this user and course course_tools = CourseToolsPluginManager.get_enabled_course_tools(request, course_key) - # Get the course tools enabled for this user and course - course_tools = CourseToolsPluginManager.get_enabled_course_tools(request, course_key) - # Render the course home fragment context = { 'request': request, diff --git a/openedx/features/course_experience/views/course_outline.py b/openedx/features/course_experience/views/course_outline.py index ad745f2cdd..d8c4263224 100644 --- a/openedx/features/course_experience/views/course_outline.py +++ b/openedx/features/course_experience/views/course_outline.py @@ -25,6 +25,8 @@ class CourseOutlineFragmentView(EdxFragmentView): course_overview = get_course_overview_with_access(request.user, 'load', course_key, check_if_enrolled=True) course_block_tree = get_course_outline_block_tree(request, course_id) + if not course_block_tree: + return None context = { 'csrf': csrf(request)['csrf_token'],