From 98328ea4264ee7006d0e2c39b75e9bcfe090092a Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 3 Feb 2020 16:27:09 -0500 Subject: [PATCH 1/2] On publish, add evenly spaced dates to self-paced courses --- .../course_api/blocks/tests/test_api.py | 6 +-- lms/djangoapps/courseware/tests/test_views.py | 8 ++-- lms/djangoapps/grades/tests/test_tasks.py | 20 ++++---- lms/djangoapps/instructor/tests/test_api.py | 9 ++++ lms/djangoapps/instructor/tests/test_tools.py | 7 ++- .../djangoapps/course_date_signals/admin.py | 44 ++++++++++++++++++ .../course_date_signals/handlers.py | 17 +++++++ .../migrations/0001_initial.py | 46 +++++++++++++++++++ .../migrations/__init__.py | 0 .../djangoapps/course_date_signals/models.py | 16 +++++++ .../djangoapps/course_date_signals/utils.py | 39 ++++++++++++++++ .../features/course_duration_limits/access.py | 22 +-------- .../tests/test_course_expiration.py | 19 ++++---- requirements/constraints.txt | 6 ++- requirements/edx/base.txt | 6 +-- requirements/edx/development.txt | 8 ++-- requirements/edx/pip-tools.txt | 2 +- requirements/edx/testing.txt | 6 +-- 18 files changed, 220 insertions(+), 61 deletions(-) create mode 100644 openedx/core/djangoapps/course_date_signals/admin.py create mode 100644 openedx/core/djangoapps/course_date_signals/migrations/0001_initial.py create mode 100644 openedx/core/djangoapps/course_date_signals/migrations/__init__.py create mode 100644 openedx/core/djangoapps/course_date_signals/models.py create mode 100644 openedx/core/djangoapps/course_date_signals/utils.py diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index b7694edfc7..46ccca1506 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -243,7 +243,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): self._get_blocks( course, expected_mongo_queries=0, - expected_sql_queries=13 if with_storage_backing else 12, + expected_sql_queries=14 if with_storage_backing else 13, ) @ddt.data( @@ -260,9 +260,9 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): clear_course_from_cache(course.id) if with_storage_backing: - num_sql_queries = 23 + num_sql_queries = 24 else: - num_sql_queries = 13 + num_sql_queries = 14 self._get_blocks( course, diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 732ac62483..aa88525002 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1492,8 +1492,8 @@ class ProgressPageTests(ProgressPageBaseTests): self.assertContains(resp, u"Download Your Certificate") @ddt.data( - (True, 53), - (False, 52) + (True, 57), + (False, 56) ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1506,8 +1506,8 @@ class ProgressPageTests(ProgressPageBaseTests): @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 61, 40), - (True, 52, 35) + (False, 65, 44), + (True, 56, 39) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 316be35a76..f088a19d75 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -164,10 +164,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(mock_block_structure_create.call_count, 1) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 37, True), - (ModuleStoreEnum.Type.mongo, 1, 37, False), - (ModuleStoreEnum.Type.split, 3, 37, True), - (ModuleStoreEnum.Type.split, 3, 37, False), + (ModuleStoreEnum.Type.mongo, 1, 38, True), + (ModuleStoreEnum.Type.mongo, 1, 38, False), + (ModuleStoreEnum.Type.split, 3, 38, True), + (ModuleStoreEnum.Type.split, 3, 38, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -179,8 +179,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 37), - (ModuleStoreEnum.Type.split, 3, 37), + (ModuleStoreEnum.Type.mongo, 1, 38), + (ModuleStoreEnum.Type.split, 3, 38), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -225,8 +225,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest ) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 20), - (ModuleStoreEnum.Type.split, 3, 20), + (ModuleStoreEnum.Type.mongo, 1, 21), + (ModuleStoreEnum.Type.split, 3, 21), ) @ddt.unpack def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): @@ -240,8 +240,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @ddt.data( - (ModuleStoreEnum.Type.mongo, 1, 38), - (ModuleStoreEnum.Type.split, 3, 38), + (ModuleStoreEnum.Type.mongo, 1, 39), + (ModuleStoreEnum.Type.split, 3, 39), ) @ddt.unpack def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index dc1cb79711..5fa668f87f 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -65,10 +65,13 @@ from openedx.core.djangoapps.course_date_signals.handlers import extract_dates from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_COMMUNITY_TA from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles +from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin +from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag from openedx.core.lib.teams_config import TeamsConfig from openedx.core.lib.xblock_utils import grade_histogram +from openedx.features.course_experience import RELATIVE_DATES_FLAG from shoppingcart.models import ( Coupon, CouponRedemption, @@ -4478,6 +4481,8 @@ class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): self.user1 = user1 self.user2 = user2 + ScheduleFactory.create(enrollment__user=self.user1, enrollment__course_id=self.course.id) + ScheduleFactory.create(enrollment__user=self.user2, enrollment__course_id=self.course.id) self.instructor = InstructorFactory(course_key=self.course.id) self.client.login(username=self.instructor.username, password='test') extract_dates(None, self.course.id) @@ -4519,6 +4524,7 @@ class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase): get_extended_due(self.course, self.week3, self.user1) ) + @override_waffle_flag(RELATIVE_DATES_FLAG, True) def test_reset_date(self): self.test_change_due_date() url = reverse('reset_due_date', kwargs={'course_id': text_type(self.course.id)}) @@ -4637,10 +4643,13 @@ class TestDueDateExtensionsDeletedDate(ModuleStoreTestCase, LoginEnrollmentTestC self.user1 = user1 self.user2 = user2 + ScheduleFactory.create(enrollment__user=self.user1, enrollment__course_id=self.course.id) + ScheduleFactory.create(enrollment__user=self.user2, enrollment__course_id=self.course.id) self.instructor = InstructorFactory(course_key=self.course.id) self.client.login(username=self.instructor.username, password='test') extract_dates(None, self.course.id) + @override_waffle_flag(RELATIVE_DATES_FLAG, True) def test_reset_extension_to_deleted_date(self): """ Test that we can delete a due date extension after deleting the normal diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py index 260d13c065..4de2354726 100644 --- a/lms/djangoapps/instructor/tests/test_tools.py +++ b/lms/djangoapps/instructor/tests/test_tools.py @@ -15,9 +15,9 @@ from django.test import TestCase from opaque_keys.edx.keys import CourseKey from pytz import UTC -from edx_when import api from edx_when.field_data import DateLookupFieldData from openedx.core.djangoapps.course_date_signals import handlers +from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory from student.tests.factories import UserFactory from xmodule.fields import Date from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase @@ -228,6 +228,8 @@ class TestSetDueDateExtension(ModuleStoreTestCase): self.week3 = week3 self.user = user + ScheduleFactory.create(enrollment__user=self.user, enrollment__course_id=self.course.id) + inject_field_data((course, week1, week2, week3, homework, assignment), course, user) def _clear_field_data_cache(self): @@ -241,7 +243,6 @@ class TestSetDueDateExtension(ModuleStoreTestCase): block._field_data._load_dates(self.course.id, self.user, use_cached=False) # pylint: disable=protected-access block.fields['due']._del_cached_value(block) # pylint: disable=protected-access - @api.override_enabled() def test_set_due_date_extension(self): extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC) tools.set_due_date_extension(self.course, self.week1, self.user, extended) @@ -297,6 +298,8 @@ class TestDataDumps(ModuleStoreTestCase): self.week2 = week2 self.user1 = user1 self.user2 = user2 + ScheduleFactory.create(enrollment__user=self.user1, enrollment__course_id=self.course.id) + ScheduleFactory.create(enrollment__user=self.user2, enrollment__course_id=self.course.id) handlers.extract_dates(None, course.id) def test_dump_module_extensions(self): diff --git a/openedx/core/djangoapps/course_date_signals/admin.py b/openedx/core/djangoapps/course_date_signals/admin.py new file mode 100644 index 0000000000..4e63ba657d --- /dev/null +++ b/openedx/core/djangoapps/course_date_signals/admin.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- +""" +Django Admin pages for SelfPacedRelativeDatesConfig. +""" + + +from django.contrib import admin +from django.utils.translation import ugettext_lazy as _ + +from openedx.core.djangoapps.config_model_utils.admin import StackedConfigModelAdmin + +from .models import SelfPacedRelativeDatesConfig + + +class SelfPacedRelativeDatesConfigAdmin(StackedConfigModelAdmin): + """ + Admin for course duration limit + """ + fieldsets = ( + ('Context', { + 'fields': SelfPacedRelativeDatesConfig.KEY_FIELDS, + 'description': _( + 'These define the context to enable course duration limits on. ' + 'If no values are set, then the configuration applies globally. ' + 'If a single value is set, then the configuration applies to all courses ' + 'within that context. At most one value can be set at a time.
' + 'If multiple contexts apply to a course (for example, if configuration ' + 'is specified for the course specifically, and for the org that the course ' + 'is in, then the more specific context overrides the more general context.' + ), + }), + ('Configuration', { + 'fields': ('enabled',), + 'description': _( + 'If any of these values is left empty or "Unknown", then their value ' + 'at runtime will be retrieved from the next most specific context that applies. ' + 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' + 'course will be Enabled only if the org that it is in is Enabled.' + ), + }) + ) + raw_id_fields = ('course',) + +admin.site.register(SelfPacedRelativeDatesConfig, SelfPacedRelativeDatesConfigAdmin) diff --git a/openedx/core/djangoapps/course_date_signals/handlers.py b/openedx/core/djangoapps/course_date_signals/handlers.py index f88648d32a..0d8eb2ae0d 100644 --- a/openedx/core/djangoapps/course_date_signals/handlers.py +++ b/openedx/core/djangoapps/course_date_signals/handlers.py @@ -7,6 +7,8 @@ from django.dispatch import receiver from six import text_type from xblock.fields import Scope from xmodule.modulestore.django import SignalHandler, modulestore +from .models import SelfPacedRelativeDatesConfig +from .utils import get_expected_duration from edx_when.api import FIELDS_TO_EXTRACT, set_dates_for_course log = logging.getLogger(__name__) @@ -44,6 +46,21 @@ def extract_dates_from_course(course): # self-paced courses may accidentally have a course due date metadata.pop('due', None) date_items = [(course.location, metadata)] + + if SelfPacedRelativeDatesConfig.current(course_key=course.id).enabled: + duration = get_expected_duration(course) + sections = course.get_children() + time_per_week = duration / len(sections) + # Apply the same relative due date to all content inside a section, + # unless that item already has a relative date set + for idx, section in enumerate(sections): + items = [section] + while items: + next_item = items.pop() + # TODO: Once studio can manually set relative dates, + # we would need to manually check for them here + date_items.append((next_item.location, {'due': time_per_week * (idx + 1)})) + items.extend(next_item.get_children()) else: date_items = [] items = modulestore().get_items(course.id) diff --git a/openedx/core/djangoapps/course_date_signals/migrations/0001_initial.py b/openedx/core/djangoapps/course_date_signals/migrations/0001_initial.py new file mode 100644 index 0000000000..7efc61c9e3 --- /dev/null +++ b/openedx/core/djangoapps/course_date_signals/migrations/0001_initial.py @@ -0,0 +1,46 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.27 on 2020-02-03 21:22 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import openedx.core.djangoapps.config_model_utils.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('course_overviews', '0019_improve_courseoverviewtab'), + ('sites', '0002_alter_domain_unique'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='SelfPacedRelativeDatesConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.NullBooleanField(default=None, verbose_name='Enabled')), + ('org', models.CharField(blank=True, db_index=True, help_text='Configure values for all course runs associated with this Organization. This is the organization string (i.e. edX, MITx).', max_length=255, null=True)), + ('org_course', models.CharField(blank=True, db_index=True, help_text="Configure values for all course runs associated with this course. This is should be formatted as 'org+course' (i.e. MITx+6.002x, HarvardX+CS50).", max_length=255, null=True, validators=[openedx.core.djangoapps.config_model_utils.models.validate_course_in_org], verbose_name='Course in Org')), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ('course', models.ForeignKey(blank=True, help_text='Configure values for this course run. This should be formatted as the CourseKey (i.e. course-v1://MITx+6.002x+2019_Q1)', null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='course_overviews.CourseOverview', verbose_name='Course Run')), + ('site', models.ForeignKey(blank=True, help_text='Configure values for all course runs associated with this site.', null=True, on_delete=django.db.models.deletion.CASCADE, to='sites.Site')), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddIndex( + model_name='selfpacedrelativedatesconfig', + index=models.Index(fields=['site', 'org', 'course'], name='course_date_site_id_a44836_idx'), + ), + migrations.AddIndex( + model_name='selfpacedrelativedatesconfig', + index=models.Index(fields=['site', 'org', 'org_course', 'course'], name='course_date_site_id_c0164a_idx'), + ), + ] diff --git a/openedx/core/djangoapps/course_date_signals/migrations/__init__.py b/openedx/core/djangoapps/course_date_signals/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/course_date_signals/models.py b/openedx/core/djangoapps/course_date_signals/models.py new file mode 100644 index 0000000000..18a2031012 --- /dev/null +++ b/openedx/core/djangoapps/course_date_signals/models.py @@ -0,0 +1,16 @@ +""" +Models for configuration course_date_signals + +SelfPacedRelativeDatesConfig: + manage which orgs/courses/course runs have self-paced relative dates enabled +""" + +from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel + + +class SelfPacedRelativeDatesConfig(StackedConfigurationModel): + """ + Configuration to manage the SelfPacedRelativeDates settings. + + .. no_pii: + """ diff --git a/openedx/core/djangoapps/course_date_signals/utils.py b/openedx/core/djangoapps/course_date_signals/utils.py new file mode 100644 index 0000000000..a006268c9e --- /dev/null +++ b/openedx/core/djangoapps/course_date_signals/utils.py @@ -0,0 +1,39 @@ +""" +Utility functions around course dates. + +get_expected_duration: return the expected duration of a course (absent any user information) +""" + +from datetime import timedelta + +from course_modes.models import CourseMode +from openedx.core.djangoapps.catalog.utils import get_course_run_details + + +MIN_DURATION = timedelta(weeks=4) +MAX_DURATION = timedelta(weeks=18) + + +def get_expected_duration(course): + """ + Return a `datetime.timedelta` defining the expected length of the supplied course. + """ + + access_duration = MIN_DURATION + + verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True) + + if not verified_mode: + return None + + # The user course expiration date is the content availability date + # plus the weeks_to_complete field from course-discovery. + discovery_course_details = get_course_run_details(course.id, ['weeks_to_complete']) + expected_weeks = discovery_course_details.get('weeks_to_complete') + if expected_weeks: + access_duration = timedelta(weeks=expected_weeks) + + # Course access duration is bounded by the min and max duration. + access_duration = max(MIN_DURATION, min(MAX_DURATION, access_duration)) + + return access_duration diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index 8f69656b5a..ea43617e4d 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -21,13 +21,12 @@ from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_specific_student from openedx.core.djangoapps.catalog.utils import get_course_run_details from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.course_date_signals.utils import get_expected_duration from openedx.core.djangolib.markup import HTML from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from student.models import CourseEnrollment from util.date_utils import strftime_localized -MIN_DURATION = timedelta(weeks=4) -MAX_DURATION = timedelta(weeks=18) EXPIRATION_DATE_FORMAT_STR = u'%b %-d, %Y' @@ -64,28 +63,11 @@ def get_user_course_duration(user, course): - If course fields are missing, default course access duration to MIN_DURATION. """ - access_duration = MIN_DURATION - - verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True) - - if not verified_mode: - return None - enrollment = CourseEnrollment.get_enrollment(user, course.id) if enrollment is None or enrollment.mode != CourseMode.AUDIT: return None - # The user course expiration date is the content availability date - # plus the weeks_to_complete field from course-discovery. - discovery_course_details = get_course_run_details(course.id, ['weeks_to_complete']) - expected_weeks = discovery_course_details.get('weeks_to_complete') - if expected_weeks: - access_duration = timedelta(weeks=expected_weeks) - - # Course access duration is bounded by the min and max duration. - access_duration = max(MIN_DURATION, min(MAX_DURATION, access_duration)) - - return access_duration + return get_expected_duration(course) def get_user_course_expiration_date(user, course): diff --git a/openedx/features/course_duration_limits/tests/test_course_expiration.py b/openedx/features/course_duration_limits/tests/test_course_expiration.py index fb8496278a..720af17159 100644 --- a/openedx/features/course_duration_limits/tests/test_course_expiration.py +++ b/openedx/features/course_duration_limits/tests/test_course_expiration.py @@ -25,6 +25,7 @@ from lms.djangoapps.courseware.tests.factories import ( ) from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.course_date_signals.utils import MAX_DURATION, MIN_DURATION from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, @@ -33,7 +34,7 @@ from openedx.core.djangoapps.django_comment_common.models import ( ) from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory from openedx.features.content_type_gating.helpers import CONTENT_GATING_PARTITION_ID, CONTENT_TYPE_GATE_GROUP_IDS -from openedx.features.course_duration_limits.access import MAX_DURATION, MIN_DURATION, get_user_course_expiration_date +from openedx.features.course_duration_limits.access import get_user_course_expiration_date from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience.tests.views.helpers import add_course_mode from student.models import CourseEnrollment, FBEEnrollmentExclusion @@ -68,7 +69,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): result = get_user_course_expiration_date(self.user, CourseOverview.get_from_id(self.course.id)) self.assertEqual(result, None) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") @ddt.data( [int(MIN_DURATION.days / 7) - 1, MIN_DURATION, False], [7, timedelta(weeks=7), False], @@ -102,7 +103,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): ) self.assertEqual(result, enrollment.created + access_duration) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") def test_content_availability_date(self, mock_get_course_run_details): """ Content availability date is course start date or enrollment date, whichever is later. @@ -146,7 +147,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): content_availability_date = start_date.replace(microsecond=0) self.assertEqual(result, content_availability_date + access_duration) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") def test_expired_upgrade_deadline(self, mock_get_course_run_details): """ The expiration date still exists if the upgrade deadline has passed @@ -165,7 +166,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): content_availability_date = enrollment.created self.assertEqual(result, content_availability_date + access_duration) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") @ddt.data( ({'user_partition_id': CONTENT_GATING_PARTITION_ID, 'group_id': CONTENT_TYPE_GATE_GROUP_IDS['limited_access']}, True), @@ -245,7 +246,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): self.assertEqual(response.status_code, 200) return response - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") def test_masquerade_in_holdback(self, mock_get_course_run_details): mock_get_course_run_details.return_value = {'weeks_to_complete': 12} audit_student = UserFactory(username='audit') @@ -279,7 +280,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): banner_text = 'You lose all access to this course, including your progress,' self.assertNotContains(response, banner_text) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") def test_masquerade_expired(self, mock_get_course_run_details): mock_get_course_run_details.return_value = {'weeks_to_complete': 1} @@ -315,7 +316,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): banner_text = 'This learner does not have access to this course. Their access expired on' self.assertContains(response, banner_text) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") @ddt.data( InstructorFactory, StaffFactory, @@ -366,7 +367,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase): banner_text = 'This learner does not have access to this course. Their access expired on' self.assertNotContains(response, banner_text) - @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details") + @mock.patch("openedx.core.djangoapps.course_date_signals.utils.get_course_run_details") @ddt.data( FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR, diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 5ab4a83dd7..c0b3a55691 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -17,8 +17,10 @@ django-oauth-toolkit<1.2.0 # Version 4.0.0 dropped support for Django < 2.0.1 django-model-utils<4.0.0 -# Code changes must be made to support 0.7.1 -edx-when==0.7.0 +# 1.2.3 breaks unittest in +# lms.djangoapps.course_api.tests.test_views.CourseListSearchViewTest.test_list_all_with_search_term +# acceptance.tests.lms.test_lms_course_discovery.CourseDiscoveryTest.test_search +edx-search==1.2.2 # Upgrading to 2.12.0 breaks several test classes due to API changes, need to update our code accordingly factory-boy==2.8.1 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index b356487f0e..09171f2152 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -114,12 +114,12 @@ edx-proctoring-proctortrack==1.0.5 edx-proctoring==2.2.7 edx-rbac==1.1.0 # via edx-enterprise edx-rest-api-client==3.0.2 -edx-search==1.3.3 +edx-search==1.2.2 edx-sga==0.10.0 edx-submissions==3.0.4 edx-tincan-py35==0.0.5 # via edx-enterprise edx-user-state-client==1.1.2 -edx-when==0.7.0 +edx-when==1.0.1 edxval==1.2.3 elasticsearch==1.9.0 # via edx-search enum34==1.1.6 # via edxval @@ -233,7 +233,7 @@ staff-graded-xblock==0.7 stevedore==1.32.0 super-csv==0.9.6 sympy==1.5.1 -testfixtures==6.13.0 # via edx-enterprise +testfixtures==6.13.1 # via edx-enterprise text-unidecode==1.3 # via python-slugify unicodecsv==0.14.1 uritemplate==3.0.1 # via coreapi, drf-yasg diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 42f59c92c3..743ec05052 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -127,13 +127,13 @@ edx-proctoring-proctortrack==1.0.5 edx-proctoring==2.2.7 edx-rbac==1.1.0 edx-rest-api-client==3.0.2 -edx-search==1.3.3 +edx-search==1.2.2 edx-sga==0.10.0 edx-sphinx-theme==1.5.0 edx-submissions==3.0.4 edx-tincan-py35==0.0.5 edx-user-state-client==1.1.2 -edx-when==0.7.0 +edx-when==1.0.1 edxval==1.2.3 elasticsearch==1.9.0 entrypoints==0.3 @@ -215,7 +215,7 @@ pbr==5.4.4 pdfminer.six==20200124 piexif==1.1.3 pillow==7.0.0 -pip-tools==4.4.1 +pip-tools==4.5.0 pkgconfig==1.5.1 pluggy==0.13.1 polib==1.1.0 @@ -303,7 +303,7 @@ staff-graded-xblock==0.7 stevedore==1.32.0 super-csv==0.9.6 sympy==1.5.1 -testfixtures==6.13.0 +testfixtures==6.13.1 text-unidecode==1.3 toml==0.10.0 tox-battery==0.5.2 diff --git a/requirements/edx/pip-tools.txt b/requirements/edx/pip-tools.txt index 7f4416af58..5d165c891e 100644 --- a/requirements/edx/pip-tools.txt +++ b/requirements/edx/pip-tools.txt @@ -5,5 +5,5 @@ # make upgrade # click==7.0 # via pip-tools -pip-tools==4.4.1 +pip-tools==4.5.0 six==1.14.0 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 597ae1e4b1..748640e5ae 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -123,12 +123,12 @@ edx-proctoring-proctortrack==1.0.5 edx-proctoring==2.2.7 edx-rbac==1.1.0 edx-rest-api-client==3.0.2 -edx-search==1.3.3 +edx-search==1.2.2 edx-sga==0.10.0 edx-submissions==3.0.4 edx-tincan-py35==0.0.5 edx-user-state-client==1.1.2 -edx-when==0.7.0 +edx-when==1.0.1 edxval==1.2.3 elasticsearch==1.9.0 entrypoints==0.3 # via flake8 @@ -281,7 +281,7 @@ staff-graded-xblock==0.7 stevedore==1.32.0 super-csv==0.9.6 sympy==1.5.1 -testfixtures==6.13.0 +testfixtures==6.13.1 text-unidecode==1.3 toml==0.10.0 # via tox tox-battery==0.5.2 From b47eb0f24c14a8c9d117336869aa224cf39a48b0 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 21 Feb 2020 10:16:30 -0500 Subject: [PATCH 2/2] Extract common configuration/documentation into the base StackedConfigModelAdmin class --- .../djangoapps/config_model_utils/admin.py | 45 ++++++++++++++++++- .../djangoapps/course_date_signals/admin.py | 32 +------------ openedx/features/content_type_gating/admin.py | 29 +----------- .../features/course_duration_limits/admin.py | 33 +------------- 4 files changed, 46 insertions(+), 93 deletions(-) diff --git a/openedx/core/djangoapps/config_model_utils/admin.py b/openedx/core/djangoapps/config_model_utils/admin.py index decfa7f65d..e175e72947 100644 --- a/openedx/core/djangoapps/config_model_utils/admin.py +++ b/openedx/core/djangoapps/config_model_utils/admin.py @@ -5,7 +5,9 @@ Convenience classes for defining StackedConfigModel Admin pages. from config_models.admin import ConfigurationModelAdmin from django import forms +from django.utils.translation import ugettext_lazy as _ from opaque_keys.edx.keys import CourseKey +from openedx.core.djangolib.markup import HTML, Text class CourseOverviewField(forms.ModelChoiceField): @@ -28,9 +30,48 @@ class StackedConfigModelAdmin(ConfigurationModelAdmin): """ form = StackedConfigModelAdminForm - def get_fields(self, request, obj=None): + raw_id_fields = ('course',) + + def get_fieldsets(self, request, obj=None): + return ( + ('Context', { + 'fields': self.key_fields, + 'description': Text(_( + 'These define the context to enable this configuration on. ' + 'If no values are set, then the configuration applies globally. ' + 'If a single value is set, then the configuration applies to all courses ' + 'within that context. At most one value can be set at a time.{br}' + 'If multiple contexts apply to a course (for example, if configuration ' + 'is specified for the course specifically, and for the org that the course ' + 'is in, then the more specific context overrides the more general context.' + )).format(br=HTML('
')), + }), + ('Configuration', { + 'fields': self.stackable_fields, + 'description': _( + 'If any of these values are left empty or "Unknown", then their value ' + 'at runtime will be retrieved from the next most specific context that applies. ' + 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' + 'course will be Enabled only if the org that it is in is Enabled.' + ), + }) + ) + + @property + def key_fields(self): + return list(self.model.KEY_FIELDS) + + @property + def stackable_fields(self): + return list(self.model.STACKABLE_FIELDS) + + @property + def config_fields(self): fields = super(StackedConfigModelAdmin, self).get_fields(request, obj) - return list(self.model.KEY_FIELDS) + [field for field in fields if field not in self.model.KEY_FIELDS] + return [field for field in fields if field not in self.key_fields] + + def get_fields(self, request, obj=None): + return self.key_fields + self.config_fields def get_displayable_field_names(self): """ diff --git a/openedx/core/djangoapps/course_date_signals/admin.py b/openedx/core/djangoapps/course_date_signals/admin.py index 4e63ba657d..74d35fb249 100644 --- a/openedx/core/djangoapps/course_date_signals/admin.py +++ b/openedx/core/djangoapps/course_date_signals/admin.py @@ -5,40 +5,10 @@ Django Admin pages for SelfPacedRelativeDatesConfig. from django.contrib import admin -from django.utils.translation import ugettext_lazy as _ from openedx.core.djangoapps.config_model_utils.admin import StackedConfigModelAdmin from .models import SelfPacedRelativeDatesConfig -class SelfPacedRelativeDatesConfigAdmin(StackedConfigModelAdmin): - """ - Admin for course duration limit - """ - fieldsets = ( - ('Context', { - 'fields': SelfPacedRelativeDatesConfig.KEY_FIELDS, - 'description': _( - 'These define the context to enable course duration limits on. ' - 'If no values are set, then the configuration applies globally. ' - 'If a single value is set, then the configuration applies to all courses ' - 'within that context. At most one value can be set at a time.
' - 'If multiple contexts apply to a course (for example, if configuration ' - 'is specified for the course specifically, and for the org that the course ' - 'is in, then the more specific context overrides the more general context.' - ), - }), - ('Configuration', { - 'fields': ('enabled',), - 'description': _( - 'If any of these values is left empty or "Unknown", then their value ' - 'at runtime will be retrieved from the next most specific context that applies. ' - 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' - 'course will be Enabled only if the org that it is in is Enabled.' - ), - }) - ) - raw_id_fields = ('course',) - -admin.site.register(SelfPacedRelativeDatesConfig, SelfPacedRelativeDatesConfigAdmin) +admin.site.register(SelfPacedRelativeDatesConfig, StackedConfigModelAdmin) diff --git a/openedx/features/content_type_gating/admin.py b/openedx/features/content_type_gating/admin.py index be4ca4aa8a..e02b632977 100644 --- a/openedx/features/content_type_gating/admin.py +++ b/openedx/features/content_type_gating/admin.py @@ -5,37 +5,10 @@ Django Admin pages for ContentTypeGatingConfig. from django.contrib import admin -from django.utils.translation import ugettext_lazy as _ from openedx.core.djangoapps.config_model_utils.admin import StackedConfigModelAdmin from .models import ContentTypeGatingConfig -class ContentTypeGatingConfigAdmin(StackedConfigModelAdmin): - fieldsets = ( - ('Context', { - 'fields': ContentTypeGatingConfig.KEY_FIELDS, - 'description': _( - 'These define the context to enable course duration limits on. ' - 'If no values are set, then the configuration applies globally. ' - 'If a single value is set, then the configuration applies to all courses ' - 'within that context. At most one value can be set at a time.
' - 'If multiple contexts apply to a course (for example, if configuration ' - 'is specified for the course specifically, and for the org that the course ' - 'is in, then the more specific context overrides the more general context.' - ), - }), - ('Configuration', { - 'fields': ('enabled', 'enabled_as_of', 'studio_override_enabled'), - 'description': _( - 'If any of these values is left empty or "Unknown", then their value ' - 'at runtime will be retrieved from the next most specific context that applies. ' - 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' - 'course will be Enabled only if the org that it is in is Enabled.' - ), - }) - ) - raw_id_fields = ('course',) - -admin.site.register(ContentTypeGatingConfig, ContentTypeGatingConfigAdmin) +admin.site.register(ContentTypeGatingConfig, StackedConfigModelAdmin) diff --git a/openedx/features/course_duration_limits/admin.py b/openedx/features/course_duration_limits/admin.py index 0184b443a4..56e64f5c9f 100644 --- a/openedx/features/course_duration_limits/admin.py +++ b/openedx/features/course_duration_limits/admin.py @@ -5,40 +5,9 @@ Django Admin pages for CourseDurationLimitConfig. from django.contrib import admin -from django.utils.translation import ugettext_lazy as _ from openedx.core.djangoapps.config_model_utils.admin import StackedConfigModelAdmin from .models import CourseDurationLimitConfig - -class CourseDurationLimitConfigAdmin(StackedConfigModelAdmin): - """ - Admin for course duration limit - """ - fieldsets = ( - ('Context', { - 'fields': CourseDurationLimitConfig.KEY_FIELDS, - 'description': _( - 'These define the context to enable course duration limits on. ' - 'If no values are set, then the configuration applies globally. ' - 'If a single value is set, then the configuration applies to all courses ' - 'within that context. At most one value can be set at a time.
' - 'If multiple contexts apply to a course (for example, if configuration ' - 'is specified for the course specifically, and for the org that the course ' - 'is in, then the more specific context overrides the more general context.' - ), - }), - ('Configuration', { - 'fields': ('enabled', 'enabled_as_of'), - 'description': _( - 'If any of these values is left empty or "Unknown", then their value ' - 'at runtime will be retrieved from the next most specific context that applies. ' - 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' - 'course will be Enabled only if the org that it is in is Enabled.' - ), - }) - ) - raw_id_fields = ('course',) - -admin.site.register(CourseDurationLimitConfig, CourseDurationLimitConfigAdmin) +admin.site.register(CourseDurationLimitConfig, StackedConfigModelAdmin)