From d59db2876f2150df387ef9f9f37a86acaca76c22 Mon Sep 17 00:00:00 2001 From: Eugene Dyudyunov Date: Tue, 5 Sep 2023 16:11:11 +0300 Subject: [PATCH] feat: add the default enrollment start date on course creation (#33150) This is a backport from the master branch: https://github.com/openedx/edx-platform/pull/30954 The course is visible on the main page right after creation. So anonymous users can see them and access the course about page for the courses without valid data (e.g. they will see the default course overview) When courses list filtering is processed it checks the `see_exists` permission for the anonymous user. Actually, `see_exists` means `can_load` OR `can_enroll`. `can_load` is False in our case because the course start in the future. But `can_enroll` returns True because the course's enrollment_start and enrollment_end dates are blank: ``` enrollment_start = courselike.enrollment_start or datetime.min.replace(tzinfo=UTC) enrollment_end = courselike.enrollment_end or datetime.max.replace(tzinfo=UTC) if enrollment_start < now < enrollment_end: debug("Allow: in enrollment period") return ACCESS_GRANTED ``` Set the enrollment_start the same as a course start by default if the CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE feature toggle is enabled (default is False). --- .../models/tests/test_course_details.py | 53 +++++++++++-------- xmodule/course_block.py | 23 +++++++- xmodule/modulestore/tests/factories.py | 5 ++ xmodule/tests/test_course_block.py | 20 +++++-- 4 files changed, 76 insertions(+), 25 deletions(-) diff --git a/openedx/core/djangoapps/models/tests/test_course_details.py b/openedx/core/djangoapps/models/tests/test_course_details.py index 98e3c4b15df0..86301740c7f1 100644 --- a/openedx/core/djangoapps/models/tests/test_course_details.py +++ b/openedx/core/djangoapps/models/tests/test_course_details.py @@ -4,6 +4,7 @@ import datetime +from django.test import override_settings import pytest import ddt from pytz import UTC @@ -29,27 +30,37 @@ class CourseDetailsTestCase(ModuleStoreTestCase): def setUp(self): super().setUp() - self.course = CourseFactory.create() - - def test_virgin_fetch(self): - details = CourseDetails.fetch(self.course.id) - assert details.org == self.course.location.org, 'Org not copied into' - assert details.course_id == self.course.location.course, 'Course_id not copied into' - assert details.run == self.course.location.run, 'Course run not copied into' - assert details.course_image_name == self.course.course_image - assert details.start_date.tzinfo is not None - assert details.end_date is None, ('end date somehow initialized ' + str(details.end_date)) - assert details.enrollment_start is None,\ - ('enrollment_start date somehow initialized ' + str(details.enrollment_start)) - assert details.enrollment_end is None,\ - ('enrollment_end date somehow initialized ' + str(details.enrollment_end)) - assert details.certificate_available_date is None,\ - ('certificate_available_date date somehow initialized ' + str(details.certificate_available_date)) - assert details.syllabus is None, ('syllabus somehow initialized' + str(details.syllabus)) - assert details.intro_video is None, ('intro_video somehow initialized' + str(details.intro_video)) - assert details.effort is None, ('effort somehow initialized' + str(details.effort)) - assert details.language is None, ('language somehow initialized' + str(details.language)) - assert not details.self_paced + self.course = CourseFactory.create(default_enrollment_start=True) + + @ddt.data(True, False) + def test_virgin_fetch(self, should_have_default_enroll_start): + features = settings.FEATURES.copy() + features['CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE'] = should_have_default_enroll_start + + with override_settings(FEATURES=features): + course = CourseFactory.create(default_enrollment_start=should_have_default_enroll_start) + details = CourseDetails.fetch(course.id) + wrong_enrollment_start_msg = ( + 'enrollment_start not copied into' + if should_have_default_enroll_start + else f'enrollment_start date somehow initialized {str(details.enrollment_start)}' + ) + assert details.org == course.location.org, 'Org not copied into' + assert details.course_id == course.location.course, 'Course_id not copied into' + assert details.run == course.location.run, 'Course run not copied into' + assert details.course_image_name == course.course_image + assert details.start_date.tzinfo is not None + assert details.end_date is None, ('end date somehow initialized ' + str(details.end_date)) + assert details.enrollment_start == course.enrollment_start, wrong_enrollment_start_msg + assert details.enrollment_end is None,\ + ('enrollment_end date somehow initialized ' + str(details.enrollment_end)) + assert details.certificate_available_date is None,\ + ('certificate_available_date date somehow initialized ' + str(details.certificate_available_date)) + assert details.syllabus is None, ('syllabus somehow initialized' + str(details.syllabus)) + assert details.intro_video is None, ('intro_video somehow initialized' + str(details.intro_video)) + assert details.effort is None, ('effort somehow initialized' + str(details.effort)) + assert details.language is None, ('language somehow initialized' + str(details.language)) + assert not details.self_paced def test_update_and_fetch(self): jsondetails = CourseDetails.fetch(self.course.id) diff --git a/xmodule/course_block.py b/xmodule/course_block.py index 27b5cabd4464..b172b0059e1a 100644 --- a/xmodule/course_block.py +++ b/xmodule/course_block.py @@ -11,6 +11,7 @@ import requests from django.conf import settings from django.core.validators import validate_email +from edx_toggles.toggles import SettingDictToggle from lazy import lazy from lxml import etree from path import Path as path @@ -54,6 +55,22 @@ COURSE_VISIBILITY_PUBLIC_OUTLINE = 'public_outline' COURSE_VISIBILITY_PUBLIC = 'public' +# .. toggle_name: FEATURES['CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE'] +# .. toggle_implementation: SettingDictToggle +# .. toggle_default: False +# .. toggle_description: The default behavior, when this is disabled, is that a newly created course has no +# enrollment_start date set. When the feature is enabled - the newly created courses will have the +# enrollment_start_date set to DEFAULT_START_DATE. This is intended to be a permanent option. +# This toggle affects the course listing pages (platform's index page, /courses page) when course search is +# performed using the `lms.djangoapp.branding.get_visible_courses` method and the +# COURSE_CATALOG_VISIBILITY_PERMISSION setting is set to 'see_exists'. Switching the toggle to True will prevent +# the newly created (empty) course from appearing in the course listing. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2023-06-22 +CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE = SettingDictToggle( + "FEATURES", "CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE", default=False, module_name=__name__ +) + class StringOrDate(Date): # lint-amnesty, pylint: disable=missing-class-docstring def from_json(self, value): # lint-amnesty, pylint: disable=arguments-differ @@ -311,7 +328,11 @@ class CourseFields: # lint-amnesty, pylint: disable=missing-class-docstring ) wiki_slug = String(help=_("Slug that points to the wiki for this course"), scope=Scope.content) - enrollment_start = Date(help=_("Date that enrollment for this class is opened"), scope=Scope.settings) + enrollment_start = Date( + help=_("Date that enrollment for this class is opened"), + default=DEFAULT_START_DATE if CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE.is_enabled() else None, + scope=Scope.settings + ) enrollment_end = Date(help=_("Date that enrollment for this class is closed"), scope=Scope.settings) start = Date( help=_("Start time when this block is visible"), diff --git a/xmodule/modulestore/tests/factories.py b/xmodule/modulestore/tests/factories.py index b740b10e053f..9e0a2aa5cfd1 100644 --- a/xmodule/modulestore/tests/factories.py +++ b/xmodule/modulestore/tests/factories.py @@ -123,6 +123,11 @@ def _create(cls, target_class, **kwargs): # lint-amnesty, pylint: disable=argum run = kwargs.pop('run', name) user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test) emit_signals = kwargs.pop('emit_signals', False) + # By default course has enrollment_start in the future which means course is closed for enrollment. + # We're setting the 'enrollment_start' field to None to reduce number of arguments needed to setup course. + # Use the 'default_enrollment_start=True' kwarg to skip this and use the default enrollment_start date. + if not kwargs.get('enrollment_start', kwargs.pop('default_enrollment_start', False)): + kwargs['enrollment_start'] = None # Pass the metadata just as field=value pairs kwargs.update(kwargs.pop('metadata', {})) diff --git a/xmodule/tests/test_course_block.py b/xmodule/tests/test_course_block.py index 183a4f9c069e..86a98e5ffd57 100644 --- a/xmodule/tests/test_course_block.py +++ b/xmodule/tests/test_course_block.py @@ -4,20 +4,22 @@ import itertools import unittest from datetime import datetime, timedelta +import sys from unittest.mock import Mock, patch -import pytest import ddt from dateutil import parser from django.conf import settings from django.test import override_settings from fs.memoryfs import MemoryFS from opaque_keys.edx.keys import CourseKey +import pytest from pytz import utc from xblock.runtime import DictKeyValueStore, KvsFieldData from openedx.core.lib.teams_config import TeamsConfig, DEFAULT_COURSE_RUN_MAX_TEAM_SIZE import xmodule.course_block +from xmodule.course_metadata_utils import DEFAULT_START_DATE from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.xml import ImportSystem, XMLModuleStore from xmodule.modulestore.exceptions import InvalidProctoringProvider @@ -32,10 +34,22 @@ _NEXT_WEEK = _TODAY + timedelta(days=7) -class CourseFieldsTestCase(unittest.TestCase): +@ddt.ddt() +class CourseFieldsTestCase(unittest.TestCase): # lint-amnesty, pylint: disable=missing-class-docstring def test_default_start_date(self): - assert xmodule.course_block.CourseFields.start.default == datetime(2030, 1, 1, tzinfo=utc) + assert xmodule.course_block.CourseFields.start.default == DEFAULT_START_DATE + + @ddt.data(True, False) + def test_default_enrollment_start_date(self, should_have_default_enroll_start): + features = settings.FEATURES.copy() + features['CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE'] = should_have_default_enroll_start + with override_settings(FEATURES=features): + # reimport, so settings override could take effect + del sys.modules['xmodule.course_block'] + import xmodule.course_block # lint-amnesty, pylint: disable=redefined-outer-name, reimported + expected = DEFAULT_START_DATE if should_have_default_enroll_start else None + assert xmodule.course_block.CourseFields.enrollment_start.default == expected class DummySystem(ImportSystem): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring