From af033d25ccc733790a1d358459e52072a175db57 Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Thu, 28 May 2020 16:54:40 +0500 Subject: [PATCH] Added v2 for confrim email backward compatiblity updated tests fixed style issue Fixed tests for v2 api --- .../core/djangoapps/user_authn/urls_common.py | 4 + .../user_authn/views/registration_form.py | 31 ++- .../user_authn/views/tests/test_register.py | 207 ++++++++++++++++-- 3 files changed, 211 insertions(+), 31 deletions(-) diff --git a/openedx/core/djangoapps/user_authn/urls_common.py b/openedx/core/djangoapps/user_authn/urls_common.py index f51c789a9b..20d0982eec 100644 --- a/openedx/core/djangoapps/user_authn/urls_common.py +++ b/openedx/core/djangoapps/user_authn/urls_common.py @@ -27,6 +27,10 @@ urlpatterns = [ url(r'^user_api/v1/account/registration/$', register.RegistrationView.as_view(), name="user_api_registration"), + # V2 is created to avoid backward compatibility issue with confirm_email + url(r'^user_api/v2/account/registration/$', register.RegistrationView.as_view(), + name="user_api_registration_v2"), + # Moved from user_api/urls.py # `api/user` prefix is preserved for backwards compatibility. url( diff --git a/openedx/core/djangoapps/user_authn/views/registration_form.py b/openedx/core/djangoapps/user_authn/views/registration_form.py index 4ce0a8917c..afef5039b8 100644 --- a/openedx/core/djangoapps/user_authn/views/registration_form.py +++ b/openedx/core/djangoapps/user_authn/views/registration_form.py @@ -2,7 +2,6 @@ Objects and utilities used to construct registration forms. """ - import copy from importlib import import_module import re @@ -42,6 +41,7 @@ class TrueCheckbox(widgets.CheckboxInput): """ A checkbox widget that only accepts "true" (case-insensitive) as true. """ + def value_from_datadict(self, data, files, name): value = data.get(name, '') return value.lower() == 'true' @@ -164,12 +164,12 @@ class AccountCreationForm(forms.Form): ) def __init__( - self, - data=None, - extra_fields=None, - extended_profile_fields=None, - do_third_party_auth=True, - tos_required=True + self, + data=None, + extra_fields=None, + extended_profile_fields=None, + do_third_party_auth=True, + tos_required=True ): super(AccountCreationForm, self).__init__(data) @@ -430,6 +430,13 @@ class RegistrationFormFactory(object): required=self._is_field_required(field_name) ) + # remove confirm_email form v1 registration form + if 'v1' in request.get_full_path(): + for index, field in enumerate(form_desc.fields): + if field['name'] == 'confirm_email': + del form_desc.fields[index] + break + return form_desc def _add_email_field(self, form_desc, required=True): @@ -952,7 +959,6 @@ class RegistrationFormFactory(object): field_type = 'checkbox' if not separate_honor_and_tos: - field_type = 'plaintext' pp_link = marketing_link("PRIVACY") @@ -1069,10 +1075,11 @@ class RegistrationFormFactory(object): field_name, default=field_overrides[field_name] ) - if (field_name not in ['terms_of_service', 'honor_code'] - and field_overrides[field_name] - and hide_registration_fields_except_tos): - + if ( + field_name not in ['terms_of_service', 'honor_code'] and + field_overrides[field_name] and + hide_registration_fields_except_tos + ): form_desc.override_field_properties( field_name, field_type="hidden", diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index c890fae8b2..d3e83c810e 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- """Tests for account creation""" - import json from unittest import skipIf, skipUnless from datetime import datetime @@ -328,7 +327,7 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa @ddt.ddt @skip_unless_lms -class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): +class RegistrationViewTestV1(ThirdPartyAuthTestMixin, UserAPITestCase): """Tests for the registration end-points of the User API. """ maxDiff = None @@ -392,7 +391,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): link_template = u"{link_label}" def setUp(self): # pylint: disable=arguments-differ - super(RegistrationViewTest, self).setUp() + super(RegistrationViewTestV1, self).setUp() self.url = reverse("user_api_registration") @ddt.data("get", "post") @@ -494,7 +493,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): } ) - msg = u'Your password must contain at least 2 characters, including '\ + msg = u'Your password must contain at least 2 characters, including ' \ u'3 uppercase letters & 1 symbol.' self._assert_reg_field( no_extra_fields_setting, @@ -950,18 +949,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): ) def test_registration_form_confirm_email(self): - self._assert_reg_field( - {"confirm_email": "required"}, - { - "name": "confirm_email", - "type": "text", - "required": True, - "label": "Confirm Email", - "errorMessages": { - "required": "The email addresses do not match.", - } - } - ) + pass @override_settings( MKTG_URLS={"ROOT": "https://www.test.com/", "HONOR": "honor"}, @@ -1160,7 +1148,6 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "password", "favorite_movie", "favorite_editor", - "confirm_email", "city", "state", "country", @@ -1221,7 +1208,6 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "name", "username", "email", - "confirm_email", "password", "city", "state", @@ -1279,7 +1265,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): "password", "favorite_movie", "favorite_editor", - "confirm_email", + "city", "state", "country", @@ -1637,6 +1623,7 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): """ Test case to check user creation is forbidden when ALLOW_PUBLIC_ACCOUNT_CREATION feature flag is turned off """ + def _side_effect_for_get_value(value, default=None): """ returns a side_effect with given return value for a given value @@ -1735,6 +1722,188 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): }) +class RegistrationViewTestV2(RegistrationViewTestV1): + + def setUp(self): # pylint: disable=arguments-differ + super(RegistrationViewTestV1, self).setUp() + self.url = reverse("user_api_registration_v2") + + @override_settings( + REGISTRATION_EXTRA_FIELDS={ + "level_of_education": "optional", + "gender": "optional", + "year_of_birth": "optional", + "mailing_address": "optional", + "goals": "optional", + "city": "optional", + "state": "optional", + "country": "required", + "honor_code": "required", + "confirm_email": "required", + }, + REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', + REGISTRATION_FIELD_ORDER=[ + "name", + "confirm_email", + "password", + "first_name", + "last_name", + "gender", + "year_of_birth", + "level_of_education", + "company", + "title", + "mailing_address", + "goals", + "honor_code", + "terms_of_service", + ], + ) + def test_field_order_invalid_override(self): + response = self.client.get(self.url) + self.assertHttpOK(response) + + # Verify that all fields render in the correct order + form_desc = json.loads(response.content.decode('utf-8')) + field_names = [field["name"] for field in form_desc["fields"]] + + self.assertEqual(field_names, [ + "email", + "name", + "username", + "password", + "favorite_movie", + "favorite_editor", + "confirm_email", + "city", + "state", + "country", + "gender", + "year_of_birth", + "level_of_education", + "mailing_address", + "goals", + "honor_code", + ]) + + @override_settings( + REGISTRATION_EXTRA_FIELDS={ + "level_of_education": "optional", + "gender": "optional", + "year_of_birth": "optional", + "mailing_address": "optional", + "goals": "optional", + "city": "optional", + "state": "optional", + "country": "required", + "honor_code": "required", + "confirm_email": "required", + }, + REGISTRATION_FIELD_ORDER=[ + "name", + "username", + "email", + "confirm_email", + "password", + "first_name", + "last_name", + "city", + "state", + "country", + "gender", + "year_of_birth", + "level_of_education", + "company", + "title", + "job_title", + "mailing_address", + "goals", + "honor_code", + "terms_of_service", + "specialty", + "profession", + ], + ) + def test_field_order_override(self): + response = self.client.get(self.url) + self.assertHttpOK(response) + + # Verify that all fields render in the correct order + form_desc = json.loads(response.content.decode('utf-8')) + field_names = [field["name"] for field in form_desc["fields"]] + self.assertEqual(field_names, [ + "name", + "username", + "email", + "confirm_email", + "password", + "city", + "state", + "country", + "gender", + "year_of_birth", + "level_of_education", + "mailing_address", + "goals", + "honor_code", + ]) + + @override_settings( + REGISTRATION_EXTRA_FIELDS={ + "level_of_education": "optional", + "gender": "optional", + "year_of_birth": "optional", + "mailing_address": "optional", + "goals": "optional", + "city": "optional", + "state": "optional", + "country": "required", + "honor_code": "required", + "confirm_email": "required", + }, + REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', + ) + def test_field_order(self): + response = self.client.get(self.url) + self.assertHttpOK(response) + + # Verify that all fields render in the correct order + form_desc = json.loads(response.content.decode('utf-8')) + field_names = [field["name"] for field in form_desc["fields"]] + self.assertEqual(field_names, [ + "email", + "name", + "username", + "password", + "favorite_movie", + "favorite_editor", + "confirm_email", + "city", + "state", + "country", + "gender", + "year_of_birth", + "level_of_education", + "mailing_address", + "goals", + "honor_code", + ]) + + def test_registration_form_confirm_email(self): + self._assert_reg_field( + {"confirm_email": "required"}, + { + "name": "confirm_email", + "type": "text", + "required": True, + "label": "Confirm Email", + "errorMessages": { + "required": "The email addresses do not match.", + } + } + ) + + @httpretty.activate @ddt.ddt class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTestCase):