diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 0dbf338218..988feed9f6 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -41,6 +41,7 @@ class CourseMetadata: 'enrollment_start', 'enrollment_end', 'certificate_available_date', + 'certificates_display_behavior', 'tabs', 'graceperiod', 'show_timezone', diff --git a/cms/envs/common.py b/cms/envs/common.py index 91eea5ce26..a864c8725e 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -451,6 +451,17 @@ FEATURES = { # .. toggle_creation_date: 2021-03-05 # .. toggle_tickets: https://github.com/edx/edx-platform/pull/26106 'ENABLE_HELP_LINK': True, + + # .. toggle_name: FEATURES['ENABLE_V2_CERT_DISPLAY_SETTINGS'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to use the reimagined certificates_display_behavior and certificate_available_date + # .. settings. Will eventually become the default. + # .. toggle_use_cases: temporary + # .. toggle_creation_date: 2021-07-26 + # .. toggle_target_removal_date: 2021-10-01 + # .. toggle_tickets: 'https://openedx.atlassian.net/browse/MICROBA-1405' + 'ENABLE_V2_CERT_DISPLAY_SETTINGS': False, } ENABLE_JASMINE = False diff --git a/cms/static/js/factories/settings.js b/cms/static/js/factories/settings.js index 6ed11c4916..a7a6d4b21e 100644 --- a/cms/static/js/factories/settings.js +++ b/cms/static/js/factories/settings.js @@ -2,7 +2,7 @@ define([ 'jquery', 'js/models/settings/course_details', 'js/views/settings/main' ], function($, CourseDetailsModel, MainView) { 'use strict'; - return function(detailsUrl, showMinGradeWarning, showCertificateAvailableDate, upgradeDeadline) { + return function(detailsUrl, showMinGradeWarning, showCertificateAvailableDate, upgradeDeadline, useV2CertDisplaySettings) { var model; // highlighting labels when fields are focused in $('form :input') @@ -13,9 +13,16 @@ define([ $('label').removeClass('is-focused'); }); + // Toggle collapsibles when trigger is clicked + $(".collapsible .collapsible-trigger").click(function() { + const contentId = this.id.replace("-trigger", "-content") + $(`#${contentId}`).toggleClass("collapsed") + }) + model = new CourseDetailsModel(); model.urlRoot = detailsUrl; model.showCertificateAvailableDate = showCertificateAvailableDate; + model.useV2CertDisplaySettings = useV2CertDisplaySettings; model.set('upgrade_deadline', upgradeDeadline); model.fetch({ success: function(model) { @@ -24,6 +31,7 @@ define([ model: model, showMinGradeWarning: showMinGradeWarning }); + editor.useV2CertDisplaySettings = useV2CertDisplaySettings; editor.render(); }, reset: true diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index c0d8c506f4..560816fabc 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -11,6 +11,7 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js language: '', start_date: null, // maps to 'start' end_date: null, // maps to 'end' + certificates_display_behavior: "", certificate_available_date: null, enrollment_start: null, enrollment_end: null, @@ -42,6 +43,12 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js // Returns either nothing (no return call) so that validate works or an object of {field: errorstring} pairs // A bit funny in that the video key validation is asynchronous; so, it won't stop the validation. var errors = {}; + const CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS = { + END: "end", + END_WITH_DATE: "end_with_date", + EARLY_NO_INFO: "early_no_info" + }; + newattrs = DateUtils.convertDateStringsToObjects( newattrs, ['start_date', 'end_date', 'certificate_available_date', 'enrollment_start', 'enrollment_end'] @@ -75,6 +82,39 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js 'The certificate available date must be later than the course end date.' ); } + + if (this.useV2CertDisplaySettings){ + if ( + newattrs.certificates_display_behavior + && !(Object.values(CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS).includes(newattrs.certificates_display_behavior)) + ) { + + errors.certificates_display_behavior = StringUtils.interpolate( + gettext( + "The certificate display behavior must be one of: {behavior_options}" + ), + { + behavior_options: Object.values(CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS).join(', ') + } + ); + } + + // Throw error if there's a value for certificate_available_date + if( + (newattrs.certificate_available_date && newattrs.certificates_display_behavior != CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE) + || (!newattrs.certificate_available_date && newattrs.certificates_display_behavior == CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE) + ){ + errors.certificates_display_behavior = StringUtils.interpolate( + gettext( + "The certificates display behavior must be {valid_option} if certificate available date is set." + ), + { + valid_option: CERTIFICATES_DISPLAY_BEHAVIOR_OPTIONS.END_WITH_DATE + } + ); + } + } + if (newattrs.intro_video && newattrs.intro_video !== this.get('intro_video')) { if (this._videokey_illegal_chars.exec(newattrs.intro_video)) { errors.intro_video = gettext('Key should only contain letters, numbers, _, or -'); diff --git a/cms/static/js/spec/views/settings/main_spec.js b/cms/static/js/spec/views/settings/main_spec.js index f378879fb1..ce134ff0c4 100644 --- a/cms/static/js/spec/views/settings/main_spec.js +++ b/cms/static/js/spec/views/settings/main_spec.js @@ -21,7 +21,8 @@ define([ end_date: '2014-11-05T20:00:00Z', enrollment_start: '2014-10-00T00:00:00Z', enrollment_end: '2014-11-05T00:00:00Z', - certificate_available_date: '2014-11-05T20:00:00Z', + certificates_display_behavior: 'end', + certificate_available_date: null, org: '', course_id: '', run: '', @@ -351,6 +352,8 @@ define([ }); it('should disallow save with a certificate available date before end date', function() { this.model.showCertificateAvailableDate = true; + $('#certificates-display-behavior').val('end_with_date').trigger('change'); + $('#certificate-available-date').val('01/01/2020').trigger('change'); $('#course-end-date').val('01/01/2030').trigger('change'); expect(this.view.$('.message-error')).toExist(); }); diff --git a/cms/static/js/views/settings/main.js b/cms/static/js/views/settings/main.js index 03ddc9ace1..818abbdc28 100644 --- a/cms/static/js/views/settings/main.js +++ b/cms/static/js/views/settings/main.js @@ -33,7 +33,9 @@ define(['js/views/validation', 'codemirror', 'underscore', 'jquery', 'jquery.ui' this.$el.find('#course-organization').val(this.model.get('org')); this.$el.find('#course-number').val(this.model.get('course_id')); this.$el.find('#course-name').val(this.model.get('run')); - this.$el.find('.set-date').datepicker({dateFormat: 'm/d/yy'}); + this.$el.find('.set-date').datepicker({ dateFormat: 'm/d/yy' }); + this.$el.find("#certificates-display-behavior").val(this.model.get("certificates_display_behavior")); + this.updateCertificatesDisplayBehavior(); // Avoid showing broken image on mistyped/nonexistent image this.$el.find('img').error(function() { @@ -111,6 +113,8 @@ define(['js/views/validation', 'codemirror', 'underscore', 'jquery', 'jquery.ui' } else this.$el.find('.remove-course-introduction-video').hide(); this.$el.find('#' + this.fieldToSelectorMap.effort).val(this.model.get('effort')); + this.$el.find("#" + this.fieldToSelectorMap.certificates_display_behavior).val(this.model.get('certificates_display_behavior')); + this.updateCertificatesDisplayBehavior(); var courseImageURL = this.model.get('course_image_asset_path'); this.$el.find('#course-image-url').val(courseImageURL); @@ -165,6 +169,7 @@ define(['js/views/validation', 'codemirror', 'underscore', 'jquery', 'jquery.ui' enrollment_end: 'enrollment-end', upgrade_deadline: 'upgrade-deadline', certificate_available_date: 'certificate-available', + certificates_display_behavior: 'certificates-display-behavior', overview: 'course-overview', title: 'course-title', subtitle: 'course-subtitle', @@ -302,6 +307,10 @@ define(['js/views/validation', 'codemirror', 'underscore', 'jquery', 'jquery.ui' case 'course-pace-instructor-paced': this.model.set('self_paced', JSON.parse(event.currentTarget.value)); break; + case 'certificates-display-behavior': + this.setField(event); + this.updateCertificatesDisplayBehavior(); + break; case 'course-language': case 'course-effort': case 'course-title': @@ -375,6 +384,29 @@ define(['js/views/validation', 'codemirror', 'underscore', 'jquery', 'jquery.ui' } }, + updateCertificatesDisplayBehavior: function() { + /* + Hides and clears the certificate available date field if a display behavior that doesn't use it is + chosen. Because we are clearing it, toggling back to "end_with_date" will require re-entering the date + */ + if (!this.useV2CertDisplaySettings){ + return; + } + let showDatepicker = this.model.get("certificates_display_behavior") == "end_with_date"; + let datepicker = this.$el.find('#certificate-available-date'); + let certificateAvailableDateField = this.$el.find('#field-certificate-available-date'); + + if (showDatepicker) { + datepicker.prop('disabled', false); + certificateAvailableDateField.removeClass("hidden"); + } else { + datepicker.prop('disabled', true); + datepicker.val(null); + this.clearValidationErrors(); + this.setAndValidate("certificate_available_date", null) + certificateAvailableDateField.addClass("hidden"); + } + }, revertView: function() { // Make sure that the CodeMirror instance has the correct // data from its corresponding textarea diff --git a/cms/static/sass/views/_settings.scss b/cms/static/sass/views/_settings.scss index de7c156cd9..10d2ccc327 100644 --- a/cms/static/sass/views/_settings.scss +++ b/cms/static/sass/views/_settings.scss @@ -157,6 +157,29 @@ color: $red; } + .collapsible { + .collapsible-trigger{ + color: theme-color("primary"); + &:hover{ + cursor: pointer; + } + } + + .collapsible-content{ + font-size: 12px; + padding-top: ($baseline/2); + .collapsible-description-heading { + font-weight: bold; + margin-top: $baseline; + } + + &.collapsed{ + height: 0; + overflow: hidden; + } + } + } + // buttons .remove-item { @include white-button; @@ -222,10 +245,14 @@ } } + &.hidden { + display: none; + } + label, input, textarea { - display: block; + display: inline-block; } label { @@ -242,7 +269,8 @@ } input, - textarea { + textarea, + select { @extend %t-copy-base; @include placeholder($gray-l4); @@ -286,6 +314,11 @@ display: inline-block; } } + + .datepicker-icon { + display: inline; + margin-left: -($baseline * 1.5); + } } .field-group { @@ -478,7 +511,7 @@ .field { @include float(left); - width: flex-grid(3, 9); + width: flex-grid(4, 9); margin-bottom: ($baseline/4); margin-right: flex-gutter(); } diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 22832c4adc..73aec235c5 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -42,7 +42,8 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}' "${details_url | n, js_escaped_string}", ${show_min_grade_warning | n, dump_js_escaped_json}, ${can_show_certificate_available_date_field(context_course) | n, dump_js_escaped_json}, - "${upgrade_deadline | n, js_escaped_string}" + "${upgrade_deadline | n, js_escaped_string}", + ${settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS") | n, dump_js_escaped_json} ); }); @@ -223,6 +224,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}'
+ ${_("First day the course begins")}
@@ -237,6 +239,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}'
+ ${_("Last day your course is active")}
@@ -248,13 +251,61 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}' + <% + use_v2_cert_display_settings = settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False) + %> % if can_show_certificate_available_date_field(context_course):
  1. -
    +
    + + % if use_v2_cert_display_settings: + + % else: + + % endif + ${_("Certificates are awarded at the end of a course run")} + + % if use_v2_cert_display_settings: + +
    + + +
    + % endif +
    + + % if use_v2_cert_display_settings: +
@@ -265,6 +316,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}'
+ ${_("First day students can enroll")}
@@ -282,6 +334,7 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url | n, js_escaped_string}'
+ ${_("Last day students can enroll.")} % if not enrollment_end_editable: diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 44a4631e58..4c8a723b08 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -51,6 +51,7 @@ from openedx.core.djangoapps.certificates.api import auto_certificate_generation from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming.helpers import get_themes from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect +from xmodule.data import CertificatesDisplayBehaviors # Enumeration of per-course verification statuses # we display on the student dashboard. @@ -513,15 +514,11 @@ def _cert_info(user, enrollment, cert_status): status = template_state.get(cert_status['status'], default_status) is_hidden_status = status in ('processing', 'generating', 'notpassing', 'auditing') - if ( - not certificates_viewable_for_course(course_overview) and - CertificateStatuses.is_passing_status(status) and - course_overview.certificate_available_date - ): + if _is_certificate_earned_but_not_available(course_overview, status): status = certificate_earned_but_not_available_status if ( - course_overview.certificates_display_behavior == 'early_no_info' and + course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO and is_hidden_status ): return default_info @@ -610,6 +607,36 @@ def _cert_info(user, enrollment, cert_status): return status_dict +def _is_certificate_earned_but_not_available(course_overview, status): + """ + Returns True if the user is passing the course, but the certificate is not visible due to display behavior or + available date + + Params: + course_overview (CourseOverview): The course to check we're checking the certificate for + status (str): The certificate status the user has in the course + + Returns: + (bool): True if the user earned the certificate but it's hidden due to display behavior, else False + + """ + if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"): + return ( + not certificates_viewable_for_course(course_overview) + and CertificateStatuses.is_passing_status(status) + and course_overview.certificates_display_behavior in ( + CertificatesDisplayBehaviors.END_WITH_DATE, + CertificatesDisplayBehaviors.END + ) + ) + else: + return ( + not certificates_viewable_for_course(course_overview) and + CertificateStatuses.is_passing_status(status) and + course_overview.certificate_available_date + ) + + def process_survey_link(survey_link, user): """ If {UNIQUE_ID} appears in the link, replace it with a unique id for the user. diff --git a/common/djangoapps/student/tests/test_certificates.py b/common/djangoapps/student/tests/test_certificates.py index 6f01490386..f83d76dff1 100644 --- a/common/djangoapps/student/tests/test_certificates.py +++ b/common/djangoapps/student/tests/test_certificates.py @@ -22,6 +22,7 @@ from lms.djangoapps.certificates.tests.factories import ( from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.data import CertificatesDisplayBehaviors # pylint: disable=no-member @@ -40,7 +41,7 @@ class CertificateDisplayTestBase(SharedModuleStoreTestCase): def setUpClass(cls): super().setUpClass() cls.course = CourseFactory() - cls.course.certificates_display_behavior = "early_with_info" + cls.course.certificates_display_behavior = CertificatesDisplayBehaviors.EARLY_NO_INFO with cls.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, cls.course.id): cls.store.update_item(cls.course, cls.USERNAME) @@ -116,40 +117,54 @@ class CertificateDashboardMessageDisplayTest(CertificateDisplayTestBase): @classmethod def setUpClass(cls): super().setUpClass() - cls.course.certificates_display_behavior = "end" + cls.course.certificates_display_behavior = CertificatesDisplayBehaviors.END_WITH_DATE cls.course.save() cls.store.update_item(cls.course, cls.USERNAME) - def _check_message(self, certificate_available_date): # lint-amnesty, pylint: disable=missing-function-docstring + def _check_message(self, visible_date): # lint-amnesty, pylint: disable=missing-function-docstring response = self.client.get(reverse('dashboard')) test_message = 'Your grade and certificate will be ready after' - if certificate_available_date is None: + + is_past = visible_date < datetime.datetime.now(UTC) + + if is_past: self.assertNotContains(response, test_message) self.assertNotContains(response, "View Test_Certificate") - elif datetime.datetime.now(UTC) < certificate_available_date: - self.assertContains(response, test_message) - self.assertNotContains(response, "View Test_Certificate") - else: self._check_can_download_certificate() - @ddt.data(True, False, None) - def test_certificate_available_date(self, past_certificate_available_date): + else: + self.assertContains(response, test_message) + self.assertNotContains(response, "View Test_Certificate") + + @ddt.data( + (CertificatesDisplayBehaviors.END, True), + (CertificatesDisplayBehaviors.END, False), + (CertificatesDisplayBehaviors.END_WITH_DATE, True), + (CertificatesDisplayBehaviors.END_WITH_DATE, False) + ) + @ddt.unpack + def test_certificate_available_date(self, certificates_display_behavior, past_date): cert = self._create_certificate('verified') cert.status = CertificateStatuses.downloadable cert.save() - if past_certificate_available_date is None: - certificate_available_date = None - elif past_certificate_available_date: - certificate_available_date = PAST_DATE - elif not past_certificate_available_date: - certificate_available_date = FUTURE_DATE + self.course.certificates_display_behavior = certificates_display_behavior + + if certificates_display_behavior == CertificatesDisplayBehaviors.END: + if past_date: + self.course.end = PAST_DATE + else: + self.course.end = FUTURE_DATE + if certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE: + if past_date: + self.course.certificate_available_date = PAST_DATE + else: + self.course.certificate_available_date = FUTURE_DATE - self.course.certificate_available_date = certificate_available_date self.course.save() self.store.update_item(self.course, self.USERNAME) - self._check_message(certificate_available_date) + self._check_message(PAST_DATE if past_date else FUTURE_DATE) @ddt.ddt diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index c60919a410..dbaeb1f6dc 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -40,6 +40,7 @@ from openedx.core.djangoapps.content.course_overviews.tests.factories import Cou from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration_context from openedx.features.course_duration_limits.models import CourseDurationLimitConfig from openedx.features.course_experience.tests.views.helpers import add_course_mode +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -230,6 +231,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, id=course_key, end_date=THREE_YEARS_AGO, certificate_available_date=TOMORROW, + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE, lowest_passing_grade=0.3 ) CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=CourseMode.VERIFIED) @@ -245,6 +247,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, id=course_key, end_date=TOMORROW, certificate_available_date=TOMORROW, + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE, lowest_passing_grade=0.3 ) CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=CourseMode.VERIFIED) @@ -260,6 +263,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, id=course_key, end_date=ONE_WEEK_AGO, certificate_available_date=now(), + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE, lowest_passing_grade=0.3 ) CourseEnrollmentFactory(course_id=course.id, user=self.user, mode=CourseMode.VERIFIED) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index d0cf19e097..3870bd393c 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -49,6 +49,8 @@ from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms from xmodule.modulestore.tests.django_utils import ModuleStoreEnum, ModuleStoreTestCase, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls +from xmodule.data import CertificatesDisplayBehaviors + log = logging.getLogger(__name__) @@ -75,7 +77,8 @@ class CourseEndingTest(ModuleStoreTestCase): survey_url = "http://a_survey.com" course = CourseOverviewFactory.create( end_of_course_survey_url=survey_url, - certificates_display_behavior='end', + certificates_display_behavior=CertificatesDisplayBehaviors.END, + end=datetime.now(pytz.UTC) - timedelta(days=2) ) cert = GeneratedCertificateFactory.create( user=user, @@ -152,6 +155,7 @@ class CourseEndingTest(ModuleStoreTestCase): ) enrollment3 = CourseEnrollmentFactory(user=user, course_id=course3.id, mode=CourseMode.VERIFIED) # test when the display is unavailable or notpassing, we get the correct results out + course2.certificates_display_behavior = CertificatesDisplayBehaviors.EARLY_NO_INFO cert_status = {'status': 'unavailable', 'mode': 'honor', 'uuid': None} assert _cert_info(user, enrollment3, cert_status) == {'status': 'processing', 'show_survey_button': False, 'can_unenroll': True} @@ -186,7 +190,8 @@ class CourseEndingTest(ModuleStoreTestCase): survey_url = "http://a_survey.com" course = CourseOverviewFactory.create( end_of_course_survey_url=survey_url, - certificates_display_behavior='end', + certificates_display_behavior=CertificatesDisplayBehaviors.END, + end=datetime.now(pytz.UTC) - timedelta(days=2), ) enrollment = CourseEnrollmentFactory(user=user, course_id=course.id, mode=CourseMode.VERIFIED) @@ -212,7 +217,8 @@ class CourseEndingTest(ModuleStoreTestCase): survey_url = "http://a_survey.com" course = CourseOverviewFactory.create( end_of_course_survey_url=survey_url, - certificates_display_behavior='end', + certificates_display_behavior=CertificatesDisplayBehaviors.END, + end=datetime.now(pytz.UTC) - timedelta(days=2), ) cert_status = {'status': 'generating', 'mode': 'honor', 'uuid': None} enrollment = CourseEnrollmentFactory(user=user, course_id=course.id, mode=CourseMode.VERIFIED) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 1e23efcef1..720b4859cf 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -22,6 +22,7 @@ from openedx.core.lib.license import LicenseMixin from openedx.core.lib.teams_config import TeamsConfig # lint-amnesty, pylint: disable=unused-import from xmodule import course_metadata_utils from xmodule.course_metadata_utils import DEFAULT_GRADING_POLICY, DEFAULT_START_DATE +from xmodule.data import CertificatesDisplayBehaviors from xmodule.graders import grader_from_conf from xmodule.seq_module import SequenceBlock from xmodule.tabs import CourseTabList, InvalidTabsException @@ -555,15 +556,11 @@ class CourseFields: # lint-amnesty, pylint: disable=missing-class-docstring certificates_display_behavior = String( display_name=_("Certificates Display Behavior"), help=_( - "Enter end, early_with_info, or early_no_info. After certificate generation, students who passed see a " - "link to their certificates on the dashboard and students who did not pass see information about the " - "grading configuration. The default is end, which displays this certificate information to all students " - "after the course end date. To display this certificate information to all students as soon as " - "certificates are generated, enter early_with_info. To display only the links to passing students as " - "soon as certificates are generated, enter early_no_info." + "This field, together with certificate_available_date will determine when a " + "user can see their certificate for the course" ), scope=Scope.settings, - default="end" + default=CertificatesDisplayBehaviors.END, ) course_image = String( display_name=_("Course About Page Image"), @@ -1061,7 +1058,8 @@ class CourseBlock( except InvalidTabsException as err: raise type(err)(f'{str(err)} For course: {str(self.id)}') # lint-amnesty, pylint: disable=line-too-long - self.set_default_certificate_available_date() + if not settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"): + self.set_default_certificate_available_date() def set_grading_policy(self, course_policy): """ diff --git a/common/lib/xmodule/xmodule/data.py b/common/lib/xmodule/xmodule/data.py new file mode 100644 index 0000000000..b7622547d7 --- /dev/null +++ b/common/lib/xmodule/xmodule/data.py @@ -0,0 +1,25 @@ +""" +Public data structures for this app. + +See OEP-49 for details +""" +from enum import Enum + + +class CertificatesDisplayBehaviors(str, Enum): + """ + Options for the certificates_display_behavior field of a course + + end: Certificates are available at the end of the course + end_with_date: Certificates are available after the certificate_available_date (post course end) + early_no_info: Certificates are available immediately after earning them. + + Only in affect for instructor based courses. + """ + END = "end" + END_WITH_DATE = "end_with_date" + EARLY_NO_INFO = "early_no_info" + + @classmethod + def includes_value(cls, value): + return value in set(item.value for item in cls) diff --git a/common/lib/xmodule/xmodule/tests/test_course_module.py b/common/lib/xmodule/xmodule/tests/test_course_module.py index ff1982e79b..f4c8dc8e49 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_module.py +++ b/common/lib/xmodule/xmodule/tests/test_course_module.py @@ -18,6 +18,7 @@ from xblock.runtime import DictKeyValueStore, KvsFieldData from openedx.core.lib.teams_config import TeamsConfig, DEFAULT_COURSE_RUN_MAX_TEAM_SIZE import xmodule.course_module +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.xml import ImportSystem, XMLModuleStore from xmodule.modulestore.exceptions import InvalidProctoringProvider @@ -105,36 +106,41 @@ class HasEndedMayCertifyTestCase(unittest.TestCase): super().setUp() system = DummySystem(load_error_modules=True) # lint-amnesty, pylint: disable=unused-variable - #sample_xml = """ - # - # - # Two houses, ... - # - # - #""".format(org=ORG, course=COURSE) + past_end = (datetime.now() - timedelta(days=12)).strftime("%Y-%m-%dT%H:%M:00") future_end = (datetime.now() + timedelta(days=12)).strftime("%Y-%m-%dT%H:%M:00") - self.past_show_certs = get_dummy_course("2012-01-01T12:00", end=past_end, certs='early_with_info') - self.past_show_certs_no_info = get_dummy_course("2012-01-01T12:00", end=past_end, certs='early_no_info') - self.past_noshow_certs = get_dummy_course("2012-01-01T12:00", end=past_end, certs='end') - self.future_show_certs = get_dummy_course("2012-01-01T12:00", end=future_end, certs='early_with_info') - self.future_show_certs_no_info = get_dummy_course("2012-01-01T12:00", end=future_end, certs='early_no_info') - self.future_noshow_certs = get_dummy_course("2012-01-01T12:00", end=future_end, certs='end') - #self.past_show_certs = system.process_xml(sample_xml.format(end=past_end, cert=True)) - #self.past_noshow_certs = system.process_xml(sample_xml.format(end=past_end, cert=False)) - #self.future_show_certs = system.process_xml(sample_xml.format(end=future_end, cert=True)) - #self.future_noshow_certs = system.process_xml(sample_xml.format(end=future_end, cert=False)) + self.past_show_certs = get_dummy_course( + "2012-01-01T12:00", + end=past_end, + certs=CertificatesDisplayBehaviors.EARLY_NO_INFO + ) + self.past_show_certs_no_info = get_dummy_course( + "2012-01-01T12:00", + end=past_end, + certs=CertificatesDisplayBehaviors.EARLY_NO_INFO + ) + self.past_noshow_certs = get_dummy_course( + "2012-01-01T12:00", + end=past_end, + certs=CertificatesDisplayBehaviors.END + ) + + self.future_show_certs_no_info = get_dummy_course( + "2012-01-01T12:00", + end=future_end, + certs=CertificatesDisplayBehaviors.EARLY_NO_INFO + ) + self.future_noshow_certs = get_dummy_course( + "2012-01-01T12:00", + end=future_end, + certs=CertificatesDisplayBehaviors.END + ) def test_has_ended(self): """Check that has_ended correctly tells us when a course is over.""" assert self.past_show_certs.has_ended() assert self.past_show_certs_no_info.has_ended() assert self.past_noshow_certs.has_ended() - assert not self.future_show_certs.has_ended() assert not self.future_show_certs_no_info.has_ended() assert not self.future_noshow_certs.has_ended() @@ -402,14 +408,6 @@ class CourseBlockTestCase(unittest.TestCase): """ assert self.course.number == COURSE - def test_set_default_certificate_available_date(self): - """ - The certificate_available_date field should default to two days - after the course end date. - """ - expected_certificate_available_date = self.course.end + timedelta(days=2) - assert expected_certificate_available_date == self.course.certificate_available_date - class ProctoringProviderTestCase(unittest.TestCase): """ diff --git a/lms/djangoapps/certificates/api.py b/lms/djangoapps/certificates/api.py index 0df1d661c5..8a41a2e1a9 100644 --- a/lms/djangoapps/certificates/api.py +++ b/lms/djangoapps/certificates/api.py @@ -9,9 +9,8 @@ certificates models or any other certificates modules. import logging -from datetime import datetime -from pytz import UTC +from django.conf import settings from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q @@ -46,6 +45,8 @@ from lms.djangoapps.certificates.utils import ( certificate_status_for_student as _certificate_status_for_student, ) from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from xmodule.data import CertificatesDisplayBehaviors log = logging.getLogger("edx.certificate") User = get_user_model() @@ -245,9 +246,18 @@ def certificate_downloadable_status(student, course_key): } course_overview = get_course_overview_or_none(course_key) + + if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"): + display_behavior_is_valid = ( + course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE + ) + else: + display_behavior_is_valid = True + if ( not certificates_viewable_for_course(course_overview) and CertificateStatuses.is_passing_status(current_status['status']) and + display_behavior_is_valid and course_overview.certificate_available_date ): response_data['earned_but_not_available'] = True @@ -576,25 +586,29 @@ def get_certificate_footer_context(): def certificates_viewable_for_course(course): """ Returns True if certificates are viewable for any student enrolled in the course, False otherwise. + + Arguments: + course (CourseOverview or course descriptor): The course to check if certificates are viewable + + Returns: + boolean: whether the certificates are viewable or not """ - if course.self_paced: - return True - if ( - course.certificates_display_behavior in ('early_with_info', 'early_no_info') - or course.certificates_show_before_end - ): - return True - if ( - course.certificate_available_date - and course.certificate_available_date <= datetime.now(UTC) - ): - return True - if ( - course.certificate_available_date is None - and course.has_ended() - ): - return True - return False + + # The CourseOverview contains validation logic on the certificates_display_behavior and certificate_available_date + # fields. Thus, we prefer to use the CourseOverview, but will fall back to the course in case the CourseOverview is + # not available. + if not isinstance(course, CourseOverview): + course_overview = get_course_overview_or_none(course.id) + if course_overview: + course = course_overview + + return _should_certificate_be_visible( + course.certificates_display_behavior, + course.certificates_show_before_end, + course.has_ended(), + course.certificate_available_date, + course.self_paced + ) def get_allowlisted_users(course_key): @@ -754,35 +768,6 @@ def get_enrolled_allowlisted_not_passing_users(course_key): ) -def should_certificate_be_visible( - certificates_display_behavior, - certificates_show_before_end, - has_ended, - certificate_available_date, - self_paced -): - """ - Returns whether it is acceptable to show the student a certificate download - link for a course, based on provided attributes of the course. - Arguments: - certificates_display_behavior (str): string describing the course's - certificate display behavior. - See CourseFields.certificates_display_behavior.help for more detail. - certificates_show_before_end (bool): whether user can download the - course's certificates before the course has ended. - has_ended (bool): Whether the course has ended. - certificate_available_date (datetime): the date the certificate is available on for the course. - self_paced (bool): Whether the course is self-paced. - """ - return _should_certificate_be_visible( - certificates_display_behavior, - certificates_show_before_end, - has_ended, - certificate_available_date, - self_paced - ) - - def certificate_info_for_user(user, course_id, grade, user_is_allowlisted, user_certificate): """ Returns the certificate info for a user for grade report. diff --git a/lms/djangoapps/certificates/docs/decisions/005-cert-display-settings.rst b/lms/djangoapps/certificates/docs/decisions/005-cert-display-settings.rst new file mode 100644 index 0000000000..77a0e40979 --- /dev/null +++ b/lms/djangoapps/certificates/docs/decisions/005-cert-display-settings.rst @@ -0,0 +1,98 @@ +Certificate Display Settings +============================ + +Status +------ +Accepted + +Background +---------- +Courses have three settings that directly affect certificates visibility. These three settings were all built separately to handle specific use cases and haven't interacted in a quickly understood or consistent manner. These settings are: + +``certificate_display_date`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +A date at which a certificate can be shown to the learner. Defaulted to two days after course end. + +``certificates_display_behavior`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +A string that determines when the learner is able to see details about their certificate. This had no option validation and strings were all hardcoded throughout the platform. The three options were ``end``, ``early_no_info``, ``early_with_info``. + +``certificates_show_before_end`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +A string boolean that determines if the leaner is able to see their certificate before the course ends. It is deprecated but still used to determine if the learner should be shown the certificate + +Decision +-------- +We are choosing to re-imagine the first two of these existing settings, and leaving the third one deprecated until future removal. ``certificates_display_behavior`` has been updated to use three constants: ``end``, ``end_with_date``, and ``early_no_info`` that are selectable via a more user friendly worded dropdown in Studio. These settings are only used for instructor-paced courses. + +``end`` (Studio: "End date of course"): This will be the default and course certificates will become available to the learner once the course end date is reached. + +``early_no_info`` (Studio: "Immediately upon passing"): This will cause the certificates to become available to the learner immediately upon the learner achieving a passing grade in the course. + +``end_with_date`` (Studio: "A date after the course end date"): This will not display the learner's certificates until the date set in ``certificate_available_date``. If this option is not chosen, ``certificate_available_date`` will not be visible in Studio, the value will set to None, and the date will not effect any learner certificates. + +Mongo Data +~~~~~~~~~~ +Since Mongo/modulestore data can be hard to trust (due to the fact that course teams can upload data via XML) and there is no way to complete a migration similar to what we do in Django/RDBMS when we are requiring all data to fit a new paradigm, we had to add a new translation layer when reading from modulestore that validates the fields. Now, when a CourseOverview model or CourseDetails object is built with modulestore data, the ``certificate_available_date`` and ``certificates_display_behavior`` fields will be translated to valid combinations. This **does not** write the updated data back into modulestore. It simply takes possibly bad modulestore data and presents the rest of the codebase functional data. + +The rules are thus: +1. If ``certificates_display_behavior`` is set to ``early_no_info`` the ``certificate_available_date`` will be ``None``. +2. If the ``certificate_available_date`` is set and the ``certificates_display_behavior`` isn't ``early_no_info``, the ``certificates_display_behavior`` will be changed to ``end_with_date`` +3. If neither of those are true, the ``certificate_available_date`` will be set to ``None`` and the ``certificates_display_behavior`` will be set to ``end`` + +This results in the following translation table: + +For brevity: + +CAD = ``certificate_available_date`` + +CDB = ``certificates_display_behavior`` + + +.. list-table:: Translation Table + :header-rows: 1 + + * - CAD in modulestore + - CDB in modulestore + - Validated CAD + - Validated CDB + + * - + - "end" + - + - "end_with_date" + + * - + - "end_with_date" + - + - "end_with_date" + + * - + - "early_no_info" + - null + - "early_no_info" + + * - + - + - + - "end_with_date" + + * - null + - "end" + - null + - "end" + + * - null + - "end_with_date" + - null + - "end" + + * - null + - "early_no_info" + - null + - "early_no_info" + + * - null + - + - null + - "end" diff --git a/lms/djangoapps/certificates/tests/test_api.py b/lms/djangoapps/certificates/tests/test_api.py index 65858492af..45720bc650 100644 --- a/lms/djangoapps/certificates/tests/test_api.py +++ b/lms/djangoapps/certificates/tests/test_api.py @@ -18,6 +18,7 @@ from freezegun import freeze_time from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator from testfixtures import LogCapture +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -210,7 +211,8 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes ) @ddt.unpack @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True}) - def test_cert_api_return(self, self_paced, cert_avail_delta, cert_downloadable_status, earned_but_not_available): + @patch.dict(settings.FEATURES, {'ENABLE_V2_CERT_DISPLAY_SETTINGS': False}) + def test_cert_api_return_v1(self, self_paced, cert_avail_delta, cert_downloadable_status, earned_but_not_available): """ Test 'downloadable status' """ @@ -225,6 +227,41 @@ class CertificateDownloadableStatusTests(WebCertificateTestMixin, ModuleStoreTes assert downloadable_status['is_downloadable'] == cert_downloadable_status assert downloadable_status.get('earned_but_not_available') == earned_but_not_available + @ddt.data( + (True, timedelta(days=2), CertificatesDisplayBehaviors.END_WITH_DATE, True, None), + (False, -timedelta(days=2), CertificatesDisplayBehaviors.EARLY_NO_INFO, True, None), + (False, timedelta(days=2), CertificatesDisplayBehaviors.EARLY_NO_INFO, True, None), + (False, -timedelta(days=2), CertificatesDisplayBehaviors.END, True, None), + (False, timedelta(days=2), CertificatesDisplayBehaviors.END, False, True), + (False, -timedelta(days=2), CertificatesDisplayBehaviors.END_WITH_DATE, True, None), + (False, timedelta(days=2), CertificatesDisplayBehaviors.END_WITH_DATE, False, True), + ) + @ddt.unpack + @patch.dict(settings.FEATURES, {'CERTIFICATES_HTML_VIEW': True}) + @patch.dict(settings.FEATURES, {'ENABLE_V2_CERT_DISPLAY_SETTINGS': True}) + def test_cert_api_return_v2( + self, + self_paced, + cert_avail_delta, + certificates_display_behavior, + cert_downloadable_status, + earned_but_not_available + ): + """ + Test 'downloadable status' + """ + cert_avail_date = datetime.now(pytz.UTC) + cert_avail_delta + self.course.self_paced = self_paced + self.course.certificate_available_date = cert_avail_date + self.course.certificates_display_behavior = certificates_display_behavior + self.course.save() + + self._setup_course_certificate() + + downloadable_status = certificate_downloadable_status(self.student, self.course.id) + assert downloadable_status['is_downloadable'] == cert_downloadable_status + assert downloadable_status.get('earned_but_not_available') == earned_but_not_available + @ddt.ddt class CertificateIsInvalid(WebCertificateTestMixin, ModuleStoreTestCase): diff --git a/lms/djangoapps/certificates/tests/test_utils.py b/lms/djangoapps/certificates/tests/test_utils.py index 94c9ede054..b22be864db 100644 --- a/lms/djangoapps/certificates/tests/test_utils.py +++ b/lms/djangoapps/certificates/tests/test_utils.py @@ -5,11 +5,13 @@ from datetime import datetime, timedelta from unittest.mock import patch import ddt +from django.conf import settings from django.test import TestCase from pytz import utc from lms.djangoapps.certificates.utils import has_html_certificates_enabled, should_certificate_be_visible from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from xmodule.data import CertificatesDisplayBehaviors _TODAY = datetime.now(utc) _LAST_MONTH = _TODAY - timedelta(days=30) @@ -54,19 +56,64 @@ class CertificateUtilityTests(TestCase): assert not has_html_certificates_enabled(self.course_overview) @ddt.data( - ('early_with_info', True, True, _LAST_MONTH, False, True), - ('early_no_info', False, False, _LAST_MONTH, False, True), - ('end', True, False, _LAST_MONTH, False, True), - ('end', False, True, _LAST_MONTH, False, True), - ('end', False, False, _NEXT_WEEK, False, False), - ('end', False, False, _LAST_WEEK, False, True), - ('end', False, False, None, False, False), - ('early_with_info', False, False, None, False, True), - ('end', False, False, _NEXT_WEEK, False, False), - ('end', False, False, _NEXT_WEEK, True, True), + # Test certificates_show_before_end + (CertificatesDisplayBehaviors.EARLY_NO_INFO, True, False, _LAST_MONTH, False, True), + (CertificatesDisplayBehaviors.END, True, False, _LAST_MONTH, False, True), + (CertificatesDisplayBehaviors.END_WITH_DATE, True, False, _NEXT_WEEK, False, True), + + # Test that EARLY_NO_INFO + (CertificatesDisplayBehaviors.EARLY_NO_INFO, True, True, _LAST_MONTH, False, True), + (CertificatesDisplayBehaviors.EARLY_NO_INFO, False, False, _LAST_MONTH, False, True), + + # Test END_WITH_DATE + (CertificatesDisplayBehaviors.END_WITH_DATE, False, False, _LAST_MONTH, False, True), + (CertificatesDisplayBehaviors.END_WITH_DATE, False, False, _LAST_WEEK, False, True), + (CertificatesDisplayBehaviors.END_WITH_DATE, False, False, _NEXT_WEEK, False, False), + (CertificatesDisplayBehaviors.END_WITH_DATE, False, False, None, False, False), + + # Test END + (CertificatesDisplayBehaviors.END, False, False, _LAST_MONTH, False, False), + (CertificatesDisplayBehaviors.END, False, True, _LAST_MONTH, False, True), + + # Test self_paced + (CertificatesDisplayBehaviors.END, False, False, _LAST_MONTH, False, False), + (CertificatesDisplayBehaviors.END, False, False, _LAST_MONTH, True, True), ) @ddt.unpack - def test_should_certificate_be_visible( + @patch.dict(settings.FEATURES, ENABLE_V2_CERT_DISPLAY_SETTINGS=True) + def test_should_certificate_be_visible_v2( + self, + certificates_display_behavior, + certificates_show_before_end, + has_ended, + certificate_available_date, + self_paced, + expected_value + ): + """Test whether the certificate should be visible to user given multiple usecases""" + assert should_certificate_be_visible( + certificates_display_behavior, + certificates_show_before_end, + has_ended, + certificate_available_date, + self_paced + ) == expected_value + + @ddt.data( + ('early_with_info', True, True, _LAST_MONTH, False, True), + ('early_no_info', False, False, _LAST_MONTH, False, True), + ('end', True, False, _LAST_MONTH, False, True), + ('end', False, True, _LAST_MONTH, False, True), + ('end', False, False, _NEXT_WEEK, False, False), + ('end', False, False, _LAST_WEEK, False, True), + ('end', False, False, None, False, False), + ('early_with_info', False, False, None, False, True), + ('end', False, False, _NEXT_WEEK, False, False), + ('end', False, False, _NEXT_WEEK, True, True), + ) + @ddt.unpack + @patch.dict(settings.FEATURES, ENABLE_V2_CERT_DISPLAY_SETTINGS=False) + def test_should_certificate_be_visible_v1( self, certificates_display_behavior, certificates_show_before_end, diff --git a/lms/djangoapps/certificates/tests/test_views.py b/lms/djangoapps/certificates/tests/test_views.py index 062d252d62..80e8fb95fc 100644 --- a/lms/djangoapps/certificates/tests/test_views.py +++ b/lms/djangoapps/certificates/tests/test_views.py @@ -16,6 +16,7 @@ from lms.djangoapps.certificates.models import ( from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.certificates.utils import get_certificate_url from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -60,7 +61,8 @@ class CertificatesViewsSiteTests(ModuleStoreTestCase): org='testorg', number='run1', display_name='refundable course', - certificate_available_date=datetime.datetime.today() - datetime.timedelta(days=1) + certificate_available_date=datetime.datetime.today() - datetime.timedelta(days=1), + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE ) self.course.cert_html_view_enabled = True self.course.save() diff --git a/lms/djangoapps/certificates/tests/test_webview_views.py b/lms/djangoapps/certificates/tests/test_webview_views.py index 9d1c1df083..a389e297a9 100644 --- a/lms/djangoapps/certificates/tests/test_webview_views.py +++ b/lms/djangoapps/certificates/tests/test_webview_views.py @@ -50,6 +50,7 @@ from openedx.core.djangoapps.site_configuration.tests.test_util import ( from openedx.core.djangolib.js_utils import js_escaped_string from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from openedx.core.lib.tests.assertions.events import assert_event_matches +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -81,6 +82,7 @@ class CommonCertificatesTestCase(ModuleStoreTestCase): number='run1', display_name='refundable course', certificate_available_date=datetime.datetime.today() - datetime.timedelta(days=1), + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE ) self.course_id = self.course.location.course_key self.user = UserFactory.create( diff --git a/lms/djangoapps/certificates/utils.py b/lms/djangoapps/certificates/utils.py index 70a69381df..da3fd0db45 100644 --- a/lms/djangoapps/certificates/utils.py +++ b/lms/djangoapps/certificates/utils.py @@ -13,6 +13,7 @@ from pytz import utc from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import GeneratedCertificate from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none +from xmodule.data import CertificatesDisplayBehaviors log = logging.getLogger(__name__) @@ -150,15 +151,30 @@ def should_certificate_be_visible( certificate_available_date (datetime): the date the certificate is available on for the course. self_paced (bool): Whether the course is self-paced. """ - show_early = ( - certificates_display_behavior in ('early_with_info', 'early_no_info') - or certificates_show_before_end - ) - past_available_date = ( - certificate_available_date - and certificate_available_date < datetime.now(utc) - ) - ended_without_available_date = (certificate_available_date is None) and has_ended + if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"): + show_early = ( + certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO + or certificates_show_before_end + ) + past_available_date = ( + certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE + and certificate_available_date + and certificate_available_date < datetime.now(utc) + ) + ended_without_available_date = ( + certificates_display_behavior == CertificatesDisplayBehaviors.END + and has_ended + ) + else: + show_early = ( + certificates_display_behavior in ('early_with_info', 'early_no_info') + or certificates_show_before_end + ) + past_available_date = ( + certificate_available_date + and certificate_available_date < datetime.now(utc) + ) + ended_without_available_date = (certificate_available_date is None) and has_ended return any((self_paced, show_early, past_available_date, ended_without_available_date)) diff --git a/lms/djangoapps/certificates/views/webview.py b/lms/djangoapps/certificates/views/webview.py index 3e3861039b..f37b3c64e4 100644 --- a/lms/djangoapps/certificates/views/webview.py +++ b/lms/djangoapps/certificates/views/webview.py @@ -46,10 +46,12 @@ from lms.djangoapps.certificates.permissions import PREVIEW_CERTIFICATES from lms.djangoapps.certificates.utils import emit_certificate_event, get_certificate_url from openedx.core.djangoapps.catalog.api import get_course_run_details from openedx.core.djangoapps.certificates.api import display_date_for_certificate +from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none from openedx.core.djangoapps.lang_pref.api import get_closest_released_language from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.lib.courses import course_image_url from openedx.core.lib.courses import get_course_by_id +from xmodule.data import CertificatesDisplayBehaviors log = logging.getLogger(__name__) _ = translation.ugettext @@ -89,7 +91,7 @@ def get_certificate_description(mode, certificate_type, platform_name): return certificate_type_description -def _update_certificate_context(context, course, user_certificate, platform_name): +def _update_certificate_context(context, course, course_overview, user_certificate, platform_name): """ Build up the certificate web view context using the provided values (Helper method to keep the view clean) @@ -105,8 +107,15 @@ def _update_certificate_context(context, course, user_certificate, platform_name suffix=context.get('certificate_verify_url_suffix') ) + # We prefer a CourseOverview for this function because it validates and corrects certificate_available_date + # and certificates_display_behavior values. However, not all certificates are guaranteed to have a CourseOverview + # associated with them, so we fall back on the course in that case. This shouldn't cause a problem because courses + # that are missing CourseOverviews are generally old courses, and thus their display values are no longer relevant + if course_overview: + date = display_date_for_certificate(course_overview, user_certificate) + else: + date = display_date_for_certificate(course, user_certificate) # Translators: The format of the date includes the full name of the month - date = display_date_for_certificate(course, user_certificate) context['certificate_date_issued'] = strftime_localized(date, settings.CERTIFICATE_DATE_FORMAT) # Translators: This text represents the verification of the certificate @@ -322,27 +331,42 @@ def _update_context_with_user_info(context, user, user_certificate): ) -def _get_user_certificate(request, user, course_key, course, preview_mode=None): +def _get_user_certificate(request, user, course_key, course_overview, preview_mode=None): """ Retrieves user's certificate from db. Creates one in case of preview mode. Returns None if there is no certificate generated for given user otherwise returns `GeneratedCertificate` instance. + + We use the course_overview instead of the course descriptor here, so we get the certificate_available_date and + certificates_display_behavior validation logic, rather than the raw data from the course descriptor. """ user_certificate = None if preview_mode: # certificate is being previewed from studio - if request.user.has_perm(PREVIEW_CERTIFICATES, course): - if course.certificate_available_date and not course.self_paced: - modified_date = course.certificate_available_date + if request.user.has_perm(PREVIEW_CERTIFICATES, course_overview): + if not settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"): + if course_overview.certificate_available_date and not course_overview.self_paced: + modified_date = course_overview.certificate_available_date + else: + modified_date = datetime.now().date() else: - modified_date = datetime.now().date() + if ( + course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE + and course_overview.certificate_available_date + and not course_overview.self_paced + ): + modified_date = course_overview.certificate_available_date + elif course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END: + modified_date = course_overview.end + else: + modified_date = datetime.now().date() user_certificate = GeneratedCertificate( mode=preview_mode, verify_uuid=str(uuid4().hex), modified_date=modified_date, created_date=datetime.now().date(), ) - elif certificates_viewable_for_course(course): + elif certificates_viewable_for_course(course_overview): # certificate is being viewed by learner or public try: user_certificate = GeneratedCertificate.eligible_certificates.get( @@ -506,6 +530,8 @@ def render_html_view(request, course_id, certificate=None): log.info(error_str, course_id, str(exception)) return _render_invalid_certificate(request, course_id, platform_name, configuration) + course_overview = get_course_overview_or_none(course_key) + # Kick the user back to the "Invalid" screen if the feature is disabled for the course if not course.cert_html_view_enabled: log.info( @@ -516,7 +542,7 @@ def render_html_view(request, course_id, certificate=None): return _render_invalid_certificate(request, course_id, platform_name, configuration) # Load user's certificate - user_certificate = _get_user_certificate(request, user, course_key, course, preview_mode) + user_certificate = _get_user_certificate(request, user, course_key, course_overview, preview_mode) if not user_certificate: log.info( "Invalid cert: User %d does not have eligible cert for %s.", @@ -590,7 +616,7 @@ def render_html_view(request, course_id, certificate=None): _update_social_context(request, context, course, user_certificate, platform_name) # Append/Override the existing view context values with certificate specific values - _update_certificate_context(context, course, user_certificate, platform_name) + _update_certificate_context(context, course, course_overview, user_certificate, platform_name) # Append badge info _update_badge_context(context, course, user) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index a8def5309a..e5fc05696d 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -99,6 +99,7 @@ from common.djangoapps.util.tests.test_date_utils import fake_pgettext, fake_uge from common.djangoapps.util.url import reload_django_url_config from common.djangoapps.util.views import ensure_valid_course_key from xmodule.course_module import COURSE_VISIBILITY_PRIVATE, COURSE_VISIBILITY_PUBLIC, COURSE_VISIBILITY_PUBLIC_OUTLINE +from xmodule.data import CertificatesDisplayBehaviors from xmodule.graders import ShowCorrectness from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore @@ -1288,6 +1289,7 @@ class ProgressPageBaseTests(ModuleStoreTestCase): grade_cutoffs={'çü†øƒƒ': 0.75, 'Pass': 0.5}, end=datetime.now(), certificate_available_date=datetime.now(UTC), + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE, **options ) diff --git a/lms/envs/common.py b/lms/envs/common.py index dcc208bff0..e7b81e4d93 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -939,6 +939,17 @@ FEATURES = { # .. toggle_warnings: None # .. toggle_tickets: 'https://openedx.atlassian.net/browse/OSPR-5290' 'ENABLE_BULK_USER_RETIREMENT': False, + + # .. toggle_name: FEATURES['ENABLE_V2_CERT_DISPLAY_SETTINGS'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to use the reimagined certificates_display_behavior and certificate_available_date + # .. settings. Will eventually become the default. + # .. toggle_use_cases: temporary + # .. toggle_creation_date: 2021-07-26 + # .. toggle_target_removal_date: 2021-10-01 + # .. toggle_tickets: 'https://openedx.atlassian.net/browse/MICROBA-1405' + 'ENABLE_V2_CERT_DISPLAY_SETTINGS': False, } # Specifies extra XBlock fields that should available when requested via the Course Blocks API diff --git a/lms/templates/dashboard/_dashboard_certificate_information.html b/lms/templates/dashboard/_dashboard_certificate_information.html index 5c48344611..c0ded0d125 100644 --- a/lms/templates/dashboard/_dashboard_certificate_information.html +++ b/lms/templates/dashboard/_dashboard_certificate_information.html @@ -7,6 +7,7 @@ from django.utils.translation import ugettext as _ from openedx.core.djangolib.markup import HTML, Text from common.djangoapps.course_modes.models import CourseMode from lms.djangoapps.certificates.data import CertificateStatuses +from xmodule.data import CertificatesDisplayBehaviors %> <%namespace name='static' file='../static_content.html'/> @@ -39,7 +40,14 @@ else:

<% - certificate_available_date_string = course_overview.certificate_available_date.strftime('%Y-%m-%dT%H:%M:%S%z') + if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False): + certificate_available_date_string = "" + if course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE and course_overview.certificate_available_date: + certificate_available_date_string = course_overview.certificate_available_date.strftime('%Y-%m-%dT%H:%M:%S%z') + elif course_overview.certificates_display_behavior == CertificatesDisplayBehaviors.END and course_overview.end: + certificate_available_date_string = course_overview.end.strftime('%Y-%m-%dT%H:%M:%S%z') + else: + certificate_available_date_string = course_overview.certificate_available_date.strftime('%Y-%m-%dT%H:%M:%S%z') container_string = _("Your grade and certificate will be ready after {date}.") format = 'shortDate' %> diff --git a/openedx/core/djangoapps/certificates/api.py b/openedx/core/djangoapps/certificates/api.py index 68f7a326d8..5e12095a63 100644 --- a/openedx/core/djangoapps/certificates/api.py +++ b/openedx/core/djangoapps/certificates/api.py @@ -7,10 +7,13 @@ import logging from datetime import datetime from pytz import UTC +from django.conf import settings + from lms.djangoapps.certificates import api as certs_api from lms.djangoapps.certificates.data import CertificateStatuses from openedx.core.djangoapps.certificates.config import waffle from common.djangoapps.student.models import CourseEnrollment +from xmodule.data import CertificatesDisplayBehaviors log = logging.getLogger(__name__) @@ -56,7 +59,17 @@ def can_show_certificate_available_date_field(course): def _course_uses_available_date(course): - return can_show_certificate_available_date_field(course) and course.certificate_available_date + """Returns if the course has an certificate_available_date set and that it should be used""" + if settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS"): + display_behavior_is_valid = course.certificates_display_behavior == CertificatesDisplayBehaviors.END_WITH_DATE + else: + display_behavior_is_valid = True + + return ( + can_show_certificate_available_date_field(course) + and course.certificate_available_date + and display_behavior_is_valid + ) def available_date_for_certificate(course, certificate, certificate_available_date=None): @@ -64,7 +77,7 @@ def available_date_for_certificate(course, certificate, certificate_available_da Returns the available date to use with a certificate Arguments: - course (CourseOverview): The course we're checking + course (CourseOverview or course descriptor): The course we're checking certificate (GeneratedCertificate): The certificate we're getting the date for certificate_available_date (datetime): An optional date to override the from the course overview. """ @@ -74,6 +87,14 @@ def available_date_for_certificate(course, certificate, certificate_available_da def display_date_for_certificate(course, certificate): + """ + Returns the display date that a certificate should display. + + Arguments: + course (CourseOverview or course descriptor): The course we're getting the date for + Returns: + datetime.date + """ if _course_uses_available_date(course) and course.certificate_available_date < datetime.now(UTC): return course.certificate_available_date return certificate.modified_date diff --git a/openedx/core/djangoapps/certificates/tests/test_api.py b/openedx/core/djangoapps/certificates/tests/test_api.py index 1e7e933146..15c607391e 100644 --- a/openedx/core/djangoapps/certificates/tests/test_api.py +++ b/openedx/core/djangoapps/certificates/tests/test_api.py @@ -11,10 +11,11 @@ from edx_toggles.toggles import LegacyWaffleSwitch from edx_toggles.toggles.testutils import override_waffle_switch from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory from openedx.core.djangoapps.certificates import api from openedx.core.djangoapps.certificates.config import waffle as certs_waffle from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory -from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory +from xmodule.data import CertificatesDisplayBehaviors # TODO: Copied from lms.djangoapps.certificates.models, @@ -167,6 +168,7 @@ class CertificatesApiTestCase(TestCase): # With an available date set in the past, both return the available date (if configured) self.course.certificate_available_date = datetime(2017, 2, 1, tzinfo=pytz.UTC) + self.course.certificates_display_behavior = CertificatesDisplayBehaviors.END_WITH_DATE maybe_avail = self.course.certificate_available_date if uses_avail_date else self.certificate.modified_date assert maybe_avail == api.available_date_for_certificate(self.course, self.certificate) assert maybe_avail == api.display_date_for_certificate(self.course, self.certificate) diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 50ed256a3a..46e839052a 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -36,7 +36,6 @@ from xmodule.error_module import ErrorBlock from xmodule.modulestore.django import modulestore from xmodule.tabs import CourseTab - log = logging.getLogger(__name__) @@ -55,6 +54,10 @@ class CourseOverview(TimeStampedModel): course catalog (courses to enroll in) course about (meta data about the course) + When you bump the VERSION you will invalidate all existing course overviews. This + will cause a slew of modulestore reads as each course needs to be re-cached into + the course overview. + .. no_pii: """ @@ -62,7 +65,7 @@ class CourseOverview(TimeStampedModel): app_label = 'course_overviews' # IMPORTANT: Bump this whenever you modify this model and/or add a migration. - VERSION = 15 + VERSION = 16 # Cache entry versioning. version = IntegerField() @@ -210,13 +213,18 @@ class CourseOverview(TimeStampedModel): course_overview.course_image_url = course_image_url(course) course_overview.social_sharing_url = course.social_sharing_url - course_overview.certificates_display_behavior = course.certificates_display_behavior + updated_available_date, updated_display_behavior = CourseDetails.validate_certificate_settings( + course.certificate_available_date, + course.certificates_display_behavior + ) + + course_overview.certificates_display_behavior = updated_display_behavior + course_overview.certificate_available_date = updated_available_date course_overview.certificates_show_before_end = course.certificates_show_before_end course_overview.cert_html_view_enabled = course.cert_html_view_enabled course_overview.has_any_active_web_certificate = (get_active_web_certificate(course) is not None) course_overview.cert_name_short = course.cert_name_short course_overview.cert_name_long = course.cert_name_long - course_overview.certificate_available_date = course.certificate_available_date course_overview.lowest_passing_grade = lowest_passing_grade course_overview.end_of_course_survey_url = course.end_of_course_survey_url diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py index aea25829fd..66f5d39b8c 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py @@ -2,16 +2,21 @@ import datetime from unittest.mock import patch +from collections import namedtuple import pytest import ddt +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls from ..models import CourseOverview +# represents a change of a course overview field. Used to avoid confusing indicies +Change = namedtuple("Change", ["field_name", "initial_value", "changed_value"]) + @ddt.ddt class CourseOverviewSignalsTestCase(ModuleStoreTestCase): @@ -71,8 +76,11 @@ class CourseOverviewSignalsTestCase(ModuleStoreTestCase): self.store.delete_course(course.id, ModuleStoreEnum.UserID.test) CourseOverview.get_from_id(course.id) - def assert_changed_signal_sent(self, field_name, initial_value, changed_value, mock_signal): # lint-amnesty, pylint: disable=missing-function-docstring - course = CourseFactory.create(emit_signals=True, **{field_name: initial_value}) + def assert_changed_signal_sent(self, changes, mock_signal): # lint-amnesty, pylint: disable=missing-function-docstring + course = CourseFactory.create( + emit_signals=True, + **{change.field_name: change.initial_value for change in changes} + ) # changing display name doesn't fire the signal course.display_name = course.display_name + 'changed' @@ -80,18 +88,27 @@ class CourseOverviewSignalsTestCase(ModuleStoreTestCase): assert not mock_signal.called # changing the given field fires the signal - setattr(course, field_name, changed_value) + for change in changes: + setattr(course, change.field_name, change.changed_value) self.store.update_item(course, ModuleStoreEnum.UserID.test) assert mock_signal.called @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_START_DATE_CHANGED.send') def test_start_changed(self, mock_signal): - self.assert_changed_signal_sent('start', self.TODAY, self.NEXT_WEEK, mock_signal) + self.assert_changed_signal_sent([Change('start', self.TODAY, self.NEXT_WEEK)], mock_signal) @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_PACING_CHANGED.send') def test_pacing_changed(self, mock_signal): - self.assert_changed_signal_sent('self_paced', True, False, mock_signal) + self.assert_changed_signal_sent([Change('self_paced', True, False)], mock_signal) @patch('openedx.core.djangoapps.content.course_overviews.signals.COURSE_CERT_DATE_CHANGE.send_robust') def test_cert_date_changed(self, mock_signal): - self.assert_changed_signal_sent('certificate_available_date', self.TODAY, self.NEXT_WEEK, mock_signal) + changes = [ + Change("certificate_available_date", self.TODAY, self.NEXT_WEEK), + Change( + "certificates_display_behavior", + CertificatesDisplayBehaviors.END, + CertificatesDisplayBehaviors.END_WITH_DATE + ) + ] + self.assert_changed_signal_sent(changes, mock_signal) diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index 8625aa8d8f..64d2710653 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -2,7 +2,7 @@ Tests for courseware API """ import unittest -from datetime import datetime +from datetime import datetime, timedelta from urllib.parse import urlencode from typing import Optional @@ -39,6 +39,7 @@ from common.djangoapps.student.roles import CourseInstructorRole from common.djangoapps.student.tests.factories import CourseEnrollmentCelebrationFactory, UserFactory from openedx.core.djangoapps.agreements.api import create_integrity_signature from openedx.core.djangoapps.agreements.toggles import ENABLE_INTEGRITY_SIGNATURE +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import ItemFactory, ToyCourseFactory @@ -46,6 +47,8 @@ from xmodule.modulestore.tests.factories import ItemFactory, ToyCourseFactory User = get_user_model() +_NEXT_WEEK = datetime.now() + timedelta(days=7) + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class BaseCoursewareTests(SharedModuleStoreTestCase): @@ -64,6 +67,8 @@ class BaseCoursewareTests(SharedModuleStoreTestCase): enrollment_end=datetime(2028, 1, 1, 1, 1, 1), emit_signals=True, modulestore=cls.store, + certificate_available_date=_NEXT_WEEK, + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE ) cls.chapter = ItemFactory(parent=cls.course, category='chapter') cls.sequence = ItemFactory(parent=cls.chapter, category='sequential', display_name='sequence') diff --git a/openedx/core/djangoapps/models/course_details.py b/openedx/core/djangoapps/models/course_details.py index 748e90b591..b27522d47e 100644 --- a/openedx/core/djangoapps/models/course_details.py +++ b/openedx/core/djangoapps/models/course_details.py @@ -10,6 +10,7 @@ from django.conf import settings from openedx.core.djangolib.markup import HTML from openedx.core.lib.courses import course_image_url +from xmodule.data import CertificatesDisplayBehaviors from xmodule.fields import Date from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -48,6 +49,8 @@ class CourseDetails: self.end_date = None # 'end' self.enrollment_start = None self.enrollment_end = None + self.certificate_available_date = None + self.certificates_display_behavior = None self.syllabus = None # a pdf file asset self.title = "" self.subtitle = "" @@ -108,7 +111,12 @@ class CourseDetails: course_details = cls(course_key.org, course_key.course, course_key.run) course_details.start_date = course_descriptor.start course_details.end_date = course_descriptor.end - course_details.certificate_available_date = course_descriptor.certificate_available_date # lint-amnesty, pylint: disable=attribute-defined-outside-init + updated_available_date, updated_display_behavior = cls.validate_certificate_settings( + course_descriptor.certificate_available_date, + course_descriptor.certificates_display_behavior + ) + course_details.certificate_available_date = updated_available_date + course_details.certificates_display_behavior = updated_display_behavior course_details.enrollment_start = course_descriptor.enrollment_start course_details.enrollment_end = course_descriptor.enrollment_end course_details.pre_requisite_courses = course_descriptor.pre_requisite_courses @@ -244,6 +252,13 @@ class CourseDetails: dirty = True descriptor.certificate_available_date = converted + if ( + 'certificates_display_behavior' in jsondict + and jsondict['certificates_display_behavior'] != descriptor.certificates_display_behavior + ): + descriptor.certificates_display_behavior = jsondict['certificates_display_behavior'] + dirty = True + if 'course_image_name' in jsondict and jsondict['course_image_name'] != descriptor.course_image: descriptor.course_image = jsondict['course_image_name'] dirty = True @@ -338,3 +353,31 @@ class CourseDetails: 'frameborder="0" allowfullscreen="">').format(video_key) ) return result + + @classmethod + def validate_certificate_settings(cls, certificate_available_date, certificates_display_behavior): + """ + Takes the stored values for certificate_available_date and certificates_display_behavior and verifies they work + together in tandem per ADR: lms/djangoapps/certificates/docs/decisions/005-cert-display-settings.rst + + Arguments: + stored_certificate_available_date (str): certificate_available_date from the modulestore + stored_certificates_display_behavior (str): + + Returns: + tuple[str, str]: updated certificate_available_date, updated certificates_display_behavior + None + """ + # If V2 is not enable, return original values + if not settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False): + return (certificate_available_date, certificates_display_behavior) + + # "early_no_info" will always show regardless of settings + if certificates_display_behavior == CertificatesDisplayBehaviors.EARLY_NO_INFO: + return (None, CertificatesDisplayBehaviors.EARLY_NO_INFO) + + # If the date is set and "early_no_info" isn't + if certificate_available_date: + return (certificate_available_date, CertificatesDisplayBehaviors.END_WITH_DATE) + + return (None, CertificatesDisplayBehaviors.END) diff --git a/openedx/core/djangoapps/models/tests/test_course_details.py b/openedx/core/djangoapps/models/tests/test_course_details.py index fcd3f27ed5..c7593ba686 100644 --- a/openedx/core/djangoapps/models/tests/test_course_details.py +++ b/openedx/core/djangoapps/models/tests/test_course_details.py @@ -7,12 +7,18 @@ import datetime import pytest import ddt from pytz import UTC +from unittest.mock import patch + +from django.conf import settings from openedx.core.djangoapps.models.course_details import ABOUT_ATTRIBUTES, CourseDetails from xmodule.modulestore import ModuleStoreEnum +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +EXAMPLE_CERTIFICATE_AVAILABLE_DATE = datetime.date(2020, 1, 1) + @ddt.ddt class CourseDetailsTestCase(ModuleStoreTestCase): @@ -142,3 +148,82 @@ class CourseDetailsTestCase(ModuleStoreTestCase): assert CourseDetails.fetch_youtube_video_id(self.course.id) == video_value video_url = CourseDetails.fetch_video_url(self.course.id) self.assertRegex(video_url, fr'http://.*{video_value}') + + @ddt.data( + ( + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.END, + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.END_WITH_DATE + ), + ( + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.END_WITH_DATE, + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.END_WITH_DATE + ), + ( + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.EARLY_NO_INFO, + None, + CertificatesDisplayBehaviors.EARLY_NO_INFO + ), + ( + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + "invalid_option", + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.END_WITH_DATE + ), + ( + None, + CertificatesDisplayBehaviors.END, + None, + CertificatesDisplayBehaviors.END + ), + ( + None, + CertificatesDisplayBehaviors.END_WITH_DATE, + None, + CertificatesDisplayBehaviors.END + ), + ( + None, + CertificatesDisplayBehaviors.EARLY_NO_INFO, + None, + CertificatesDisplayBehaviors.EARLY_NO_INFO + ), + ( + None, + "invalid_option", + None, + CertificatesDisplayBehaviors.END + ), + ) + @ddt.unpack + @patch.dict(settings.FEATURES, ENABLE_V2_CERT_DISPLAY_SETTINGS=True) + def test_validate_certificate_settings_v2(self, stored_date, stored_behavior, expected_date, expected_behavior): + assert CourseDetails.validate_certificate_settings( + stored_date, stored_behavior + ) == (expected_date, expected_behavior) + + @ddt.data( + ( + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.END_WITH_DATE, + EXAMPLE_CERTIFICATE_AVAILABLE_DATE, + CertificatesDisplayBehaviors.END_WITH_DATE + ), + ( + None, + "invalid_option", + None, + "invalid_option" + ), + ) + @ddt.unpack + @patch.dict(settings.FEATURES, ENABLE_V2_CERT_DISPLAY_SETTINGS=False) + def test_validate_certificate_settings_v1(self, stored_date, stored_behavior, expected_date, expected_behavior): + """Test that method just returns passed in arguments if v2 is off""" + assert CourseDetails.validate_certificate_settings( + stored_date, stored_behavior + ) == (expected_date, expected_behavior) diff --git a/openedx/core/djangoapps/programs/tests/test_tasks.py b/openedx/core/djangoapps/programs/tests/test_tasks.py index 0f43629ba9..911489d03a 100644 --- a/openedx/core/djangoapps/programs/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tests/test_tasks.py @@ -30,6 +30,7 @@ from openedx.core.djangoapps.oauth_dispatch.tests.factories import ApplicationFa from openedx.core.djangoapps.programs import tasks from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory, SiteFactory from openedx.core.djangolib.testing.utils import skip_unless_lms +from xmodule.data import CertificatesDisplayBehaviors log = logging.getLogger(__name__) @@ -520,6 +521,7 @@ class AwardCourseCertificatesTestCase(CredentialsApiConfigMixin, TestCase): self.course = CourseOverviewFactory.create( self_paced=True, # Any option to allow the certificate to be viewable for the course certificate_available_date=self.available_date, + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE ) self.student = UserFactory.create(username='test-student') # Instantiate the Certificate first so that the config doesn't execute issuance diff --git a/openedx/core/djangoapps/programs/tests/test_utils.py b/openedx/core/djangoapps/programs/tests/test_utils.py index c6f9125125..4418b9b286 100644 --- a/openedx/core/djangoapps/programs/tests/test_utils.py +++ b/openedx/core/djangoapps/programs/tests/test_utils.py @@ -51,6 +51,7 @@ from openedx.core.djangoapps.site_configuration.tests.factories import SiteFacto from openedx.core.djangolib.testing.utils import skip_unless_lms from common.djangoapps.student.tests.factories import AnonymousUserFactory, CourseEnrollmentFactory, UserFactory from common.djangoapps.util.date_utils import strftime_localized +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ( ModuleStoreTestCase, SharedModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE @@ -499,7 +500,7 @@ class TestProgramProgressMeter(ModuleStoreTestCase): end=two_days_ago, self_paced=False, certificate_available_date=tomorrow, - certificates_display_behavior='end' + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE ) third_course_run_key = str(course3.id) @@ -607,6 +608,7 @@ class TestProgramProgressMeter(ModuleStoreTestCase): # 3 certs, all available, program cert in the past/now course3_overview = CourseOverview.get_from_id(course3.id) course3_overview.certificate_available_date = yesterday + course3_overview.certificates_display_behavior = CertificatesDisplayBehaviors.END_WITH_DATE course3_overview.save() meter = ProgramProgressMeter(self.site, self.user) self._assert_progress( @@ -622,7 +624,7 @@ class TestProgramProgressMeter(ModuleStoreTestCase): def test_old_course_runs(self, mock_get_programs): """ Test that old course runs may exist for a program which do not exist in LMS. - In that case, continue considering the course run to've been failed by the learner + In that case, continue considering the course run to have been failed by the learner """ course_run = CourseRunFactory.create() course = CourseFactory.create(course_runs=[course_run]) diff --git a/openedx/core/djangoapps/programs/utils.py b/openedx/core/djangoapps/programs/utils.py index da8679bdfb..5ed84f768a 100644 --- a/openedx/core/djangoapps/programs/utils.py +++ b/openedx/core/djangoapps/programs/utils.py @@ -448,13 +448,7 @@ class ProgramProgressMeter: except CourseOverview.DoesNotExist: may_certify = True else: - may_certify = certificate_api.should_certificate_be_visible( - course_overview.certificates_display_behavior, - course_overview.certificates_show_before_end, - course_overview.has_ended(), - course_overview.certificate_available_date, - course_overview.self_paced - ) + may_certify = certificate_api.certificates_viewable_for_course(course_overview) if ( CertificateStatuses.is_passing_status(certificate['status']) @@ -592,13 +586,7 @@ class ProgramDataExtender: run_mode['upgrade_url'] = None def _attach_course_run_may_certify(self, run_mode): - run_mode['may_certify'] = certificate_api.should_certificate_be_visible( - self.course_overview.certificates_display_behavior, - self.course_overview.certificates_show_before_end, - self.course_overview.has_ended(), - self.course_overview.certificate_available_date, - self.course_overview.self_paced - ) + run_mode['may_certify'] = certificate_api.certificates_viewable_for_course(self.course_overview) def _attach_course_run_is_mobile_only(self, run_mode): run_mode['is_mobile_only'] = self.mobile_only diff --git a/openedx/features/learner_profile/tests/views/test_learner_profile.py b/openedx/features/learner_profile/tests/views/test_learner_profile.py index 97d42a5cb9..1dbcc6dbda 100644 --- a/openedx/features/learner_profile/tests/views/test_learner_profile.py +++ b/openedx/features/learner_profile/tests/views/test_learner_profile.py @@ -21,6 +21,7 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin from openedx.features.learner_profile.toggles import REDIRECT_TO_PROFILE_MICROFRONTEND from openedx.features.learner_profile.views.learner_profile import learner_profile_context +from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -222,7 +223,8 @@ class LearnerProfileViewTest(SiteMixin, UrlResetMixin, ModuleStoreTestCase): """ # add new course with certificate_available_date is future date. course = CourseFactory.create( - certificate_available_date=datetime.datetime.now() + datetime.timedelta(days=5) + certificate_available_date=datetime.datetime.now() + datetime.timedelta(days=5), + certificates_display_behavior=CertificatesDisplayBehaviors.END_WITH_DATE ) cert = self._create_certificate(course_key=course.id)