From 89c463ff2978eeb37a46bb783ccada688ce227ef Mon Sep 17 00:00:00 2001 From: Dillon Dumesnil Date: Thu, 6 May 2021 16:42:10 -0400 Subject: [PATCH] refactor: Switch to using course_overview start and end everywhere In DE-1822, we believed we needed to switch to start_date and end_date. It was determined this was not the case, so this updates the comment to ensure future users use the correct fields (start and end) and updates any pieces of code that may have used start_date or end_date. --- common/djangoapps/util/course.py | 5 ++--- lms/djangoapps/courseware/course_tools.py | 2 +- lms/djangoapps/courseware/tests/test_course_tools.py | 2 +- .../core/djangoapps/content/course_overviews/models.py | 10 +++++----- openedx/core/djangoapps/credit/services.py | 2 +- openedx/features/course_experience/utils.py | 6 ++---- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/common/djangoapps/util/course.py b/common/djangoapps/util/course.py index 03793baff7..fe6721499a 100644 --- a/common/djangoapps/util/course.py +++ b/common/djangoapps/util/course.py @@ -76,12 +76,11 @@ def has_certificates_enabled(course): def should_display_grade(course_overview): """ Returns True or False depending upon either certificate available date - or course-end-date + or course end date """ - course_end_date = course_overview.end_date cert_available_date = course_overview.certificate_available_date current_date = now().replace(hour=0, minute=0, second=0, microsecond=0) if cert_available_date: return cert_available_date < current_date - return course_end_date and course_end_date < current_date + return course_overview.end and course_overview.end < current_date diff --git a/lms/djangoapps/courseware/course_tools.py b/lms/djangoapps/courseware/course_tools.py index 86644ff05e..6a17374c9f 100644 --- a/lms/djangoapps/courseware/course_tools.py +++ b/lms/djangoapps/courseware/course_tools.py @@ -107,7 +107,7 @@ class FinancialAssistanceTool(CourseTool): return False # hide link for archived courses - if course_overview is not None and course_overview.end_date is not None and now > course_overview.end_date: + if course_overview is not None and course_overview.end is not None and now > course_overview.end: return False # hide link if not logged in or user not enrolled in the course diff --git a/lms/djangoapps/courseware/tests/test_course_tools.py b/lms/djangoapps/courseware/tests/test_course_tools.py index ff59493181..c057c55b17 100644 --- a/lms/djangoapps/courseware/tests/test_course_tools.py +++ b/lms/djangoapps/courseware/tests/test_course_tools.py @@ -179,7 +179,7 @@ class FinancialAssistanceToolTest(SharedModuleStoreTestCase): assert not FinancialAssistanceTool().is_enabled(self.request, self.course.id) def test_tool_not_visible_when_end_date_passed(self): - self.course_overview.end_date = self.now - datetime.timedelta(days=30) + self.course_overview.end = self.now - datetime.timedelta(days=30) self.course_overview.save() assert not FinancialAssistanceTool().is_enabled(self.request, self.course_overview.id) diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 7a78ffd424..741f8ef4f0 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -76,12 +76,15 @@ class CourseOverview(TimeStampedModel): display_number_with_default = TextField() display_org_with_default = TextField() - # Start/end dates - # TODO Remove 'start' & 'end' in removing field in column renaming, DE-1822 start = DateTimeField(null=True) end = DateTimeField(null=True) + + # These are deprecated and unused, but cannot be dropped via simple migration due to the size of the downstream + # history table. See DENG-19 for details. + # Please use start and end above for these values. start_date = DateTimeField(null=True) end_date = DateTimeField(null=True) + advertised_start = TextField(null=True) announcement = DateTimeField(null=True) @@ -194,10 +197,7 @@ class CourseOverview(TimeStampedModel): course_overview.display_org_with_default = course.display_org_with_default course_overview.start = start - # Add writes to new fields 'start_date' & 'end_date'. - course_overview.start_date = start course_overview.end = end - course_overview.end_date = end course_overview.advertised_start = course.advertised_start course_overview.announcement = course.announcement diff --git a/openedx/core/djangoapps/credit/services.py b/openedx/core/djangoapps/credit/services.py index 6966bbf691..4db8a944a5 100644 --- a/openedx/core/djangoapps/credit/services.py +++ b/openedx/core/djangoapps/credit/services.py @@ -103,7 +103,7 @@ class CreditService: course_overview = CourseOverview.get_from_id(course_key) result.update({ 'course_name': course_overview.display_name, - 'course_end_date': course_overview.end_date, + 'course_end_date': course_overview.end, }) return result diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index b26788cc79..d696a3ed0f 100644 --- a/openedx/features/course_experience/utils.py +++ b/openedx/features/course_experience/utils.py @@ -172,11 +172,9 @@ def dates_banner_should_display(course_key, user): return False, False course_overview = CourseOverview.objects.get(id=str(course_key)) - course_end_date = getattr(course_overview, 'end_date', None) - is_self_paced = getattr(course_overview, 'self_paced', False) # Only display the banner for self-paced courses - if not is_self_paced: + if not course_overview.self_paced: return False, False # Only display the banner for enrolled users @@ -184,7 +182,7 @@ def dates_banner_should_display(course_key, user): return False, False # Don't display the banner if the course has ended - if course_end_date and course_end_date < timezone.now(): + if course_overview.end and course_overview.end < timezone.now(): return False, False store = modulestore()