diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index 53fe85aa97..5c1e7015c1 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -337,7 +337,7 @@ class RegisterFromCombinedPageTest(UniqueCourseTest): # Verify that the expected errors are displayed. errors = self.register_page.wait_for_errors() self.assertIn(u'Please enter your Public username.', errors) - self.assertIn(u'You must agree to the edX Terms of Service and Honor Code.', errors) + self.assertIn(u'You must agree to the edX Terms of Service and Honor Code', errors) self.assertIn(u'Please select your Country.', errors) self.assertIn(u'Please tell us your favorite movie.', errors) diff --git a/lms/static/js/spec/student_account/register_spec.js b/lms/static/js/spec/student_account/register_spec.js index d6ea9987e6..1b5b614224 100644 --- a/lms/static/js/spec/student_account/register_spec.js +++ b/lms/static/js/spec/student_account/register_spec.js @@ -162,7 +162,7 @@ { placeholder: '', name: 'honor_code', - label: 'I agree to the Terms of Service and Honor Code', + label: 'I agree to the Terms of Service and Honor Code', defaultValue: '', type: 'checkbox', required: true, diff --git a/lms/static/js/student_account/views/FormView.js b/lms/static/js/student_account/views/FormView.js index 92dbd73b2c..7bd52379c3 100644 --- a/lms/static/js/student_account/views/FormView.js +++ b/lms/static/js/student_account/views/FormView.js @@ -85,7 +85,9 @@ html.push(_.template(fieldTpl)($.extend(data[i], { form: this.formType, - requiredStr: this.requiredStr + requiredStr: this.requiredStr, + supplementalText: data[i].supplementalText || '', + supplementalLink: data[i].supplementalLink || '' }))); } diff --git a/lms/static/sass/views/_login-register.scss b/lms/static/sass/views/_login-register.scss index 8584ee9b48..ace30fe581 100644 --- a/lms/static/sass/views/_login-register.scss +++ b/lms/static/sass/views/_login-register.scss @@ -244,8 +244,9 @@ &.error { color: $red; } - - &[for="register-honor_code"], &[for="register-terms_of_service"] { + + &[for="register-honor_code"], + &[for="register-terms_of_service"] { display: inline-block; margin: 5px 5px 0 0; width: 90%; @@ -255,10 +256,6 @@ &[for="login-remember"] { display: inline-block; } - - a { - font-family: $sans-serif; - } } .field-link { @@ -604,3 +601,7 @@ font-weight: bold; } } + +.supplemental-link { + margin: 1rem 0; +} diff --git a/lms/templates/student_account/form_field.underscore b/lms/templates/student_account/form_field.underscore index ab55a1eb1c..4ea63bebba 100644 --- a/lms/templates/student_account/form_field.underscore +++ b/lms/templates/student_account/form_field.underscore @@ -4,6 +4,11 @@ <%= label %> <% if ( required && requiredStr ) { %> <%= requiredStr %><% } %> + <% if (supplementalLink && supplementalText) { %> +
+ <% } %> <% } %> <% if ( type === 'select' ) { %> @@ -24,6 +29,11 @@ <% }); %> <% if ( instructions ) { %> <%= instructions %><% } %> + <% if (supplementalLink && supplementalText) { %> + + <% } %> <% } else if ( type === 'textarea' ) { %> <% if ( instructions ) { %> <%= instructions %><% } %> + <% if (supplementalLink && supplementalText) { %> + + <% } %> <% } else { %> aria-describedby="<%= form %>-<%= name %>-desc" <% } %> <% if ( restrictions.min_length ) { %> minlength="<%= restrictions.min_length %>"<% } %> <% if ( restrictions.max_length ) { %> maxlength="<%= restrictions.max_length %>"<% } %> - <% if ( required ) { %> aria-required="true" required<% } %> + <% if ( required ) { %> required<% } %> <% if ( typeof errorMessages !== 'undefined' ) { _.each(errorMessages, function( msg, type ) {%> data-errormsg-<%= type %>="<%= msg %>" @@ -65,6 +80,11 @@ <% } %> <% if ( instructions ) { %> <%= instructions %><% } %> + <% if (supplementalLink && supplementalText) { %> + + <% } %> <% } %> <% if( form === 'login' && name === 'password' ) { %> diff --git a/openedx/core/djangoapps/user_api/helpers.py b/openedx/core/djangoapps/user_api/helpers.py index ff59f1410b..8321b315d5 100644 --- a/openedx/core/djangoapps/user_api/helpers.py +++ b/openedx/core/djangoapps/user_api/helpers.py @@ -138,7 +138,7 @@ class FormDescription(object): OVERRIDE_FIELD_PROPERTIES = [ "label", "type", "defaultValue", "placeholder", "instructions", "required", "restrictions", - "options" + "options", "supplementalLink", "supplementalText" ] def __init__(self, method, submit_url): @@ -158,6 +158,7 @@ class FormDescription(object): self, name, label=u"", field_type=u"text", default=u"", placeholder=u"", instructions=u"", required=True, restrictions=None, options=None, include_default_option=False, error_messages=None, + supplementalLink=u"", supplementalText=u"" ): """Add a field to the form description. @@ -198,6 +199,12 @@ class FormDescription(object): that the messages should be displayed if the user does not provide a value for a required field. + supplementalLink (unicode): A qualified URL to provide supplemental information + for the form field. An example may be a link to documentation for creating + strong passwords. + + supplementalText (unicode): The visible text for the supplemental link above. + Raises: InvalidFieldError @@ -219,6 +226,8 @@ class FormDescription(object): "required": required, "restrictions": {}, "errorMessages": {}, + "supplementalLink": supplementalLink, + "supplementalText": supplementalText } if field_type == "select": diff --git a/openedx/core/djangoapps/user_api/tests/test_helpers.py b/openedx/core/djangoapps/user_api/tests/test_helpers.py index 82e66fdb11..a0983a0573 100644 --- a/openedx/core/djangoapps/user_api/tests/test_helpers.py +++ b/openedx/core/djangoapps/user_api/tests/test_helpers.py @@ -88,7 +88,9 @@ class FormDescriptionTest(TestCase): }, error_messages={ "required": "You must provide a value!" - } + }, + supplementalLink="", + supplementalText="", ) self.assertEqual(desc.to_json(), json.dumps({ @@ -109,7 +111,9 @@ class FormDescriptionTest(TestCase): }, "errorMessages": { "required": "You must provide a value!" - } + }, + "supplementalLink": "", + "supplementalText": "" } ] })) diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index d6668e498e..bb190b75f9 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -596,6 +596,8 @@ class LoginSessionViewTest(UserAPITestCase): "max_length": EMAIL_MAX_LENGTH }, "errorMessages": {}, + "supplementalText": "", + "supplementalLink": "", }, { "name": "password", @@ -610,6 +612,8 @@ class LoginSessionViewTest(UserAPITestCase): "max_length": PASSWORD_MAX_LENGTH }, "errorMessages": {}, + "supplementalText": "", + "supplementalLink": "", }, { "name": "remember", @@ -621,6 +625,8 @@ class LoginSessionViewTest(UserAPITestCase): "instructions": "", "restrictions": {}, "errorMessages": {}, + "supplementalText": "", + "supplementalLink": "", }, ]) @@ -758,6 +764,8 @@ class PasswordResetViewTest(UserAPITestCase): "max_length": EMAIL_MAX_LENGTH }, "errorMessages": {}, + "supplementalText": "", + "supplementalLink": "", } ]) @@ -1173,22 +1181,22 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): ) @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": True}) def test_registration_honor_code_mktg_site_enabled(self): - link_html = 'Terms of Service and Honor Code' + link_label = 'Terms of Service and Honor Code' self._assert_reg_field( {"honor_code": "required"}, { - "label": "I agree to the {platform_name} {link_html}.".format( + "label": "I agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} {link_html}.".format( + "required": "You must agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ) } } @@ -1197,22 +1205,22 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): @override_settings(MKTG_URLS_LINK_MAP={"HONOR": "honor"}) @mock.patch.dict(settings.FEATURES, {"ENABLE_MKTG_SITE": False}) def test_registration_honor_code_mktg_site_disabled(self): - link_html = 'Terms of Service and Honor Code' + link_label = 'Terms of Service and Honor Code' self._assert_reg_field( {"honor_code": "required"}, { - "label": "I agree to the {platform_name} {link_html}.".format( + "label": "I agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} {link_html}.".format( + "required": "You must agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ) } } @@ -1227,44 +1235,44 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): def test_registration_separate_terms_of_service_mktg_site_enabled(self): # Honor code field should say ONLY honor code, # not "terms of service and honor code" - link_html = 'Honor Code' + link_label = 'Honor Code' self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the {platform_name} {link_html}.".format( + "label": "I agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ), "name": "honor_code", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} {link_html}.".format( + "required": "You must agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ) } } ) # Terms of service field should also be present - link_html = 'Terms of Service' + link_label = 'Terms of Service' self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the {platform_name} {link_html}.".format( + "label": "I agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ), "name": "terms_of_service", "defaultValue": False, "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} {link_html}.".format( + "required": "You must agree to the {platform_name} {link_label}".format( platform_name=settings.PLATFORM_NAME, - link_html=link_html + link_label=link_label ) } } @@ -1278,7 +1286,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the {platform_name} Honor Code.".format( + "label": "I agree to the {platform_name} Honor Code".format( platform_name=settings.PLATFORM_NAME ), "name": "honor_code", @@ -1286,7 +1294,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} Honor Code.".format( + "required": "You must agree to the {platform_name} Honor Code".format( platform_name=settings.PLATFORM_NAME ) } @@ -1297,7 +1305,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): self._assert_reg_field( {"honor_code": "required", "terms_of_service": "required"}, { - "label": "I agree to the {platform_name} Terms of Service.".format( + "label": "I agree to the {platform_name} Terms of Service".format( platform_name=settings.PLATFORM_NAME ), "name": "terms_of_service", @@ -1305,7 +1313,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "type": "checkbox", "required": True, "errorMessages": { - "required": "You must agree to the {platform_name} Terms of Service.".format( + "required": "You must agree to the {platform_name} Terms of Service".format( # pylint: disable=line-too-long platform_name=settings.PLATFORM_NAME ) } diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index 1b2db66d7f..70215786c7 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -755,31 +755,30 @@ class RegistrationView(APIView): """ # Separate terms of service and honor code checkboxes if self._is_field_visible("terms_of_service"): - terms_text = _(u"Honor Code") + terms_label = _(u"Honor Code") + terms_link = marketing_link("HONOR") + terms_text = _(u"Review the Honor Code") # Combine terms of service and honor code checkboxes else: # Translators: This is a legal document users must agree to # in order to register a new account. - terms_text = _(u"Terms of Service and Honor Code") + terms_label = _(u"Terms of Service and Honor Code") + terms_link = marketing_link("HONOR") + terms_text = _(u"Review the Terms of Service and Honor Code") - terms_link = u"{terms_text}".format( - url=marketing_link("HONOR"), - terms_text=terms_text + # Translators: "Terms of Service" is a legal document users must agree to + # in order to register a new account. + label = _(u"I agree to the {platform_name} {terms_of_service}").format( + platform_name=configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), + terms_of_service=terms_label ) # Translators: "Terms of Service" is a legal document users must agree to # in order to register a new account. - label = _(u"I agree to the {platform_name} {terms_of_service}.").format( + error_msg = _(u"You must agree to the {platform_name} {terms_of_service}").format( platform_name=configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), - terms_of_service=terms_link - ) - - # Translators: "Terms of Service" is a legal document users must agree to - # in order to register a new account. - error_msg = _(u"You must agree to the {platform_name} {terms_of_service}.").format( - platform_name=configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), - terms_of_service=terms_link + terms_of_service=terms_label ) form_desc.add_field( @@ -790,7 +789,9 @@ class RegistrationView(APIView): required=required, error_messages={ "required": error_msg - } + }, + supplementalLink=terms_link, + supplementalText=terms_text ) def _add_terms_of_service_field(self, form_desc, required=True): @@ -805,24 +806,22 @@ class RegistrationView(APIView): """ # Translators: This is a legal document users must agree to # in order to register a new account. - terms_text = _(u"Terms of Service") - terms_link = u"{terms_text}".format( - url=marketing_link("TOS"), - terms_text=terms_text + terms_label = _(u"Terms of Service") + terms_link = marketing_link("TIS") + terms_text = _(u"Review the Terms of Service") + + # Translators: "Terms of service" is a legal document users must agree to + # in order to register a new account. + label = _(u"I agree to the {platform_name} {terms_of_service}").format( + platform_name=configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), + terms_of_service=terms_label ) # Translators: "Terms of service" is a legal document users must agree to # in order to register a new account. - label = _(u"I agree to the {platform_name} {terms_of_service}.").format( + error_msg = _(u"You must agree to the {platform_name} {terms_of_service}").format( platform_name=configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), - terms_of_service=terms_link - ) - - # Translators: "Terms of service" is a legal document users must agree to - # in order to register a new account. - error_msg = _(u"You must agree to the {platform_name} {terms_of_service}.").format( - platform_name=configuration_helpers.get_value("PLATFORM_NAME", settings.PLATFORM_NAME), - terms_of_service=terms_link + terms_of_service=terms_label ) form_desc.add_field( @@ -833,7 +832,9 @@ class RegistrationView(APIView): required=required, error_messages={ "required": error_msg - } + }, + supplementalLink=terms_link, + supplementalText=terms_text ) def _apply_third_party_auth_overrides(self, request, form_desc):