Skip to content

Commit b6b08c9

Browse files
committed
refactor: separate extended profile form validation from api layer
1 parent b22af20 commit b6b08c9

3 files changed

Lines changed: 160 additions & 149 deletions

File tree

openedx/core/djangoapps/user_api/accounts/api.py

Lines changed: 8 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,7 @@
4040
)
4141
from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences
4242
from openedx.core.djangoapps.user_authn.utils import check_pwned_password
43-
from openedx.core.djangoapps.user_authn.views.registration_form import (
44-
get_extended_profile_model, get_registration_extension_form, validate_name, validate_username
45-
)
43+
from openedx.core.djangoapps.user_authn.views.registration_form import validate_name, validate_username
4644
from openedx.core.lib.api.view_utils import add_serializer_errors
4745
from openedx.features.enterprise_support.utils import get_enterprise_readonly_account_fields
4846
from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed
@@ -115,7 +113,7 @@ def get_account_settings(request, usernames=None, configuration=None, view=None)
115113

116114

117115
@helpers.intercept_errors(errors.UserAPIInternalError, ignore_errors=[errors.UserAPIRequestError])
118-
def update_account_settings(requesting_user, update, username=None):
116+
def update_account_settings(requesting_user, update, username=None, extended_profile_form=None):
119117
"""Update user account information.
120118
121119
Note:
@@ -128,6 +126,7 @@ def update_account_settings(requesting_user, update, username=None):
128126
update (dict): The updated account field values.
129127
username (str): Optional username specifying which account should be updated. If not specified,
130128
`requesting_user.username` is assumed.
129+
extended_profile_form (Optional[forms.Form]): Optional validated extended profile form instance.
131130
132131
Raises:
133132
errors.UserNotFound: no user with username `username` exists (or `requesting_user.username` if
@@ -172,7 +171,6 @@ def update_account_settings(requesting_user, update, username=None):
172171

173172
old_name = _validate_name_change(user_profile, update, field_errors)
174173
old_language_proficiencies = _get_old_language_proficiencies_if_updating(user_profile, update)
175-
extended_profile_form = _get_and_validate_extended_profile_form(update, user, field_errors)
176174

177175
if field_errors:
178176
raise errors.AccountValidationError(field_errors)
@@ -200,143 +198,6 @@ def update_account_settings(requesting_user, update, username=None):
200198
_send_email_change_requests_if_needed(update, user)
201199

202200

203-
def _get_and_validate_extended_profile_form(update_data: dict, user: User, field_errors: dict) -> Optional[forms.Form]:
204-
"""
205-
Get and validate the extended profile form if it exists in the update.
206-
207-
Args:
208-
update_data (dict): The update data containing potential extended_profile fields
209-
user (User): The user instance for whom the extended profile form is being validated
210-
field_errors (dict): Dictionary to collect field validation errors
211-
212-
Returns:
213-
Optional[forms.Form]: The validated extended profile form instance,
214-
or None if no extended profile form is needed
215-
"""
216-
extended_profile = update_data.get("extended_profile")
217-
if not extended_profile:
218-
return None
219-
220-
extended_profile_fields_data = _extract_extended_profile_fields_data(extended_profile, field_errors)
221-
if not extended_profile_fields_data:
222-
return None
223-
224-
extended_profile_form = _get_extended_profile_form_instance(extended_profile_fields_data, user, field_errors)
225-
if not extended_profile_form:
226-
return None
227-
228-
_validate_extended_profile_form_and_collect_errors(extended_profile_form, field_errors)
229-
230-
return extended_profile_form
231-
232-
233-
def _extract_extended_profile_fields_data(extended_profile: Optional[list], field_errors: dict) -> dict:
234-
"""
235-
Extract extended profile fields data from extended_profile structure.
236-
237-
Args:
238-
extended_profile (Optional[list]): List of field data dictionaries
239-
field_errors (dict): Dictionary to collect validation errors
240-
241-
Returns:
242-
dict: Extracted custom fields data
243-
"""
244-
if not isinstance(extended_profile, list):
245-
field_errors["extended_profile"] = {
246-
"developer_message": "extended_profile must be a list",
247-
"user_message": _("Invalid extended profile format"),
248-
}
249-
return {}
250-
251-
extended_profile_fields_data = {}
252-
253-
for field_data in extended_profile:
254-
if not isinstance(field_data, dict):
255-
logger.warning("Invalid field_data structure in extended_profile: %s", field_data)
256-
continue
257-
258-
field_name = field_data.get("field_name")
259-
field_value = field_data.get("field_value")
260-
261-
if not field_name:
262-
logger.warning("Missing field_name in extended_profile field_data: %s", field_data)
263-
continue
264-
265-
if field_value is not None:
266-
extended_profile_fields_data[field_name] = field_value
267-
268-
return extended_profile_fields_data
269-
270-
271-
def _get_extended_profile_form_instance(
272-
extended_profile_fields_data: dict, user: User, field_errors: dict
273-
) -> Optional[forms.Form]:
274-
"""
275-
Get or create an extended profile form instance.
276-
277-
Attempts to create a form instance using the configured `REGISTRATION_EXTENSION_FORM`.
278-
If an extended profile model exists, tries to bind to existing user data or creates
279-
a new instance. Handles import errors and missing configurations gracefully.
280-
281-
Args:
282-
extended_profile_fields_data (dict): Extended profile field data to populate the form
283-
user (User): User instance to associate with the extended profile
284-
field_errors (dict): Dictionary to collect validation errors if form creation fails
285-
286-
Returns:
287-
Optional[forms.Form]: Extended profile form instance with user data, or None if
288-
no extended profile form is configured or creation fails
289-
"""
290-
try:
291-
extended_profile_model = get_extended_profile_model()
292-
except ImportError as e:
293-
logger.warning("Extended profile model not available: %s", str(e))
294-
return None
295-
296-
kwargs = {}
297-
298-
try:
299-
kwargs["instance"] = extended_profile_model.objects.get(user=user)
300-
except AttributeError:
301-
logger.info("No extended profile model configured")
302-
except ObjectDoesNotExist:
303-
logger.info("No existing extended profile found for user %s, creating new instance", user.username)
304-
305-
try:
306-
extended_profile_form = get_registration_extension_form(data=extended_profile_fields_data, **kwargs)
307-
except Exception as e: # pylint: disable=broad-exception-caught
308-
logger.error("Unexpected error creating custom form for user %s: %s", user.username, str(e))
309-
field_errors["extended_profile"] = {
310-
"developer_message": f"Error creating custom form: {str(e)}",
311-
"user_message": _("There was an error processing the extended profile information"),
312-
}
313-
return None
314-
315-
return extended_profile_form
316-
317-
318-
def _validate_extended_profile_form_and_collect_errors(extended_profile_form: forms.Form, field_errors: dict) -> None:
319-
"""
320-
Validate the extended profile form and collect any validation errors.
321-
322-
Args:
323-
extended_profile_form (forms.Form): The extended profile form to validate
324-
field_errors (dict): Dictionary to collect validation errors
325-
"""
326-
if extended_profile_form.is_valid():
327-
return
328-
329-
logger.info("Extended profile form validation failed with errors: %s", extended_profile_form.errors)
330-
331-
for field_name, field_errors_list in extended_profile_form.errors.items():
332-
first_error = field_errors_list[0] if field_errors_list else "Unknown error"
333-
334-
field_errors[field_name] = {
335-
"developer_message": f"Error in extended profile field {field_name}: {first_error}",
336-
"user_message": str(first_error),
337-
}
338-
339-
340201
def _validate_read_only_fields(user, data, field_errors):
341202
# Check for fields that are not editable. Marking them read-only causes them to be ignored, but we wish to 400.
342203
read_only_fields = set(data.keys()).intersection(
@@ -500,7 +361,7 @@ def _update_extended_profile_if_needed(
500361
501362
This function handles two types of extended profile updates:
502363
1. Updates the user profile meta fields with extended_profile data
503-
2. Saves the extended profile form data to the extended profile model if valid
364+
2. Saves the extended profile form data to the extended profile model if a validated form is provided
504365
505366
Args:
506367
data (dict): Dictionary containing the update data, may include 'extended_profile' key
@@ -509,12 +370,12 @@ def _update_extended_profile_if_needed(
509370
containing extended profile data, or None if no extended profile form is provided
510371
511372
Note:
512-
If 'extended_profile' is present in data, the function will:
513-
- Extract field_name and field_value pairs from extended_profile list
514-
- Update the user_profile.meta dictionary with new values
373+
If `extended_profile` is present in data, the function will:
374+
- Extract `field_name` and `field_value` pairs from extended_profile list
375+
- Update the `user_profile.meta` dictionary with new values
515376
- Save the updated user_profile
516377
517-
If extended_profile_form is provided and valid, the function will:
378+
If `extended_profile_form` is provided and valid, the function will:
518379
- Save the form data to the extended profile model
519380
- Associate the model instance with the user if it's a new instance
520381
- Log any errors that occur during the save process

openedx/core/djangoapps/user_api/accounts/forms.py

Lines changed: 142 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,21 @@
22
Django forms for accounts
33
"""
44

5+
import logging
6+
from typing import Optional, Tuple
57

68
from django import forms
7-
from django.core.exceptions import ValidationError
9+
from django.core.exceptions import ObjectDoesNotExist, ValidationError
10+
from django.utils.translation import gettext as _
811

12+
from common.djangoapps.student.models import User
913
from openedx.core.djangoapps.user_api.accounts.utils import handle_retirement_cancellation
14+
from openedx.core.djangoapps.user_authn.views.registration_form import (
15+
get_extended_profile_model,
16+
get_registration_extension_form,
17+
)
18+
19+
logger = logging.getLogger(__name__)
1020

1121

1222
class RetirementQueueDeletionForm(forms.Form):
@@ -35,3 +45,134 @@ def save(self, retirement):
3545
raise ValidationError('Retirement is in the wrong state!')
3646

3747
handle_retirement_cancellation(retirement)
48+
49+
50+
def extract_extended_profile_fields_data(extended_profile: Optional[list]) -> Tuple[dict, dict]:
51+
"""
52+
Extract extended profile fields data from extended_profile structure.
53+
54+
Args:
55+
extended_profile (Optional[list]): List of field data dictionaries with keys
56+
'field_name' and 'field_value'
57+
58+
Returns:
59+
tuple: A tuple containing (extended_profile_fields_data, field_errors)
60+
- extended_profile_fields_data (dict): Extracted custom fields data
61+
- field_errors (dict): Dictionary of validation errors, if any
62+
"""
63+
field_errors = {}
64+
65+
if not isinstance(extended_profile, list):
66+
field_errors["extended_profile"] = {
67+
"developer_message": "extended_profile must be a list",
68+
"user_message": _("Invalid extended profile format"),
69+
}
70+
return {}, field_errors
71+
72+
extended_profile_fields_data = {}
73+
74+
for field_data in extended_profile:
75+
if not isinstance(field_data, dict):
76+
logger.warning("Invalid field_data structure in extended_profile: %s", field_data)
77+
continue
78+
79+
field_name = field_data.get("field_name")
80+
field_value = field_data.get("field_value")
81+
82+
if not field_name:
83+
logger.warning("Missing field_name in extended_profile field_data: %s", field_data)
84+
continue
85+
86+
if field_value is not None:
87+
extended_profile_fields_data[field_name] = field_value
88+
89+
return extended_profile_fields_data, field_errors
90+
91+
92+
def get_extended_profile_form(extended_profile_fields_data: dict, user: User) -> Tuple[Optional[forms.Form], dict]:
93+
"""
94+
Get and validate an extended profile form instance.
95+
96+
Args:
97+
extended_profile_fields_data (dict): Extended profile field data to populate the form
98+
user (User): User instance to associate with the extended profile
99+
100+
Returns:
101+
tuple: A tuple containing (extended_profile_form, field_errors)
102+
- extended_profile_form (Optional[forms.Form]): The validated form instance, or None if
103+
no extended profile form is configured or creation fails
104+
- field_errors (dict): Dictionary of validation errors, if any
105+
"""
106+
field_errors = {}
107+
108+
try:
109+
extended_profile_model = get_extended_profile_model()
110+
except ImportError as e:
111+
logger.warning("Extended profile model not available: %s", str(e))
112+
return None, field_errors
113+
114+
kwargs = {}
115+
116+
try:
117+
kwargs["instance"] = extended_profile_model.objects.get(user=user)
118+
except AttributeError:
119+
logger.info("No extended profile model configured")
120+
except ObjectDoesNotExist:
121+
logger.info("No existing extended profile found for user %s, creating new instance", user.username)
122+
123+
try:
124+
extended_profile_form = get_registration_extension_form(data=extended_profile_fields_data, **kwargs)
125+
except Exception as e: # pylint: disable=broad-exception-caught
126+
logger.error("Unexpected error creating custom form for user %s: %s", user.username, str(e))
127+
field_errors["extended_profile"] = {
128+
"developer_message": f"Error creating custom form: {str(e)}",
129+
"user_message": _("There was an error processing the extended profile information"),
130+
}
131+
return None, field_errors
132+
133+
if not extended_profile_form.is_valid():
134+
logger.info("Extended profile form validation failed with errors: %s", extended_profile_form.errors)
135+
136+
for field_name, field_errors_list in extended_profile_form.errors.items():
137+
first_error = field_errors_list[0] if field_errors_list else "Unknown error"
138+
139+
field_errors[field_name] = {
140+
"developer_message": f"Error in extended profile field {field_name}: {first_error}",
141+
"user_message": str(first_error),
142+
}
143+
144+
return extended_profile_form, field_errors
145+
146+
147+
def validate_and_get_extended_profile_form(
148+
extended_profile_data: list, user: User
149+
) -> Tuple[Optional[forms.Form], dict]:
150+
"""
151+
Validate and return an extended profile form instance.
152+
153+
This function orchestrates the extraction and validation of extended profile data.
154+
155+
Args:
156+
extended_profile_data (list): The raw extended_profile data from the API request
157+
user (User): The user instance for whom the extended profile is being validated
158+
159+
Returns:
160+
tuple: A tuple containing (validated_form, field_errors)
161+
- validated_form (Optional[forms.Form]): The validated form instance, or None if
162+
validation fails or no extended profile is configured
163+
- field_errors (dict): Dictionary of validation errors, if any
164+
"""
165+
extended_profile_fields_data, field_errors = extract_extended_profile_fields_data(extended_profile_data)
166+
167+
if field_errors:
168+
return None, field_errors
169+
170+
if not extended_profile_fields_data:
171+
return None, {}
172+
173+
extended_profile_form, form_errors = get_extended_profile_form(extended_profile_fields_data, user)
174+
175+
if form_errors:
176+
field_errors.update(form_errors)
177+
178+
return extended_profile_form, field_errors

openedx/core/djangoapps/user_api/accounts/views.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
UserRetirementStatus,
8282
)
8383
from .api import get_account_settings, update_account_settings
84+
from .forms import validate_and_get_extended_profile_form
8485
from .permissions import (
8586
CanCancelUserRetirement,
8687
CanDeactivateUser,
@@ -408,9 +409,17 @@ def partial_update(self, request, username):
408409
):
409410
return Response({"error": "Too many requests"}, status=status.HTTP_429_TOO_MANY_REQUESTS)
410411

412+
extended_profile_form = None
413+
if "extended_profile" in request.data:
414+
extended_profile_form, form_errors = validate_and_get_extended_profile_form(
415+
request.data["extended_profile"], request.user
416+
)
417+
if form_errors:
418+
return Response({"field_errors": form_errors}, status=status.HTTP_400_BAD_REQUEST)
419+
411420
try:
412421
with transaction.atomic():
413-
update_account_settings(request.user, request.data, username=username)
422+
update_account_settings(request.user, request.data, username, extended_profile_form)
414423
account_settings = get_account_settings(request, [username])[0]
415424
except UserNotAuthorized:
416425
return Response(status=status.HTTP_403_FORBIDDEN)

0 commit comments

Comments
 (0)