From 82b7046f2818a6270825f5de17b58df120ce0097 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Thu, 31 Jul 2025 18:46:50 -0500 Subject: [PATCH 1/3] feat: use extended profile model in the account settings --- .../core/djangoapps/user_api/accounts/api.py | 192 +++++++++++++++++- .../user_api/accounts/serializers.py | 56 +++-- .../user_authn/views/registration_form.py | 33 +++ 3 files changed, 258 insertions(+), 23 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index db58071fdd..4f77e78696 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -5,8 +5,11 @@ import datetime +import logging import re +from typing import Optional +from django import forms from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.core.validators import ValidationError, validate_email @@ -37,7 +40,9 @@ ) from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences from openedx.core.djangoapps.user_authn.utils import check_pwned_password -from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username +from openedx.core.djangoapps.user_authn.views.registration_form import ( + get_extended_profile_model, get_registration_extension_form, validate_name, validate_username +) from openedx.core.lib.api.view_utils import add_serializer_errors from openedx.features.enterprise_support.utils import get_enterprise_readonly_account_fields from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed @@ -48,6 +53,8 @@ # pylint: disable=import-error from edx_name_affirmation.name_change_validator import NameChangeValidator +logger = logging.getLogger(__name__) + # Public access point for this function. visible_fields = _visible_fields @@ -155,6 +162,7 @@ def update_account_settings(requesting_user, update, username=None): _validate_secondary_email(user, update, field_errors) old_name = _validate_name_change(user_profile, update, field_errors) old_language_proficiencies = _get_old_language_proficiencies_if_updating(user_profile, update) + extended_profile_form = _get_and_validate_extended_profile_form(update, user, field_errors) if field_errors: raise errors.AccountValidationError(field_errors) @@ -167,7 +175,7 @@ def update_account_settings(requesting_user, update, username=None): _update_preferences_if_needed(update, requesting_user, user) _notify_language_proficiencies_update_if_needed(update, user, user_profile, old_language_proficiencies) _store_old_name_if_needed(old_name, user_profile, requesting_user) - _update_extended_profile_if_needed(update, user_profile) + _update_extended_profile_if_needed(update, user_profile, extended_profile_form) _update_state_if_needed(update, user_profile) # Allow a plugin to save the updated values @@ -189,6 +197,141 @@ def update_account_settings(requesting_user, update, username=None): _send_email_change_requests_if_needed(update, user) +def _get_and_validate_extended_profile_form(update: dict, user, field_errors: dict) -> Optional[forms.Form]: + """ + Get and validate the extended profile form if it exists in the update. + + Args: + update (dict): The update data containing potential extended_profile fields + user (User): The user instance for whom the extended profile form is being validated + field_errors (dict): Dictionary to collect field validation errors + + Returns: + Optional[forms.Form]: The validated extended profile form instance, + or None if no extended profile form is needed + """ + extended_profile = update.get("extended_profile") + if not extended_profile: + return None + + extended_profile_fields_data = _extract_extended_profile_fields_data(extended_profile, field_errors) + if not extended_profile_fields_data: + return None + + extended_profile_form = _get_extended_profile_form_instance(extended_profile_fields_data, user, field_errors) + if not extended_profile_form: + return None + + _validate_extended_profile_form_and_collect_errors(extended_profile_form, field_errors) + + return extended_profile_form + + +def _extract_extended_profile_fields_data(extended_profile: Optional[list], field_errors: dict) -> dict: + """ + Extract extended profile fields data from extended_profile structure. + + Args: + extended_profile (Optional[list]): List of field data dictionaries + field_errors (dict): Dictionary to collect validation errors + + Returns: + dict: Extracted custom fields data + """ + if not isinstance(extended_profile, list): + field_errors["extended_profile"] = { + "developer_message": "extended_profile must be a list", + "user_message": _("Invalid extended profile format"), + } + return {} + + extended_profile_fields_data = {} + + for field_data in extended_profile: + if not isinstance(field_data, dict): + logger.warning("Invalid field_data structure in extended_profile: %s", field_data) + continue + + field_name = field_data.get("field_name") + field_value = field_data.get("field_value") + + if not field_name: + logger.warning("Missing field_name in extended_profile field_data: %s", field_data) + continue + + if field_value is not None: + extended_profile_fields_data[field_name] = field_value + + return extended_profile_fields_data + + +def _get_extended_profile_form_instance( + extended_profile_fields_data: dict, user, field_errors: dict +) -> Optional[forms.Form]: + """ + Get or create an extended profile form instance. + + Attempts to create a form instance using the configured `REGISTRATION_EXTENSION_FORM`. + If an extended profile model exists, tries to bind to existing user data or creates + a new instance. Handles import errors and missing configurations gracefully. + + Args: + extended_profile_fields_data (dict): Extended profile field data to populate the form + user (User): User instance to associate with the extended profile + field_errors (dict): Dictionary to collect validation errors if form creation fails + + Returns: + Optional[forms.Form]: Extended profile form instance with user data, or None if + no extended profile form is configured or creation fails + """ + try: + extended_profile_model = get_extended_profile_model() + + kwargs = {} + if not extended_profile_model: + logger.info("No extended profile model configured") + else: + try: + kwargs["instance"] = extended_profile_model.objects.get(user=user) + except ObjectDoesNotExist: + logger.info("No existing extended profile found for user %s, creating new instance", user.username) + + extended_profile_form = get_registration_extension_form(data=extended_profile_fields_data, **kwargs) + + return extended_profile_form + + except ImportError as e: + logger.warning("Extended profile model not available: %s", str(e)) + return None + 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"] = { + "developer_message": f"Error creating custom form: {str(e)}", + "user_message": _("There was an error processing the extended profile information"), + } + return None + + +def _validate_extended_profile_form_and_collect_errors(extended_profile_form: forms.Form, field_errors: dict) -> None: + """ + Validate the extended profile form and collect any validation errors. + + Args: + extended_profile_form (forms.Form): The extended profile form to validate + field_errors (dict): Dictionary to collect validation errors + """ + if not extended_profile_form.is_valid(): + logger.info("Extended profile form validation failed with errors: %s", extended_profile_form.errors) + + for field_name, field_errors_list in extended_profile_form.errors.items(): + first_error = field_errors_list[0] if field_errors_list else "Unknown error" + + field_errors[field_name] = { + "developer_message": f"Error in extended profile field {field_name}: {first_error}", + "user_message": str(first_error), + } + + def _validate_read_only_fields(user, data, field_errors): # Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400. read_only_fields = set(data.keys()).intersection( @@ -344,17 +487,52 @@ def _notify_language_proficiencies_update_if_needed(data, user, user_profile, ol ) -def _update_extended_profile_if_needed(data, user_profile): - if 'extended_profile' in data: +def _update_extended_profile_if_needed( + data: dict, user_profile: UserProfile, extended_profile_form: Optional[forms.Form] +) -> None: + """ + Update the extended profile information if present in the data. + + This function handles two types of extended profile updates: + 1. Updates the user profile meta fields with extended_profile data + 2. Saves the extended profile form data to the extended profile model if valid + + Args: + data (dict): Dictionary containing the update data, may include 'extended_profile' key + user_profile (UserProfile): The UserProfile instance to update + extended_profile_form (Optional[forms.Form]): The validated extended profile form + containing extended profile data, or None if no extended profile form is provided + + Note: + If 'extended_profile' is present in data, the function will: + - Extract field_name and field_value pairs from extended_profile list + - Update the user_profile.meta dictionary with new values + - Save the updated user_profile + + If extended_profile_form is provided and valid, the function will: + - Save the form data to the extended profile model + - Associate the model instance with the user if it's a new instance + - Log any errors that occur during the save process + """ + if "extended_profile" in data: meta = user_profile.get_meta() - new_extended_profile = data['extended_profile'] + new_extended_profile = data["extended_profile"] for field in new_extended_profile: - field_name = field['field_name'] - new_value = field['field_value'] + field_name = field["field_name"] + new_value = field["field_value"] meta[field_name] = new_value user_profile.set_meta(meta) user_profile.save() + if extended_profile_form: + try: + extended_profile_model = extended_profile_form.save(commit=False) + if not hasattr(extended_profile_model, "user") or extended_profile_model.user is None: + extended_profile_model.user = user_profile.user + extended_profile_model.save() + except Exception as e: # pylint: disable=broad-exception-caught + logger.error("Error saving extended profile model: %s", e) + def _update_state_if_needed(data, user_profile): # If the country was changed to something other than US, remove the state. diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index 360f4f65c7..fe27f63686 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -10,6 +10,7 @@ from django.conf import settings from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import ObjectDoesNotExist +from django.forms.models import model_to_dict from django.urls import reverse from rest_framework import serializers @@ -27,7 +28,9 @@ from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled from openedx.core.djangoapps.user_api.models import RetirementState, UserPreference, UserRetirementStatus from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin -from openedx.core.djangoapps.user_authn.views.registration_form import contains_html, contains_url +from openedx.core.djangoapps.user_authn.views.registration_form import ( + contains_html, contains_url, get_extended_profile_model +) from openedx.features.name_affirmation_api.utils import get_name_affirmation_service from . import ( @@ -569,26 +572,47 @@ def validate_new_name(self, new_name): raise serializers.ValidationError('Name cannot contain a URL') -def get_extended_profile(user_profile): +def get_extended_profile(user_profile: UserProfile) -> list[dict[str, str]]: """ - Returns the extended user profile fields stored in user_profile.meta + Retrieve extended user profile fields for API serialization. + + This function extracts custom profile fields that extend beyond the standard + UserProfile model. It first attempts to get data from a custom extended profile + model (if configured), then falls back to the user_profile.meta JSON field. + The returned data is filtered to include only fields specified in the + 'extended_profile_fields' site configuration. + + The function supports two data sources: + 1. Custom model: If `REGISTRATION_EXTENSION_FORM` setting points to a form with + a `Meta.model`, data is retrieved from that model using `model_to_dict()` + 2. Fallback: JSON data stored in `UserProfile.meta` field + + Args: + user_profile (UserProfile): The user profile instance to get extended fields from. + + 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) """ + def get_extended_profile_data(): + extended_profile_model = get_extended_profile_model() - # pick the keys from the site configuration - extended_profile_field_names = configuration_helpers.get_value('extended_profile_fields', []) + if extended_profile_model: + try: + profile_obj = extended_profile_model.objects.get(user=user_profile.user) + return model_to_dict(profile_obj) + except (AttributeError, extended_profile_model.DoesNotExist): + return {} - try: - extended_profile_fields_data = json.loads(user_profile.meta) - except ValueError: - extended_profile_fields_data = {} + try: + return json.loads(user_profile.meta or "{}") + except (ValueError, TypeError, AttributeError): + return {} - extended_profile = [] - for field_name in extended_profile_field_names: - extended_profile.append({ - "field_name": field_name, - "field_value": extended_profile_fields_data.get(field_name, "") - }) - return extended_profile + data = get_extended_profile_data() + field_names = configuration_helpers.get_value("extended_profile_fields", []) + return [{"field_name": name, "field_value": data.get(name, "")} for name in field_names] def get_profile_visibility(user_profile, user, configuration): diff --git a/openedx/core/djangoapps/user_authn/views/registration_form.py b/openedx/core/djangoapps/user_authn/views/registration_form.py index 79cf7dd598..6adbc7ba3e 100644 --- a/openedx/core/djangoapps/user_authn/views/registration_form.py +++ b/openedx/core/djangoapps/user_authn/views/registration_form.py @@ -6,6 +6,7 @@ from importlib import import_module from eventtracking import tracker import re +import logging from django import forms from django.conf import settings @@ -16,6 +17,8 @@ from django.urls import reverse from django.utils.translation import gettext as _ from django_countries import countries +from django.db.models import Model +from typing import Optional, Type from common.djangoapps import third_party_auth from common.djangoapps.edxmako.shortcuts import marketing_link @@ -37,6 +40,8 @@ validate_password, ) +logger = logging.getLogger(__name__) + class TrueCheckbox(widgets.CheckboxInput): """ @@ -312,6 +317,34 @@ def get_registration_extension_form(*args, **kwargs): return getattr(module, klass)(*args, **kwargs) +def get_extended_profile_model() -> Optional[Type[Model]]: + """ + Get the model class for the extended profile form. + + Returns the Django model class associated with the form specified in + the `REGISTRATION_EXTENSION_FORM` setting. + + Returns: + Optional[Type[Model]]: The model class if found and valid, None otherwise. + + Example: + # In settings.py: REGISTRATION_EXTENSION_FORM = 'myapp.forms.ExtendedForm' + model_class = get_extended_profile_model() + """ + setting_value = getattr(settings, "REGISTRATION_EXTENSION_FORM", None) + if not setting_value: + return None + + try: + module_path, klass_name = setting_value.rsplit(".", 1) + module = import_module(module_path) + form_class = getattr(module, klass_name) + return getattr(form_class.Meta, "model", None) + except (ValueError, ImportError, ModuleNotFoundError, AttributeError) as e: + logger.warning("Could not load extended profile model from '%s': %s", setting_value, e) + return None + + class RegistrationFormFactory: """ Construct Registration forms and associated fields. From 3f239e9ce0c9dd971ac9ab2989fb96186586078a Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Fri, 8 Aug 2025 17:26:53 -0500 Subject: [PATCH 2/3] fix: only updates the payload fields --- openedx/core/djangoapps/user_api/accounts/api.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 4f77e78696..275cb45995 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -514,12 +514,14 @@ def _update_extended_profile_if_needed( - Associate the model instance with the user if it's a new instance - Log any errors that occur during the save process """ - if "extended_profile" in data: + updated_fields = [] + new_extended_profile_fields = data.get("extended_profile") + if new_extended_profile_fields: meta = user_profile.get_meta() - new_extended_profile = data["extended_profile"] - for field in new_extended_profile: + for field in new_extended_profile_fields: field_name = field["field_name"] new_value = field["field_value"] + updated_fields.append(field_name) meta[field_name] = new_value user_profile.set_meta(meta) user_profile.save() @@ -529,7 +531,9 @@ def _update_extended_profile_if_needed( extended_profile_model = extended_profile_form.save(commit=False) if not hasattr(extended_profile_model, "user") or extended_profile_model.user is None: extended_profile_model.user = user_profile.user - extended_profile_model.save() + extended_profile_model.save() + else: + extended_profile_model.save(update_fields=updated_fields) except Exception as e: # pylint: disable=broad-exception-caught logger.error("Error saving extended profile model: %s", e) From 2962a329896b551f70c0d66983a38b30114d33dd Mon Sep 17 00:00:00 2001 From: Bryann Valderrama Date: Mon, 11 Aug 2025 12:47:30 -0500 Subject: [PATCH 3/3] fix: improve error handling in extended profile form processing --- .../core/djangoapps/user_api/accounts/api.py | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 275cb45995..cce59ef715 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -280,6 +280,9 @@ def _get_extended_profile_form_instance( user (User): User instance to associate with the extended profile field_errors (dict): Dictionary to collect validation errors if form creation fails + Raises: + AccountUpdateError: If there is an error creating the extended profile form + Returns: Optional[forms.Form]: Extended profile form instance with user data, or None if no extended profile form is configured or creation fails @@ -304,12 +307,7 @@ def _get_extended_profile_form_instance( logger.warning("Extended profile model not available: %s", str(e)) return None 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"] = { - "developer_message": f"Error creating custom form: {str(e)}", - "user_message": _("There was an error processing the extended profile information"), - } - return None + raise AccountUpdateError(f"Error creating custom form: {str(e)}") from e def _validate_extended_profile_form_and_collect_errors(extended_profile_form: forms.Form, field_errors: dict) -> None: @@ -319,17 +317,23 @@ def _validate_extended_profile_form_and_collect_errors(extended_profile_form: fo Args: extended_profile_form (forms.Form): The extended profile form to validate field_errors (dict): Dictionary to collect validation errors - """ - if not extended_profile_form.is_valid(): - logger.info("Extended profile form validation failed with errors: %s", extended_profile_form.errors) - for field_name, field_errors_list in extended_profile_form.errors.items(): - first_error = field_errors_list[0] if field_errors_list else "Unknown error" - - field_errors[field_name] = { - "developer_message": f"Error in extended profile field {field_name}: {first_error}", - "user_message": str(first_error), - } + Raises: + AccountUpdateError: If there is an error validating the extended profile form + """ + try: + if not extended_profile_form.is_valid(): + logger.info("Extended profile form validation failed with errors: %s", extended_profile_form.errors) + + for field_name, field_errors_list in extended_profile_form.errors.items(): + first_error = field_errors_list[0] if field_errors_list else "Unknown error" + + field_errors[field_name] = { + "developer_message": f"Error in extended profile field {field_name}: {first_error}", + "user_message": str(first_error), + } + except Exception as error: # pylint: disable=broad-exception-caught + raise AccountUpdateError(f"Error thrown when validating extended profile form: '{str(error)}'") from error def _validate_read_only_fields(user, data, field_errors):