From c1a58499a534565e235e55f12cd7c4694c8e8ddc Mon Sep 17 00:00:00 2001 From: Syed Sajjad Hussain Shah Date: Thu, 16 Jun 2022 11:34:34 +0500 Subject: [PATCH] fix: remove fields based on extended_profile configuration Meta field in UserProfile model will only store those fields which are available in extended_profile configuration, so we are removing the fields that are not available in extended_profile configuration because their data will not be stored VAN-977 --- common/djangoapps/student/models.py | 2 ++ .../djangoapps/user_authn/api/form_fields.py | 2 +- .../core/djangoapps/user_authn/api/helper.py | 20 ++++++++--- .../user_authn/api/tests/test_views.py | 33 ++++++++++++++++++- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 5ef8ea06b4..03577a45af 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -497,6 +497,8 @@ class UserProfile(models.Model): user = models.OneToOneField(User, unique=True, db_index=True, related_name='profile', on_delete=models.CASCADE) name = models.CharField(blank=True, max_length=255, db_index=True) + # How meta field works: meta will only store those fields which are available in extended_profile configuration, + # so in order to store a field in meta, it must be available in extended_profile configuration. meta = models.TextField(blank=True) # JSON dictionary for future expansion courseware = models.CharField(blank=True, max_length=255, default='course.xml') diff --git a/openedx/core/djangoapps/user_authn/api/form_fields.py b/openedx/core/djangoapps/user_authn/api/form_fields.py index bfceb93fc6..3f60841db2 100644 --- a/openedx/core/djangoapps/user_authn/api/form_fields.py +++ b/openedx/core/djangoapps/user_authn/api/form_fields.py @@ -177,7 +177,7 @@ def add_profession_field(is_field_required=False): def add_specialty_field(is_field_required=False): """ - Returns the user speciality field description + Returns the user specialty field description """ # Translators: This label appears above a dropdown menu to select # the user's specialty diff --git a/openedx/core/djangoapps/user_authn/api/helper.py b/openedx/core/djangoapps/user_authn/api/helper.py index f5e355afe6..5a1762172e 100644 --- a/openedx/core/djangoapps/user_authn/api/helper.py +++ b/openedx/core/djangoapps/user_authn/api/helper.py @@ -9,6 +9,7 @@ from rest_framework.views import APIView from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_authn.api import form_fields from openedx.core.djangoapps.user_authn.views.registration_form import get_registration_extension_form +from common.djangoapps.student.models import UserProfile class RegistrationFieldsContext(APIView): @@ -37,6 +38,7 @@ class RegistrationFieldsContext(APIView): 'specialty', 'marketing_emails_opt_in', ] + user_profile_fields = [field.name for field in UserProfile._meta.get_fields()] def _get_field_order(self): """ @@ -84,6 +86,16 @@ class RegistrationFieldsContext(APIView): ): self.valid_fields.append(field_name) + def _field_can_be_saved(self, field): + """ + Checks if the field exists in UserProfile Model fields or extended_profile configuration, + if it exists, then the field is valid to save because the meta field in UserProfile model + only stores those fields which are available in extended_profile configuration, so we only + want to send those fields which can be saved. + """ + return (field in self.user_profile_fields or + field in configuration_helpers.get_value('extended_profile_fields', [])) + def _get_fields(self): """ Returns the required or optional fields configured in REGISTRATION_EXTRA_FIELDS settings. @@ -92,6 +104,8 @@ class RegistrationFieldsContext(APIView): custom_form = get_registration_extension_form() or {} response = {} for field in self.valid_fields: + if field == 'confirm_email' and self.field_type == 'optional' or not self._field_can_be_saved(field): + continue if custom_form and field in custom_form.fields: response[field] = form_fields.add_extension_form_field( field, custom_form, custom_form.fields[field], self.field_type @@ -99,10 +113,6 @@ class RegistrationFieldsContext(APIView): else: field_handler = getattr(form_fields, f'add_{field}_field', None) if field_handler: - if field == 'confirm_email': - if self.field_type == 'required': - response[field] = field_handler(self.field_type == 'required') - else: - response[field] = field_handler(self.field_type == 'required') + response[field] = field_handler(self.field_type == 'required') return response diff --git a/openedx/core/djangoapps/user_authn/api/tests/test_views.py b/openedx/core/djangoapps/user_authn/api/tests/test_views.py index c7aff51071..88ae19cc42 100644 --- a/openedx/core/djangoapps/user_authn/api/tests/test_views.py +++ b/openedx/core/djangoapps/user_authn/api/tests/test_views.py @@ -202,6 +202,11 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase): assert response.status_code == status.HTTP_200_OK assert response.data['registration_fields']['fields'] == {} + @with_site_configuration( + configuration={ + 'extended_profile_fields': ['first_name', 'last_name'] + } + ) @override_settings( ENABLE_DYNAMIC_REGISTRATION_FIELDS=True, REGISTRATION_EXTRA_FIELDS={'state': 'required', 'last_name': 'required', 'first_name': 'required'}, @@ -241,7 +246,8 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase): @with_site_configuration( configuration={ - 'EXTRA_FIELD_OPTIONS': {'profession': ['Software Engineer', 'Teacher', 'Other']} + 'EXTRA_FIELD_OPTIONS': {'profession': ['Software Engineer', 'Teacher', 'Other']}, + 'extended_profile_fields': ['profession', 'specialty'] } ) @override_settings( @@ -272,6 +278,11 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase): assert response.status_code == status.HTTP_200_OK assert response.data['optional_fields']['fields'] == expected_response + @with_site_configuration( + configuration={ + 'extended_profile_fields': ['specialty'] + } + ) @override_settings( ENABLE_DYNAMIC_REGISTRATION_FIELDS=True, REGISTRATION_EXTRA_FIELDS={'goals': 'optional', 'specialty': 'optional'}, @@ -285,6 +296,26 @@ class MFEContextViewTest(ThirdPartyAuthTestMixin, APITestCase): assert response.status_code == status.HTTP_200_OK assert list(response.data['optional_fields']['fields'].keys()) == ['specialty', 'goals'] + @with_site_configuration( + configuration={ + 'extended_profile_fields': ['specialty'] + } + ) + @override_settings( + ENABLE_DYNAMIC_REGISTRATION_FIELDS=True, + REGISTRATION_EXTRA_FIELDS={'profession': 'required', 'specialty': 'required'}, + REGISTRATION_FIELD_ORDER=['specialty', 'profession'], + ) + def test_field_not_available_in_extended_profile_config(self): + """ + Test that if the field is not available in extended_profile configuration then the field + will not be sent in response. + """ + self.query_params.update({'is_registered': True}) + response = self.client.get(self.url, self.query_params) + assert response.status_code == status.HTTP_200_OK + assert list(response.data['registration_fields']['fields'].keys()) == ['specialty'] + @skip_unless_lms class SendAccountActivationEmail(UserAPITestCase):