From 2a7106acfb9f0112947a19cd71d19316848e21f2 Mon Sep 17 00:00:00 2001 From: Matt Tuchfarber Date: Wed, 2 Jun 2021 15:28:28 -0400 Subject: [PATCH] feat: reimagine certificate display settings The course settings `certificate_available_date` (CAD) and `certificates_display_behavior` (CDB) were previously acting indedependantly of one another. They now work in tandem. This change: - limits CDB to a dropdown - removes "early_with_info" and adds "end_with_date" - only takes CAD into account if "end_with_date" is selected - Moves CDB to the main course schedule settings page - updates CourseOverview to validate these fields and choose sane defaults if they aren't expected values Certificates will now show under the following circumstances: "Immediately upon passing" certificate_availability_date = null certificates_display_behavior = "early_no_info" "End date of course" certificate_availability_date = null certificates_display_behavior = "end" "A date after the course end date" certificate_availability_date = certificates_display_behavior = "end_with_date" --- .../models/settings/course_metadata.py | 1 + cms/static/js/factories/settings.js | 6 + .../js/models/settings/course_details.js | 81 +- .../js/spec/views/settings/main_spec.js | 5 +- cms/static/js/views/settings/main.js | 902 +++++++++--------- cms/static/sass/views/_settings.scss | 38 +- cms/templates/settings.html | 43 +- common/djangoapps/student/helpers.py | 12 +- .../student/tests/test_certificates.py | 51 +- common/djangoapps/student/tests/test_views.py | 4 + common/djangoapps/student/tests/tests.py | 13 +- .../xmodule/xmodule/course_metadata_utils.py | 13 +- common/lib/xmodule/xmodule/course_module.py | 19 +- common/lib/xmodule/xmodule/data.py | 25 + .../tests/test_course_metadata_utils.py | 33 +- .../xmodule/tests/test_course_module.py | 59 +- lms/djangoapps/certificates/api.py | 9 +- lms/djangoapps/certificates/tests/test_api.py | 21 +- .../certificates/tests/test_views.py | 4 +- .../certificates/tests/test_webview_views.py | 2 + lms/djangoapps/certificates/views/webview.py | 9 +- lms/djangoapps/courseware/tests/test_views.py | 2 + .../_dashboard_certificate_information.html | 8 +- openedx/core/djangoapps/certificates/api.py | 6 +- .../djangoapps/certificates/tests/test_api.py | 4 +- .../migrations/0026_auto_20210615_1706.py | 24 + .../content/course_overviews/models.py | 49 +- .../course_overviews/tests/test_signals.py | 30 +- .../courseware_api/tests/test_views.py | 8 +- .../core/djangoapps/models/course_details.py | 12 +- .../djangoapps/programs/tests/test_tasks.py | 2 + .../djangoapps/programs/tests/test_utils.py | 6 +- .../tests/views/test_learner_profile.py | 4 +- package-lock.json | 2 +- 34 files changed, 934 insertions(+), 573 deletions(-) create mode 100644 common/lib/xmodule/xmodule/data.py create mode 100644 openedx/core/djangoapps/content/course_overviews/migrations/0026_auto_20210615_1706.py 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/static/js/factories/settings.js b/cms/static/js/factories/settings.js index 6ed11c4916..4e399215ca 100644 --- a/cms/static/js/factories/settings.js +++ b/cms/static/js/factories/settings.js @@ -13,6 +13,12 @@ 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; diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index c0d8c506f4..22c3f462a3 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -1,7 +1,7 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js/utils/date_utils', 'edx-ui-toolkit/js/utils/string-utils' ], - function(Backbone, _, gettext, ValidationHelpers, DateUtils, StringUtils) { + function (Backbone, _, gettext, ValidationHelpers, DateUtils, StringUtils) { 'use strict'; var CourseDetails = Backbone.Model.extend({ defaults: { @@ -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, @@ -38,10 +39,16 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js self_paced: null }, - validate: function(newattrs) { - // 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. + validate: function (newattrs) { + // 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'] @@ -55,15 +62,15 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js errors.end_date = gettext('The course end date must be later than the course start date.'); } if (newattrs.start_date && newattrs.enrollment_start && - newattrs.start_date < newattrs.enrollment_start) { + newattrs.start_date < newattrs.enrollment_start) { errors.enrollment_start = gettext( - 'The course start date must be later than the enrollment start date.' + 'The course start date must be later than the enrollment start date.' ); } if (newattrs.enrollment_start && newattrs.enrollment_end && - newattrs.enrollment_start >= newattrs.enrollment_end) { + newattrs.enrollment_start >= newattrs.enrollment_end) { errors.enrollment_end = gettext( - 'The enrollment start date cannot be after the enrollment end date.' + 'The enrollment start date cannot be after the enrollment end date.' ); } if (newattrs.end_date && newattrs.enrollment_end && newattrs.end_date < newattrs.enrollment_end) { @@ -75,11 +82,43 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js 'The certificate available date must be later than the course end date.' ); } + + + 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 -'); } - // TODO check if key points to a real video using google's youtube api + // TODO check if key points to a real video using google's youtube api } if (_.has(newattrs, 'entrance_exam_minimum_score_pct')) { var range = { @@ -88,37 +127,37 @@ define(['backbone', 'underscore', 'gettext', 'js/models/validation_helpers', 'js }; if (!ValidationHelpers.validateIntegerRange(newattrs.entrance_exam_minimum_score_pct, range)) { errors.entrance_exam_minimum_score_pct = StringUtils.interpolate(gettext( - 'Please enter an integer between %(min)s and %(max)s.' + 'Please enter an integer between %(min)s and %(max)s.' ), range, true); } } if (!_.isEmpty(errors)) return errors; - // NOTE don't return empty errors as that will be interpreted as an error state + // NOTE don't return empty errors as that will be interpreted as an error state }, _videokey_illegal_chars: /[^a-zA-Z0-9_-]/g, - set_videosource: function(newsource) { - // newsource either is