diff --git a/docs/concepts/extension_points.rst b/docs/concepts/extension_points.rst index cfc2c38fd689..37276e55b265 100644 --- a/docs/concepts/extension_points.rst +++ b/docs/concepts/extension_points.rst @@ -122,13 +122,6 @@ Here are the different integration points that python plugins can use: * - Custom profile extension form app (``PROFILE_EXTENSION_FORM`` Django setting in the LMS) - Trial, Stable - By default, the registration page for each instance of Open edX has fields that ask for information such as a user’s name, country, and highest level of education completed. You can add custom fields to the registration page and user profile for your own Open edX instance. These fields can be different types, including text entry fields and drop-down lists. See `Adding Custom Fields to the Registration Page`_. - - **Important Migration Note:** - - - ``REGISTRATION_EXTENSION_FORM`` (deprecated) continues to work with old behavior: custom fields only for registration, data stored in UserProfile.meta - - ``PROFILE_EXTENSION_FORM`` (new) enables new capabilities: custom fields in registration and account settings, data stored in dedicated model - - Sites using the deprecated setting will maintain backward compatibility. To get the new capabilities, migrate to ``PROFILE_EXTENSION_FORM``. * - Learning Context (``openedx.learning_context``) - Trial, Limited - A "Learning Context" is a course, a library, a program, a blog, an external site, or some other collection of content where learning happens. If you are trying to build a totally new learning experience that's not a type of course, you may need to implement a new learning context. Learning contexts are a new abstraction and are only supported in the nascent openedx_content-based XBlock runtime. Since existing courses use modulestore instead of openedx_content, they are not yet implemented as learning contexts. However, openedx_content-based content libraries are. See |learning_context.py|_ to learn more. diff --git a/lms/envs/common.py b/lms/envs/common.py index 3b7133836d9c..3a6c7349f2ee 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2620,40 +2620,21 @@ FINANCIAL_ASSISTANCE_MIN_LENGTH = 1250 FINANCIAL_ASSISTANCE_MAX_LENGTH = 2500 -#### Registration form extension. #### -# Only used if combined login/registration is enabled. -# This can be used to add fields to the registration page. -# It must be a path to a valid form, in dot-separated syntax. -# IE: custom_form_app.forms.RegistrationExtensionForm -# Note: If you want to use a model to store the results of the form, you will -# need to add the model's app to the ADDL_INSTALLED_APPS array in your -# lms.yml file. -# -# REGISTRATION_EXTENSION_FORM is deprecated but will continue to work for backward compatibility. -# Sites using this setting will maintain the old behavior: -# - Data is stored in UserProfile.meta JSON field -# - No ability to update extended fields after registration via account settings API -# -# To get new capabilities (model-based storage), migrate to PROFILE_EXTENSION_FORM. -REGISTRATION_EXTENSION_FORM = None # DEPRECATED: Use PROFILE_EXTENSION_FORM instead +##### User Profile Extension ##### -# PROFILE_EXTENSION_FORM is a Django ModelForm class used for extending user profiles -# beyond the default fields. This setting enables new capabilities for profile management: -# - Data is stored in a dedicated model (not just UserProfile.meta) -# - Users can update their extended profile fields via the account settings API -# -# This setting supersedes REGISTRATION_EXTENSION_FORM and provides more accurate naming -# for profile extension functionality. -# -# Example: PROFILE_EXTENSION_FORM = 'myapp.forms.ExtendedProfileForm' -# -# The custom form's model should have: -# - A OneToOneField to User (typically named 'user') -# - Additional fields for extended profile data +# PROFILE_EXTENSION_FORM is a Django ModelForm used to add extended user +# fields to the platform. This setting integrates these fields into both +# the registration form and the account settings api. + +# Usage: +# - Set this to the dot-separated path of a valid ModelForm class. +# Example: PROFILE_EXTENSION_FORM = 'my_custom_app.forms.ExtendedProfileForm' # -# MIGRATION NOTE: If you're currently using REGISTRATION_EXTENSION_FORM (deprecated), -# your custom fields will continue working as before (data in meta field). -# To get the new capabilities, migrate to PROFILE_EXTENSION_FORM. +# Requirements: +# 1. Linked Model: The form must be based on a model with a OneToOneField to +# the User (typically named 'user'). +# 2. Installed Apps: Ensure the app containing your model is included in +# the ADDL_INSTALLED_APPS array. PROFILE_EXTENSION_FORM = None # Identifier included in the User Agent from Open edX mobile apps. diff --git a/openedx/core/djangoapps/user_api/accounts/forms.py b/openedx/core/djangoapps/user_api/accounts/forms.py index 336eb85f9cf9..529fdcbd1a72 100644 --- a/openedx/core/djangoapps/user_api/accounts/forms.py +++ b/openedx/core/djangoapps/user_api/accounts/forms.py @@ -11,8 +11,8 @@ from common.djangoapps.student.models import User from openedx.core.djangoapps.user_api.accounts.utils import handle_retirement_cancellation from openedx.core.djangoapps.user_authn.views.registration_form import ( - get_extended_profile_model, - get_registration_extension_form, + get_profile_extension_form, + get_profile_extension_model, ) logger = logging.getLogger(__name__) @@ -108,22 +108,22 @@ def get_extended_profile_form( - field_errors (dict): Dictionary of validation errors, if any """ field_errors, kwargs = {}, {} - extended_profile_model = get_extended_profile_model() + profile_extension_model = get_profile_extension_model() try: - kwargs["instance"] = extended_profile_model.objects.get(user=user) + kwargs["instance"] = profile_extension_model.objects.get(user=user) except AttributeError: - logger.info("No extended profile model configured") + logger.info("No profile extension model configured") except ObjectDoesNotExist: - logger.info("No existing extended profile found for user %s, creating new instance", user.username) + logger.info("No existing profile extension found for user %s, creating new instance", user.username) try: - extended_profile_form = get_registration_extension_form(data=extended_profile_fields_data, **kwargs) + extended_profile_form = get_profile_extension_form(data=extended_profile_fields_data, **kwargs) except Exception as e: # pylint: disable=broad-exception-caught logger.error("Unexpected error creating custom form for user %s: %s", user.username, str(e)) - field_errors["extended_profile"] = { + field_errors["profile_extension"] = { "developer_message": f"Error creating custom form: {str(e)}", - "user_message": _("There was an error processing the extended profile information"), + "user_message": _("There was an error processing the profile extension information"), } return None, field_errors diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index 2d6a8b384881..2aa4feb3a0c2 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -29,7 +29,7 @@ from openedx.core.djangoapps.user_authn.views.registration_form import ( contains_html, contains_url, - get_extended_profile_model, + get_profile_extension_model, ) from openedx.features.name_affirmation_api.utils import get_name_affirmation_service @@ -577,18 +577,19 @@ def get_extended_profile(user_profile: UserProfile) -> list[dict[str, str]]: This function extracts custom profile fields that extend beyond the standard UserProfile model. It prefers data from a custom extended profile model - (when configured), and only uses the `user_profile.meta` JSON field when - no such model is configured. The returned data is filtered to include only - fields specified in the `extended_profile_fields` site configuration. + (when configured), falling back to the `user_profile.meta` JSON field for + any field not present in the model (or when the user has no model record yet). - The function supports two data sources: + The returned data is filtered to include only fields specified in the + `extended_profile_fields` site configuration. + + The function supports two data sources (applied per field): 1. Custom model: If the `PROFILE_EXTENSION_FORM` setting points to a form with a - `Meta.model`, data is retrieved from that model using `model_to_dict()`. If a - model is configured but the user does not yet have a corresponding record, - this function returns an empty mapping for extended profile fields (it does - not fall back to `user_profile.meta` in that case). - 2. Fallback: JSON data stored in `UserProfile.meta` field, used only when no - custom extended profile model is configured. + `Meta.model`, data is retrieved from that model using `model_to_dict()`. + Fields not present in the model, or fields when the user has no model record, + fall back to `user_profile.meta`. + 2. Fallback: JSON data stored in `UserProfile.meta` field, used when no custom + extended profile model is configured or when the model lacks a given field. Args: user_profile (UserProfile): The user profile instance to get extended fields from. @@ -596,23 +597,26 @@ def get_extended_profile(user_profile: UserProfile) -> list[dict[str, str]]: Returns: list[dict[str, str]]: A list of dictionaries, each containing: - field_name: The name of the extended profile field - - field_value: The value of the field (converted to string) + - field_value: The value of the field """ def get_extended_profile_data(): - extended_profile_model = get_extended_profile_model() - - if extended_profile_model: - try: - profile_obj = extended_profile_model.objects.get(user=user_profile.user) - return model_to_dict(profile_obj) - except extended_profile_model.DoesNotExist: - return {} - try: - return json.loads(user_profile.meta or "{}") + meta_data = json.loads(user_profile.meta or "{}") except (ValueError, TypeError, AttributeError): - return {} + meta_data = {} + + profile_extension_model = get_profile_extension_model() + if profile_extension_model: + try: + profile_obj = profile_extension_model.objects.get(user=user_profile.user) + model_data = model_to_dict(profile_obj) + # Model fields take precedence. Meta fills in any field absent from the model. + return {**meta_data, **model_data} + except ObjectDoesNotExist: + pass + + return meta_data data = get_extended_profile_data() field_names = configuration_helpers.get_value("extended_profile_fields", []) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_forms.py b/openedx/core/djangoapps/user_api/accounts/tests/test_forms.py index bc2c4c0acd1a..ed2a27eb2247 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_forms.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_forms.py @@ -113,7 +113,7 @@ def setUp(self): super().setUp() self.user = UserFactory.create() - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_model") def test_get_extended_profile_form_no_model_configured(self, mock_get_model: Mock): """ Test when no extended profile model is configured @@ -126,7 +126,7 @@ def test_get_extended_profile_form_no_model_configured(self, mock_get_model: Moc self.assertIsNone(form) # noqa: PT009 self.assertEqual(errors, {}) # noqa: PT009 - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_model") def test_get_extended_profile_form_model_has_no_objects(self, mock_get_model: Mock): """ Test when model doesn't have objects attribute (AttributeError) @@ -140,8 +140,8 @@ def test_get_extended_profile_form_model_has_no_objects(self, mock_get_model: Mo self.assertIsNone(form) # noqa: PT009 self.assertEqual(errors, {}) # noqa: PT009 - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_registration_extension_form") - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_form") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_model") def test_get_extended_profile_form_with_existing_instance(self, mock_get_model: Mock, mock_get_form: Mock): """ Test form creation with an existing profile instance @@ -162,8 +162,8 @@ def test_get_extended_profile_form_with_existing_instance(self, mock_get_model: mock_model.objects.get.assert_called_once_with(user=self.user) mock_get_form.assert_called_once_with(data=extended_profile_fields_data, instance=mock_instance) - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_registration_extension_form") - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_form") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_model") def test_get_extended_profile_form_without_existing_instance(self, mock_get_model: Mock, mock_get_form: Mock): """ Test form creation for a new profile (no existing instance) @@ -184,8 +184,8 @@ def test_get_extended_profile_form_without_existing_instance(self, mock_get_mode mock_model.objects.get.assert_called_once_with(user=self.user) mock_get_form.assert_called_once_with(data=extended_profile_fields_data) - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_registration_extension_form") - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_form") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_model") def test_get_extended_profile_form_validation_errors(self, mock_get_model: Mock, mock_get_form: Mock): """ Test when form validation fails @@ -205,21 +205,21 @@ def test_get_extended_profile_form_validation_errors(self, mock_get_model: Mock, self.assertEqual(errors["department"]["user_message"], "This field is required") # noqa: PT009 self.assertEqual(errors["title"]["user_message"], "Invalid value") # noqa: PT009 - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_registration_extension_form") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_form") def test_get_extended_profile_form_returns_none(self, mock_get_form: Mock): """ - Test when get_registration_extension_form returns None + Test when get_profile_extension_form returns None """ mock_get_form.return_value = None extended_profile_fields_data = {"department": "Engineering"} - with patch("openedx.core.djangoapps.user_api.accounts.forms.get_extended_profile_model"): + with patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_model"): form, errors = get_extended_profile_form(extended_profile_fields_data, self.user) self.assertIsNone(form) # noqa: PT009 self.assertEqual(errors, {}) # noqa: PT009 - @patch("openedx.core.djangoapps.user_api.accounts.forms.get_registration_extension_form") + @patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_form") def test_get_extended_profile_form_exception_during_creation(self, mock_get_form: Mock): """ Test when an unexpected exception occurs during form creation @@ -227,12 +227,12 @@ def test_get_extended_profile_form_exception_during_creation(self, mock_get_form mock_get_form.side_effect = Exception("Unexpected error") extended_profile_fields_data = {"department": "Engineering"} - with patch("openedx.core.djangoapps.user_api.accounts.forms.get_extended_profile_model"): + with patch("openedx.core.djangoapps.user_api.accounts.forms.get_profile_extension_model"): form, errors = get_extended_profile_form(extended_profile_fields_data, self.user) self.assertIsNone(form) # noqa: PT009 - self.assertIn("extended_profile", errors) # noqa: PT009 - self.assertIn("Error creating custom form", errors["extended_profile"]["developer_message"]) # noqa: PT009 + self.assertIn("profile_extension", errors) # noqa: PT009 + self.assertIn("Error creating custom form", errors["profile_extension"]["developer_message"]) # noqa: PT009 class TestValidateAndGetExtendedProfileForm(TestCase): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py b/openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py index 6fac6196aa14..f38407af6be5 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_serializers.py @@ -5,9 +5,9 @@ import logging from unittest.mock import Mock, patch +from django.core.exceptions import ObjectDoesNotExist from django.test import TestCase from django.test.client import RequestFactory -from django.test.utils import override_settings from testfixtures import LogCapture from common.djangoapps.student.models import UserProfile @@ -66,7 +66,7 @@ def setUp(self): self.user_profile = UserProfile.objects.get(user=self.user) @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") - @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") def test_get_extended_profile_from_model(self, mock_get_model: Mock, mock_config_helpers: Mock): """ Test getting extended profile from a custom model @@ -96,11 +96,10 @@ def test_get_extended_profile_from_model(self, mock_get_model: Mock, mock_config self.assertIn({"field_name": "title", "field_value": "Software Engineer"}, result) # noqa: PT009 self.assertIn({"field_name": "company", "field_value": "EdX"}, result) # noqa: PT009 - @override_settings(REGISTRATION_EXTENSION_FORM=None) @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") - def test_get_extended_profile_model_does_not_exist(self, mock_config_helpers: Mock): + def test_get_extended_profile_no_model_configured_uses_meta(self, mock_config_helpers: Mock): """ - Test fallback to meta field when model instance doesn't exist + Test fallback to meta field when no PROFILE_EXTENSION_FORM is configured. """ mock_config_helpers.get_value.return_value = ["department", "title"] self.user_profile.set_meta({"department": "Sales", "title": "Manager"}) @@ -113,7 +112,80 @@ def test_get_extended_profile_model_does_not_exist(self, mock_config_helpers: Mo self.assertIn({"field_name": "title", "field_value": "Manager"}, result) # noqa: PT009 @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") - @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") + def test_get_profile_extension_model_instance_missing_falls_back_to_meta( + self, mock_get_model: Mock, mock_config_helpers: Mock + ): + """ + Test that when a model is configured but the user has no record in it, + the function falls back to user_profile.meta instead of returning empty. + """ + mock_config_helpers.get_value.return_value = ["department", "title"] + mock_model = Mock() + mock_model.objects.get.side_effect = ObjectDoesNotExist + mock_get_model.return_value = mock_model + + self.user_profile.set_meta({"department": "Sales", "title": "Manager"}) + self.user_profile.save() + + result = get_extended_profile(self.user_profile) + + self.assertEqual(len(result), 2) # noqa: PT009 + self.assertIn({"field_name": "department", "field_value": "Sales"}, result) # noqa: PT009 + self.assertIn({"field_name": "title", "field_value": "Manager"}, result) # noqa: PT009 + + @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") + def test_get_extended_profile_field_absent_from_model_falls_back_to_meta( + self, mock_get_model: Mock, mock_config_helpers: Mock + ): + """ + Test that a configured field not present in the model is read from meta. + Model has 'department', but 'title' is only in meta and must be returned from there. + """ + mock_config_helpers.get_value.return_value = ["department", "title"] + mock_model = Mock() + mock_get_model.return_value = mock_model + + self.user_profile.set_meta({"department": "Meta-Dept", "title": "Meta-Title"}) + self.user_profile.save() + + with patch("openedx.core.djangoapps.user_api.accounts.serializers.model_to_dict") as mock_model_to_dict: + # Model only knows about 'department', but 'title' is absent. + mock_model_to_dict.return_value = {"department": "Model-Dept"} + + result = get_extended_profile(self.user_profile) + + self.assertEqual(len(result), 2) # noqa: PT009 + self.assertIn({"field_name": "department", "field_value": "Model-Dept"}, result) # noqa: PT009 + self.assertIn({"field_name": "title", "field_value": "Meta-Title"}, result) # noqa: PT009 + + @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") + def test_get_profile_extension_model_takes_precedence_over_meta( + self, mock_get_model: Mock, mock_config_helpers: Mock + ): + """ + Test that when both the model and meta have a value for the same field, + the model value takes precedence. + """ + mock_config_helpers.get_value.return_value = ["department"] + mock_model = Mock() + mock_get_model.return_value = mock_model + + self.user_profile.set_meta({"department": "Meta-Dept"}) + self.user_profile.save() + + with patch("openedx.core.djangoapps.user_api.accounts.serializers.model_to_dict") as mock_model_to_dict: + mock_model_to_dict.return_value = {"department": "Model-Dept"} + + result = get_extended_profile(self.user_profile) + + self.assertEqual(len(result), 1) # noqa: PT009 + self.assertIn({"field_name": "department", "field_value": "Model-Dept"}, result) # noqa: PT009 + + @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") def test_get_extended_profile_no_model_configured(self, mock_get_model: Mock, mock_config_helpers: Mock): """ Test fallback to meta field when no model is configured @@ -131,7 +203,7 @@ def test_get_extended_profile_no_model_configured(self, mock_get_model: Mock, mo self.assertIn({"field_name": "title", "field_value": "Director"}, result) # noqa: PT009 @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") - @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") def test_get_extended_profile_empty_meta(self, mock_get_model: Mock, mock_config_helpers: Mock): """ Test getting extended profile with empty meta field @@ -148,7 +220,7 @@ def test_get_extended_profile_empty_meta(self, mock_get_model: Mock, mock_config self.assertIn({"field_name": "title", "field_value": ""}, result) # noqa: PT009 @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") - @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") def test_get_extended_profile_invalid_json_in_meta(self, mock_get_model: Mock, mock_config_helpers: Mock): """ Test getting extended profile with invalid JSON in meta field @@ -165,7 +237,7 @@ def test_get_extended_profile_invalid_json_in_meta(self, mock_get_model: Mock, m self.assertIn({"field_name": "title", "field_value": ""}, result) # noqa: PT009 @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") - @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") def test_get_extended_profile_missing_fields(self, mock_get_model: Mock, mock_config_helpers: Mock): """ Test getting extended profile when some configured fields are missing @@ -184,7 +256,7 @@ def test_get_extended_profile_missing_fields(self, mock_get_model: Mock, mock_co self.assertIn({"field_name": "location", "field_value": ""}, result) # noqa: PT009 @patch("openedx.core.djangoapps.user_api.accounts.serializers.configuration_helpers") - @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_extended_profile_model") + @patch("openedx.core.djangoapps.user_api.accounts.serializers.get_profile_extension_model") def test_get_extended_profile_no_configured_fields(self, mock_get_model: Mock, mock_config_helpers: Mock): """ Test getting extended profile when no fields are configured diff --git a/openedx/core/djangoapps/user_authn/api/helper.py b/openedx/core/djangoapps/user_authn/api/helper.py index 9e4c8104b0ff..75ebeb10a842 100644 --- a/openedx/core/djangoapps/user_authn/api/helper.py +++ b/openedx/core/djangoapps/user_authn/api/helper.py @@ -9,7 +9,7 @@ from common.djangoapps.student.models import UserProfile 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 openedx.core.djangoapps.user_authn.views.registration_form import get_profile_extension_form class RegistrationFieldsContext(APIView): @@ -74,7 +74,7 @@ def __init__(self, field_type='required'): field for field in ordered_extra_fields if self._fields_setting.get(field) == self.field_type ] - custom_form = get_registration_extension_form() + custom_form = get_profile_extension_form() if custom_form: for field_name, field in custom_form.fields.items(): # If the field_type is required make sure the custom field is required in the form and if the @@ -101,8 +101,7 @@ def get_fields(self): Returns the required or optional fields configured in REGISTRATION_EXTRA_FIELDS settings. """ # Custom form fields can be added via the form set in settings.PROFILE_EXTENSION_FORM - # (or deprecated settings.REGISTRATION_EXTENSION_FORM) - custom_form = get_registration_extension_form() or {} + custom_form = get_profile_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): diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index 4a78ee83ef06..d36b43b646d0 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -86,7 +86,7 @@ from openedx.core.djangoapps.user_authn.views.registration_form import ( AccountCreationForm, RegistrationFormFactory, - get_registration_extension_form, + get_profile_extension_form, ) from openedx.core.djangoapps.user_authn.views.utils import get_auto_generated_username @@ -205,7 +205,7 @@ def create_account_with_params(request, params): # pylint: disable=too-many-sta do_third_party_auth=False, tos_required=tos_required, ) - custom_form = get_registration_extension_form(data=params) + custom_form = get_profile_extension_form(data=params) is_marketable = params.get('marketing_emails_opt_in') in ['true', '1'] # Perform operations within a transaction that are critical to account creation diff --git a/openedx/core/djangoapps/user_authn/views/registration_form.py b/openedx/core/djangoapps/user_authn/views/registration_form.py index 77984aa2bce7..4e035acd391d 100644 --- a/openedx/core/djangoapps/user_authn/views/registration_form.py +++ b/openedx/core/djangoapps/user_authn/views/registration_form.py @@ -317,36 +317,25 @@ def clean_country(self): return self.cleaned_data.get("country") -def get_registration_extension_form(*args, **kwargs) -> forms.Form | None: +def get_profile_extension_form(*args, **kwargs) -> forms.Form | None: """ - Convenience function for getting the custom form set in settings.PROFILE_EXTENSION_FORM - or settings.REGISTRATION_EXTENSION_FORM (deprecated). + Get an instance of the custom profile extension form. - Returns an instance of the configured profile extension form. + This form is configured via the `PROFILE_EXTENSION_FORM` Django setting - The function first checks for PROFILE_EXTENSION_FORM (recommended), then falls back to - REGISTRATION_EXTENSION_FORM for backwards compatibility. When REGISTRATION_EXTENSION_FORM - is used, a deprecation warning is logged. - - An example form app for this can be found at http://github.com/open-craft/custom-form-app + Args: + *args: Variable length argument list passed to the form's __init__ method. + **kwargs: Arbitrary keyword arguments passed to the form's __init__ method. Returns: - Form instance or None if no form is configured + forms.Form | None: An initialized form instance, or None if the setting + is not configured or the form fails to load. + + References: + An example form app for this can be found at: + http://github.com/open-craft/custom-form-app """ - # Check for the new setting first setting_value = getattr(settings, "PROFILE_EXTENSION_FORM", None) - setting_name = "PROFILE_EXTENSION_FORM" - - # Fall back to the deprecated setting - if not setting_value: - setting_value = getattr(settings, "REGISTRATION_EXTENSION_FORM", None) - if setting_value: - setting_name = "REGISTRATION_EXTENSION_FORM" - log.warning( - "REGISTRATION_EXTENSION_FORM is deprecated and will be removed in a future release. " - "Please use PROFILE_EXTENSION_FORM instead. Current value: %s", - setting_value, - ) if not setting_value: return None @@ -356,44 +345,25 @@ def get_registration_extension_form(*args, **kwargs) -> forms.Form | None: module = import_module(module) return getattr(module, klass)(*args, **kwargs) except (ValueError, ImportError, AttributeError) as e: - log.error("Could not load form from %s='%s': %s", setting_name, setting_value, str(e)) + log.error("Could not load form from PROFILE_EXTENSION_FORM='%s': %s", setting_value, str(e)) return None -def get_extended_profile_model() -> type[Model] | None: +def get_profile_extension_model() -> type[Model] | None: """ - Get the model class for the extended profile form. - - Returns the Django model class associated with the form specified in - the `PROFILE_EXTENSION_FORM` setting. + Get the model class associated with the custom profile extension form. - IMPORTANT: This function only works with PROFILE_EXTENSION_FORM. If you're using - the deprecated REGISTRATION_EXTENSION_FORM, this will return None to maintain - backward compatibility. The new profile extension capabilities (loading/saving - to a custom model) are only available when using PROFILE_EXTENSION_FORM. - - Migration path: - - Old behavior (REGISTRATION_EXTENSION_FORM): Custom fields only for registration, - data stored in UserProfile.meta field - - New behavior (PROFILE_EXTENSION_FORM): Custom fields for registration and profile, - data stored in dedicated model with ability to load/update via account settings API + This extracts the model from the Meta class of the form specified in + the `PROFILE_EXTENSION_FORM` Django setting. Returns: - type[Model] | None: The model class if PROFILE_EXTENSION_FORM is configured - and valid, None otherwise (including when using the deprecated - REGISTRATION_EXTENSION_FORM). + type[Model] | None: The Django model class if the setting is configured + and valid, None otherwise. Examples: - # New setting with model support: - # In settings.py: PROFILE_EXTENSION_FORM = 'myapp.forms.ExtendedProfileForm' - model_class = get_extended_profile_model() # Returns the model - - # Deprecated setting - maintains old behavior: - # In settings.py: REGISTRATION_EXTENSION_FORM = 'myapp.forms.ExtendedForm' - model_class = get_extended_profile_model() # Returns None (no model support) + # In settings.py: PROFILE_EXTENSION_FORM = 'myapp.forms.ExtensionForm' + model_class = get_profile_extension_model() """ - # Only check for the new setting - do NOT fall back to REGISTRATION_EXTENSION_FORM - # This ensures backward compatibility: users of the old setting keep the old behavior setting_value = getattr(settings, "PROFILE_EXTENSION_FORM", None) if not setting_value: @@ -491,7 +461,7 @@ def __init__(self): handler = getattr(self, f"_add_{field_name}_field") self.field_handlers[field_name] = handler - custom_form = get_registration_extension_form() + custom_form = get_profile_extension_form() if custom_form: custom_form_field_names = [field_name for field_name, field in custom_form.fields.items()] valid_fields.extend(custom_form_field_names) @@ -584,8 +554,7 @@ def get_registration_form(self, request): self._apply_third_party_auth_overrides(request, form_desc) # Custom form fields can be added via the form set in settings.PROFILE_EXTENSION_FORM - # (or deprecated settings.REGISTRATION_EXTENSION_FORM) - custom_form = get_registration_extension_form() + custom_form = get_profile_extension_form() if custom_form: custom_form_field_names = [field_name for field_name, field in custom_form.fields.items()] else: 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 5ba40a6c960e..1680f6d8b894 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -672,7 +672,7 @@ def test_register_form_password_complexity(self): } ) - @override_settings(REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm') + @override_settings(PROFILE_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm') def test_extension_form_fields(self): no_extra_fields_setting = {} @@ -1310,7 +1310,7 @@ def test_registration_separate_terms_of_service_mktg_site_disabled(self): "confirm_email": "required", }, REGISTRATION_FIELD_ORDER=None, - REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', + PROFILE_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', ) def test_field_order(self): response = self.client.get(self.url) @@ -1397,7 +1397,7 @@ def test_field_order_override(self): "honor_code": "required", "confirm_email": "required", }, - REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', + PROFILE_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', REGISTRATION_FIELD_ORDER=[ "name", "confirm_email", @@ -1504,7 +1504,7 @@ def test_register_with_profile_info(self): assert account_settings["goals"] == self.GOALS assert account_settings["country"] == self.COUNTRY - @override_settings(REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm') + @override_settings(PROFILE_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm') @mock.patch('openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm.DUMMY_STORAGE', new_callable=dict) @mock.patch( 'openedx.core.djangoapps.user_api.tests.test_helpers.DummyRegistrationExtensionModel', @@ -2072,7 +2072,7 @@ def setUp(self): # pylint: disable=arguments-differ "honor_code": "required", "confirm_email": "required", }, - REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', + PROFILE_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', REGISTRATION_FIELD_ORDER=[ "name", "confirm_email", @@ -2180,7 +2180,7 @@ def test_field_order_override(self): "confirm_email": "required", }, REGISTRATION_FIELD_ORDER=None, - REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', + PROFILE_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', ) def test_field_order(self): response = self.client.get(self.url) @@ -2222,7 +2222,7 @@ def test_field_order(self): "confirm_email": "required", }, REGISTRATION_FIELD_ORDER=None, - REGISTRATION_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', + PROFILE_EXTENSION_FORM='openedx.core.djangoapps.user_api.tests.test_helpers.TestCaseForm', ) def test_year_of_birth_field_with_feature_flag(self): """ diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_utils.py b/openedx/core/djangoapps/user_authn/views/tests/test_utils.py index e9fb7f71a81c..c3d002a00fa2 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_utils.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_utils.py @@ -11,8 +11,8 @@ from django.test.utils import override_settings from openedx.core.djangoapps.user_authn.views.registration_form import ( - get_extended_profile_model, - get_registration_extension_form, + get_profile_extension_form, + get_profile_extension_model, ) from openedx.core.djangoapps.user_authn.views.utils import _get_username_prefix, get_auto_generated_username @@ -100,7 +100,7 @@ def test_get_extended_profile_model_no_setting_or_empty_string(self, setting_val Test when `PROFILE_EXTENSION_FORM` setting is not configured """ with override_settings(PROFILE_EXTENSION_FORM=setting_value): - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertIsNone(result) # noqa: PT009 @@ -110,7 +110,7 @@ def test_get_extended_profile_model_invalid_module(self, mock_logger: Mock): """ Test when the module path is invalid """ - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertIsNone(result) # noqa: PT009 mock_logger.warning.assert_called_once() @@ -121,7 +121,7 @@ def test_get_extended_profile_model_no_meta_class(self): """ Test when the form class doesn't have a Meta class """ - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertIsNone(result) # noqa: PT009 @@ -131,7 +131,7 @@ def test_get_extended_profile_model_malformed_path(self, mock_logger: Mock): """ Test when the setting value doesn't have a dot separator """ - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertIsNone(result) # noqa: PT009 mock_logger.warning.assert_called_once() @@ -151,7 +151,7 @@ def test_get_extended_profile_model_custom_form(self, mock_import_module: Mock): mock_module.CustomExtendedProfileForm = mock_form_class mock_import_module.return_value = mock_module - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertEqual(result, mock_model) # noqa: PT009 mock_import_module.assert_called_once_with("myapp.forms") @@ -170,7 +170,7 @@ def test_get_extended_profile_model_form_without_model(self, mock_import_module: mock_module.FormWithoutModel = mock_form_class mock_import_module.return_value = mock_module - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertIsNone(result) # noqa: PT009 @@ -187,7 +187,7 @@ def test_get_extended_profile_model_import_errors( """ mock_import_module.side_effect = exception_class(error_message) - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertIsNone(result) # noqa: PT009 mock_logger.warning.assert_called_once() @@ -203,47 +203,35 @@ def test_get_extended_profile_model_attribute_error(self, mock_logger: Mock, moc mock_module = Mock(spec=[]) mock_import_module.return_value = mock_module - result = get_extended_profile_model() + result = get_profile_extension_model() self.assertIsNone(result) # noqa: PT009 mock_logger.warning.assert_called_once() self.assertIn("Could not load extended profile model", str(mock_logger.warning.call_args)) # noqa: PT009 - @override_settings(PROFILE_EXTENSION_FORM=None, REGISTRATION_EXTENSION_FORM="myapp.forms.LegacyForm") - def test_get_extended_profile_model_with_deprecated_setting_returns_none(self): - """ - Test that using REGISTRATION_EXTENSION_FORM returns None (maintains old behavior). - - This ensures backward compatibility: sites using REGISTRATION_EXTENSION_FORM - will NOT get the new model-based profile capabilities. They continue using - the old UserProfile.meta field approach. - """ - result = get_extended_profile_model() - - self.assertIsNone(result) # noqa: PT009 @ddt.ddt -class TestGetRegistrationExtensionForm(TestCase): +class TestGetProfileExtensionForm(TestCase): """ - Tests for get_registration_extension_form function + Tests for get_profile_extension_form function """ @ddt.data(None, "") - def test_get_registration_extension_form_no_setting(self, setting_value: str | None): + def test_get_profile_extension_form_no_setting(self, setting_value: str | None): """ - Test when neither PROFILE_EXTENSION_FORM nor REGISTRATION_EXTENSION_FORM is configured + Test when PROFILE_EXTENSION_FORM is not configured """ - with override_settings(PROFILE_EXTENSION_FORM=setting_value, REGISTRATION_EXTENSION_FORM=setting_value): - result = get_registration_extension_form() + with override_settings(PROFILE_EXTENSION_FORM=setting_value): + result = get_profile_extension_form() self.assertIsNone(result) # noqa: PT009 @override_settings(PROFILE_EXTENSION_FORM="myapp.forms.CustomProfileForm") @patch("openedx.core.djangoapps.user_authn.views.registration_form.import_module") - def test_get_registration_extension_form_with_new_setting(self, mock_import_module: Mock): + def test_get_profile_extension_form_with_setting(self, mock_import_module: Mock): """ - Test loading form from PROFILE_EXTENSION_FORM (new setting) + Test loading form from PROFILE_EXTENSION_FORM setting """ mock_form_instance = Mock() mock_form_class = Mock(return_value=mock_form_instance) @@ -251,61 +239,22 @@ def test_get_registration_extension_form_with_new_setting(self, mock_import_modu mock_module.CustomProfileForm = mock_form_class mock_import_module.return_value = mock_module - result = get_registration_extension_form(data={"field": "value"}) + result = get_profile_extension_form(data={"field": "value"}) self.assertEqual(result, mock_form_instance) # noqa: PT009 mock_import_module.assert_called_once_with("myapp.forms") mock_form_class.assert_called_once_with(data={"field": "value"}) - @override_settings(PROFILE_EXTENSION_FORM="myapp.forms.NewForm", REGISTRATION_EXTENSION_FORM="myapp.forms.OldForm") - @patch("openedx.core.djangoapps.user_authn.views.registration_form.import_module") - def test_get_registration_extension_form_new_setting_precedence(self, mock_import_module: Mock): - """ - Test that PROFILE_EXTENSION_FORM takes precedence over REGISTRATION_EXTENSION_FORM - """ - mock_form_instance = Mock() - mock_form_class = Mock(return_value=mock_form_instance) - mock_module = Mock() - mock_module.NewForm = mock_form_class - mock_import_module.return_value = mock_module - - result = get_registration_extension_form() - - self.assertEqual(result, mock_form_instance) # noqa: PT009 - mock_import_module.assert_called_once_with("myapp.forms") - - @override_settings(PROFILE_EXTENSION_FORM=None, REGISTRATION_EXTENSION_FORM="myapp.forms.LegacyForm") - @patch("openedx.core.djangoapps.user_authn.views.registration_form.import_module") - @patch("openedx.core.djangoapps.user_authn.views.registration_form.log") - def test_get_registration_extension_form_deprecation_warning(self, mock_logger: Mock, mock_import_module: Mock): - """ - Test that using REGISTRATION_EXTENSION_FORM logs a deprecation warning - """ - mock_form_instance = Mock() - mock_form_class = Mock(return_value=mock_form_instance) - mock_module = Mock() - mock_module.LegacyForm = mock_form_class - mock_import_module.return_value = mock_module - - result = get_registration_extension_form() - - self.assertEqual(result, mock_form_instance) # noqa: PT009 - deprecation_calls = [call for call in mock_logger.warning.call_args_list if "deprecated" in str(call).lower()] - self.assertGreater(len(deprecation_calls), 0, "Expected a deprecation warning to be logged") # noqa: PT009 - warning_message = str(deprecation_calls[0]) - self.assertIn("REGISTRATION_EXTENSION_FORM", warning_message) # noqa: PT009 - self.assertIn("PROFILE_EXTENSION_FORM", warning_message) # noqa: PT009 - @override_settings(PROFILE_EXTENSION_FORM="invalid.path") @patch("openedx.core.djangoapps.user_authn.views.registration_form.import_module") @patch("openedx.core.djangoapps.user_authn.views.registration_form.log") - def test_get_registration_extension_form_import_error(self, mock_logger: Mock, mock_import_module: Mock): + def test_get_profile_extension_form_import_error(self, mock_logger: Mock, mock_import_module: Mock): """ Test when form import fails """ mock_import_module.side_effect = ImportError("Module not found") - result = get_registration_extension_form() + result = get_profile_extension_form() self.assertIsNone(result) # noqa: PT009 error_calls = mock_logger.error.call_args_list @@ -313,11 +262,11 @@ def test_get_registration_extension_form_import_error(self, mock_logger: Mock, m @override_settings(PROFILE_EXTENSION_FORM="invalid_path_without_dot") @patch("openedx.core.djangoapps.user_authn.views.registration_form.log") - def test_get_registration_extension_form_malformed_path(self, mock_logger: Mock): + def test_get_profile_extension_form_malformed_path(self, mock_logger: Mock): """ Test when setting value doesn't have proper format (no dot separator) """ - result = get_registration_extension_form() + result = get_profile_extension_form() self.assertIsNone(result) # noqa: PT009