Skip to content

Commit

Permalink
Add the default enrollment start date on course creation (openedx#30954)
Browse files Browse the repository at this point in the history
* fix: main page course listing

The course is visible on the main page right after creation when the feature toggle `CREATE_COURSE_WITH_DEFAULT_ENROLLMENT_START_DATE` is on.

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
  • Loading branch information
dyudyunov authored and mehedikhan72 committed Aug 24, 2023
1 parent bdbc4fa commit 4bba8df
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 25 deletions.
53 changes: 32 additions & 21 deletions openedx/core/djangoapps/models/tests/test_course_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@


import datetime
from django.test import override_settings
import pytest
import ddt
from pytz import UTC
Expand All @@ -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)
Expand Down
22 changes: 21 additions & 1 deletion xmodule/course_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -57,6 +58,21 @@
COURSE_VIDEO_SHARING_PER_VIDEO = 'per-video'
COURSE_VIDEO_SHARING_ALL_VIDEOS = 'all-on'
COURSE_VIDEO_SHARING_NONE = 'all-off'
# .. 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
Expand Down Expand Up @@ -315,7 +331,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"),
Expand Down
5 changes: 5 additions & 0 deletions xmodule/modulestore/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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', {}))
Expand Down
20 changes: 17 additions & 3 deletions xmodule/tests/test_course_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 4bba8df

Please sign in to comment.