feat: use extended profile model in account settings

[efortish]

Backport of openedx/openedx-platform#37119, merged upstream on 2026-04-30 and not present in Teak.

Why we need it

Registration already writes the extension form's fields into both UserProfile.meta and the custom model, but account settings only ever read and wrote UserProfile.meta. A field edited on the account page never reached the model.

NAU hits this directly. The Phase 1 characterization fields (NIF, employment situation, NUTS, CAE4) live on NauUserExtendedModel, and the enrollment and content gates in fccn/nau-openedx-extensions#159 read them from there. Without this, a learner who is blocked, goes to the account page and fills the fields in stays blocked, because the value lands in UserProfile.meta while the gate looks at the model. They do exactly what they are told and nothing changes.

The fork used to bridge this with the NAU_STUDENT_ACCOUNT_* extension points, reverted in 2602e05. This is the upstream replacement for them: PROFILE_EXTENSION_FORM supersedes REGISTRATION_EXTENSION_FORM and makes the account settings API read and write the model. Sites still on the old setting keep working.

Note this also means the ExtendedProfileFieldsSlot from openedx/frontend-app-account#1254, which the NAU MFE already has, is not enough on its own: that slot renders the fields, but the component saves through the account API, which is what this PR fixes.

How to test

1. Set PROFILE_EXTENSION_FORM to a ModelForm over a custom model, for NAU that is nau_openedx_extensions.custom_registration_form.forms.NauUserExtendedForm.
2. Register a learner, leaving one of the extended fields empty.
3. Edit that field on the account settings page.
4. Check the custom model, not UserProfile.meta. Before this change the value only appeared in meta.

With fccn/nau-openedx-extensions#159 also in place, the end to end check is: a learner blocked from a course by a missing profile field can now unblock themselves from the account page.

Removing this later

This is a temporary patch. It can be dropped as soon as NAU is on a release that already contains #37119, which is the next migration to Verawood

[efortish]

Needed by fccn/nau-openedx-extensions#159, which adds the Phase 1 profile fields and the course access gate. That PR is complete on its own, but until this lands a blocked learner cannot unblock themselves: they fill the fields on the account page and the value goes to UserProfile.meta while the gate reads NauUserExtendedModel.

Context in fccn/nau-technical#954.

[ManuelStarDo]

Hello @efortish , I have tested this locally and found some regressions with this PR

Please analyze and let me know what you think, take note that the FIX comments are merely suggestions, I am happy to hear your thoughts on them.

Even if you fix some of the Findings on the future ARTE project PRs, i cannot merge this PR as is as it would block future unrelated deploys on the platform

Findings

[CRITICAL] openedx/core/djangoapps/user_authn/views/registration_form.py:341,355,404 — undefined name log used in three places, none of the file's imports define it

The file never does import logging / log = logging.getLogger(__name__) (unlike the sibling files this same PR touches — accounts/api.py and accounts/forms.py both correctly add logger = logging.getLogger(__name__)). get_registration_extension_form() calls log.warning(...) (line 341, deprecation notice) and log.error(...) (line 355, load failure), and get_extended_profile_model() calls log.warning(...) (line 404, load failure). All three raise NameError: name 'log' is not defined the moment they execute.

Risk: This is not a cold/rare path. I verified two independent, realistic ways to trigger it against the live environment:

1. Any site using only the deprecated REGISTRATION_EXTENSION_FORM (this is NAU's actual current production configuration — nau-tutor-configs/plugins/registration_config.yml sets REGISTRATION_EXTENSION_FORM and does not set PROFILE_EXTENSION_FORM). The fallback branch at line 341 fires on every call. Verified live:

   • GET /api/user/v1/account/registration/ (the endpoint the registration page/MFE calls to render form fields) → HTTP 500, traceback ending in NameError: name 'log' is not defined.
   • update_account_settings() with any extended_profile field submitted (including just leaving a field blank, since field_value truthiness is never checked before adding it to the data dict) → caught one level up in forms.py's broad except Exception, surfaces as AccountValidationError({'extended_profile': {'developer_message': "Error creating custom form: name 'log' is not defined", ...}}).

   This directly contradicts the PR's stated design goal, both in the commit message ("Sites on the old setting keep working") and in lms/envs/common.py's own new comment block ("REGISTRATION_EXTENSION_FORM is deprecated but will continue to work for backward compatibility").

2. Any site using the new, recommended PROFILE_EXTENSION_FORM with a plain forms.Form (i.e. a form with no inner Meta.model — the historical pattern for this exact extension point; the docstring's own referenced example app, http://github.com/open-craft/custom-form-app, is exactly this style, not a ModelForm). get_extended_profile_model()'s except (ValueError, ImportError, AttributeError) branch is meant to catch the AttributeError from form_class.Meta not existing and return None gracefully — but the log.warning(...) call inside that except block itself raises a new, unhandled NameError, which fully replaces/masks the exception being handled and propagates out uncaught. Verified live: both update_account_settings() and get_account_settings() (the GET /api/user/v1/accounts/{username}/ endpoint — i.e. the account page itself) raise all the way up to a generic UserAPIInternalError, an uncaught 500 with no graceful degradation at all — worse than case 1, because neither forms.py nor serializers.py wraps this particular call in a try/except.

Fix: Add near the top of the file, alongside the other imports:

import logging
...
log = logging.getLogger(__name__)

This alone is necessary but not sufficient — see the next finding, which this fix alone does not resolve.

---

[CRITICAL] openedx/core/djangoapps/user_api/accounts/forms.py:96-116 (get_extended_profile_form) — does not gate model-saving on PROFILE_EXTENSION_FORM specifically, so fixing the log bug alone still breaks the "old setting keeps working" guarantee

get_extended_profile_model() (in registration_form.py) is carefully written to check only PROFILE_EXTENSION_FORM and explicitly, by design, return None for REGISTRATION_EXTENSION_FORM-only sites (its own docstring: "This ensures backward compatibility: users of the old setting keep the old behavior"). But get_extended_profile_form() in forms.py does not honor that gating for the form itself — it calls the shared get_registration_extension_form() helper unconditionally, which does fall back to REGISTRATION_EXTENSION_FORM and happily returns a fully valid, save-able form instance regardless of which setting supplied it. The caller (api.py's _update_extended_profile_if_needed) then treats any non-None extended_profile_form as something to .save() to the model, with no check for which setting actually produced it.

Risk: I patched the missing log locally (verification-only, reverted afterward — see Verification section below) and re-ran the exact REGISTRATION_EXTENSION_FORM-only scenario NAU currently ships. The crash changes shape but does not go away:

• User with an existing extended-profile row (the normal case for NAU — NauUserExtendedForm is also used at registration time, so essentially every real user already has a NauUserExtendedModel row): get_extended_profile_model() returns None, so no instance kwarg is passed to the form, so get_registration_extension_form() builds a brand-new, unbound-to-any-row form instance from REGISTRATION_EXTENSION_FORM. Saving it attempts to INSERT a second row for a OneToOneField, which is unique — this raises a real IntegrityError, caught and re-raised as AccountUpdateError, returned to the user as an HTTP 400. Verified live.
• User with no existing row: verified live that a brand-new NauUserExtendedModel row gets silently created, even though the site's admin never configured PROFILE_EXTENSION_FORM and, per the PR's own documented contract, should get pure meta-only behavior with zero model interaction.

Either way, "sites still on the old setting keep working" is false, independent of the log typo above. This is a logic error, not a typo — forms.py needs to know whether the form it got back came from PROFILE_EXTENSION_FORM (safe to save to the model) or the deprecated REGISTRATION_EXTENSION_FORM fallback (should never touch the model, exactly like the pre-PR code).

Fix: get_extended_profile_form() should check get_extended_profile_model()'s result before deciding whether to attempt any model save, e.g.:

extended_profile_model = get_extended_profile_model()
if extended_profile_model is None:
    # No PROFILE_EXTENSION_FORM configured (or it's not model-backed) -- do not
    # attempt to build/save a model-backed form. This preserves the deprecated
    # REGISTRATION_EXTENSION_FORM's meta-only behavior exactly as documented.
    return None, {}

placed before the get_registration_extension_form(...) call, so that a REGISTRATION_EXTENSION_FORM-only configuration never reaches the model-save branch in api.py at all — restoring parity with the pre-PR behavior the commit message and common.py comments explicitly promise.

---

[HIGH] No test in this PR exercises the real (unmocked) get_registration_extension_form() / get_extended_profile_model() fallback logic — this is why 700+ new test lines didn't catch the above two CRITICAL bugs

Verified by reading every new/changed test file:

• test_forms.py (new, 323 lines): every test that reaches get_extended_profile_form does so via @patch("...forms.get_registration_extension_form") and/or @patch("...forms.get_extended_profile_model") — the real implementations are never called.
• test_api.py: test_update_extended_profile_with_meta_only is the one test that calls the real, unmocked update_account_settings() with extended_profile data — but I confirmed neither lms/envs/test.py nor cms/envs/test.py sets REGISTRATION_EXTENSION_FORM or PROFILE_EXTENSION_FORM, so both remain None in this test's settings, meaning the fallback branch (if setting_value: at line 340) is never entered and log.warning/log.error are never reached. I ran this exact test live (pytest .../test_api.py::TestAccountApi::test_update_extended_profile_with_meta_only) and confirmed it passes despite the code being fundamentally broken for the scenario its own docstring claims to cover ("legacy behavior").
• test_serializers.py has one @override_settings(REGISTRATION_EXTENSION_FORM=None) test, but it sets both settings to None/absent — it does not test "REGISTRATION_EXTENSION_FORM set to a real value, PROFILE_EXTENSION_FORM unset," which is the actual deprecated/backward-compat scenario the PR is supposed to support.
• test_utils.py imports get_extended_profile_model and get_registration_extension_form (plus Mock, Model, override_settings) but never calls or references any of them anywhere in the file (see LOW finding below) — dead code, not test coverage.
• There is no dedicated test_registration_form.py anywhere in the tree, before or after this PR.

Risk: The PR's entire "backward compatibility" claim is untested end-to-end. Any reviewer relying on "tests pass" for confidence would have been misled — as I initially was, until testing the real deprecated-setting-only scenario against a live LMS.

Fix: Add at least one test (ideally in a new test_registration_form.py, since these functions currently have zero direct tests) that does:

@override_settings(REGISTRATION_EXTENSION_FORM="path.to.SomeForm", PROFILE_EXTENSION_FORM=None)
def test_deprecated_setting_only_does_not_touch_model(self):
    ...

without mocking get_registration_extension_form/get_extended_profile_model themselves, and assert both that (a) no exception is raised, and (b) the custom model is untouched (only UserProfile.meta changes) — this test would have caught both CRITICAL findings above immediately.

---

[MEDIUM] openedx/core/djangoapps/user_api/accounts/api.py:32 — logger is declared but never used; the three new except blocks don't actually log anything despite the docstring claiming they do

logger = logging.getLogger(__name__) is added, but grepping the whole file confirms logger. is never called anywhere. The new except ValidationError, except IntegrityError, and except DatabaseError blocks in _update_extended_profile_if_needed (lines ~415-435) each re-raise as AccountUpdateError without ever calling logger.error(...) first — yet the function's own docstring explicitly states: "The error is logged and an AccountUpdateError is raised to the caller." That's not what the code does.

Risk: When an extended-profile save genuinely fails in production (e.g. a real IntegrityError from a duplicate value, or a transient DatabaseError), there will be zero server-side log record of it — only a generic 400 response reaches the client. This makes the exact failure mode from the CRITICAL findings above much harder to diagnose operationally, since the only evidence is the HTTP response, not the logs.

Fix: Add logger.error(...) (or logger.exception(...) to capture the traceback) at the top of each except block before constructing the AccountUpdateError, e.g.:

except IntegrityError as exc:
    logger.error("Extended profile integrity error for user %s: %s", user_profile.user.username, exc)
    raise AccountUpdateError(...)

---

[LOW] openedx/core/djangoapps/user_api/accounts/api.py and serializers.py hardcode the extended-profile model's user-linking field name as user

_update_extended_profile_if_needed (api.py) does if not hasattr(extended_profile, "user") or extended_profile.user is None: extended_profile.user = user_profile.user, and get_extended_profile (serializers.py) does extended_profile_model.objects.get(user=user_profile.user). Both assume the model's FK-to-User field is literally named user. The docstrings acknowledge this is just a convention ("typically named user"), not an enforced contract.

Risk: For NAU's own NauUserExtendedModel this happens to be correct (verified: field is named user), so this does not block this PR for NAU today. But this is shared Open edX platform code, not NAU-specific — for any other deployment whose custom model names the field differently, the read path (serializers.py) would raise an uncaught FieldError on every account-settings page load, and the write path (api.py) would silently set a non-existent Python attribute (a no-op assignment that Django won't persist), leaving the FK unset and producing a confusing IntegrityError (if not nullable) or silent orphaned data (if nullable) — with no clear error message pointing at the actual cause.

Fix: Either document this as a hard requirement in PROFILE_EXTENSION_FORM's settings docstring (currently phrased as "typically," which reads as a suggestion, not a requirement), or better, look up the FK field dynamically (e.g. via the model's _meta to find the field with related_model=User) instead of assuming the name user.

---

[LOW / SUGGESTION] openedx/core/djangoapps/user_api/accounts/api.py — missing blank line between top-level functions

Only one blank line separates the end of _update_extended_profile_if_needed (ends line 435) and def _update_state_if_needed (line 437), where the rest of the file consistently uses two (e.g. before _store_old_name_if_needed). Minor PEP8 (E302) inconsistency; no CI is configured on this branch to catch it (gh pr checks 47 reports no checks at all).

Fix: Add the missing blank line.

---

[LOW / SUGGESTION] openedx/core/djangoapps/user_authn/views/tests/test_utils.py — five new imports added but never used

The diff adds Mock (from unittest.mock), Model (from django.db.models), override_settings (from django.test.utils), and get_extended_profile_model / get_registration_extension_form (from registration_form) — none of these appear anywhere else in the file; the only other changes in this file are cosmetic (single→double quote reformatting) of pre-existing, unrelated username-generation tests. Looks like leftover scaffolding from an earlier draft where tests may have been intended for this file before being (correctly) placed in test_forms.py/test_serializers.py instead.

Fix: Remove the five unused imports, or add the tests they suggest were meant to accompany them (see the HIGH finding above — this file would in fact be a very natural home for direct, unmocked tests of get_registration_extension_form/get_extended_profile_model).

---

[LOW / SUGGESTION] 87 # noqa: PT009 suppressions added across the new/modified test files for a lint rule this repository doesn't enforce

PT009 (flake8-pytest-style: prefer plain assert over self.assertEqual-style methods) suppressions appear 87 times across test_api.py, test_forms.py, test_serializers.py, and test_views.py — all newly added by this PR. I confirmed neither setup.cfg nor pyproject.toml in this repository configures PT009 (or flake8-pytest-style/ruff PT rules) at all, and no other file anywhere in the codebase (before or after this PR) uses this suppression. This looks like it was carried over mechanically from the upstream PR's lint environment without checking whether it's relevant here.

Fix: Not blocking — harmless dead comments — but could be stripped for cleanliness since they suppress a rule that isn't active in this repo.

---

[LOW / SUGGESTION] openedx/core/djangoapps/user_api/accounts/forms.py:100 — except AttributeError in get_extended_profile_form is broader than intended

try:
    kwargs["instance"] = extended_profile_model.objects.get(user=user)
except AttributeError:
    logger.info("No extended profile model configured")

This is meant to catch the specific case of extended_profile_model being None (so None.objects raises AttributeError). But it will just as silently catch and mislabel any other AttributeError (e.g. a malformed model class missing .objects for an unrelated reason) with the same generic "No extended profile model configured" message, obscuring the real cause.

Fix: Check if extended_profile_model is None: explicitly instead of relying on catching AttributeError from calling .objects on None.

---

Verification process

• Full local environment: nau-tutor-configs on nau/teak.master, PR #47 branch mounted at /openedx/edx-platform in dev mode, LMS/CMS healthy (/heartbeat → 200, modulestore/sql OK), using NAU's actual shipped registration_config.yml (REGISTRATION_EXTENSION_FORM only, PROFILE_EXTENSION_FORM unset).
• Confirmed the PR's advertised behavior works when PROFILE_EXTENSION_FORM is properly configured with NAU's real NauUserExtendedForm (a ModelForm): a full write→read round trip via update_account_settings() → get_extended_profile() correctly persists to and reads from NauUserExtendedModel, not just UserProfile.meta.
• Reproduced the NameError: name 'log' is not defined crash three independent ways: (1) direct Django-shell call to update_account_settings(), (2) live HTTP GET /api/user/v1/account/registration/ → 500, (3) PROFILE_EXTENSION_FORM pointed at a plain forms.Form.
• Ran the PR's own test_update_extended_profile_with_meta_only test live via pytest — confirmed it passes, and confirmed why (test settings never set either extension-form setting, so the buggy branch is never exercised).
• Applied a minimal local, verification-only patch (added the missing import logging / log = logging.getLogger(__name__)) purely to determine whether that alone would fix things — confirmed it does not: the deeper model-vs-meta gating bug then surfaces instead, as either an IntegrityError-turned-AccountUpdateError (existing-row case) or a silently created model row (no-existing-row case). This patch was reverted immediately after verification; no changes were committed or pushed anywhere.

[rguerra-fccn]

Verdict: Request changes. I agree with @ManuelStarDo's CHANGES_REQUESTED review and independently reproduced both of his CRITICAL findings live. I also found an additional CRITICAL issue he didn't cover, and — after reading through fccn/nau-technical#954 and its linked PRs — I don't think this PR is actually needed to unblock ARTE Phase 1 in the way the issue thread implies.

Confirmed live: both of Manuel's CRITICAL findings are real

Tested against a local dev stack running NAU's actual shipped config (REGISTRATION_EXTENSION_FORM only, PROFILE_EXTENSION_FORM unset — matches nau-tutor-configs' registration_config.yml today, and still matches it after companion PR nau-tutor-configs#318, see below):

1. log is undefined in registration_form.py (no import logging/logging.getLogger anywhere in the file).
   • GET /api/user/v1/account/registration/ → HTTP 500, NameError: name 'log' is not defined at registration_form.py:341.
   • update_account_settings() with any extended_profile field → AccountValidationError: "Error creating custom form: name 'log' is not defined".
2. get_extended_profile_form() (forms.py) never gates on get_extended_profile_model() before treating the form as save-able, so it doesn't actually respect "old setting keeps old behavior." Verified both sub-cases (with log patched locally, verification-only, not committed):
   • User with an existing NauUserExtendedModel row → AccountUpdateError: Extended profile integrity error: (1062, "Duplicate entry ... for key ...user_id").
   • User without an existing row → a NauUserExtendedModel row is silently created, even though PROFILE_EXTENSION_FORM was never configured — directly contradicting the commit message's and lms/envs/common.py's "sites on the old setting keep working" claim.

Manuel's suggested fixes (add the missing logger; gate get_extended_profile_form on get_extended_profile_model() is None before attempting a save) look correct and necessary to me too.

Additional CRITICAL finding: this PR removes a currently-working NAU mechanism, not just fails to fix a broken one

The PR's premise is: "account settings only ever read and wrote UserProfile.meta... A field edited on the account page never reached the model."

This is false for NAU's actual nau/teak.master. I tested update_account_settings() directly on the base branch (pre-PR):

update_account_settings(user, {"extended_profile": [{"field_name": "nif", "field_value": "100000070"}]}, username=username)
# -> no exception; NauUserExtendedModel.nif == "100000070"; UserProfile.meta == {"nif": "100000070"}

nau/teak.master already has a working bridge: api.py calls run_extension_point('NAU_STUDENT_ACCOUNT_PARTIAL_UPDATE', ...), wired by nau-openedx-extensions to context_extender.partial_update, which writes straight to the model. The read-side equivalent (NAU_STUDENT_SERIALIZER_CONTEXT_EXTENSION → context_extender.update_account_serializer) does the same for reads.

The commit message says this bridge "was reverted in 2602e05eb" — but git merge-base --is-ancestor 2602e05eb nau/teak.master shows that commit is not an ancestor of this branch (possibly from a different eduNEXT client's fork). The commit that added the bridge (e1dc8fd1fb) is very much live in nau/teak.master's history and has never been reverted here.

This PR deletes both run_extension_point call sites (api.py, serializers.py) outright. nau-openedx-extensions#159 doesn't touch context_extender.py or the NAU_STUDENT_* settings, so this isn't a coordinated migration — that module's functions would become orphaned, and the working write/read path would simply stop firing, replaced by the broken path above. No test in this PR (or anywhere in openedx-platform) exercises the NAU_STUDENT_* extension points, so CI wouldn't catch this either.

Ask: please confirm whether 2602e05eb refers to a different deployment, and whether retiring nau-openedx-extensions' context_extender.py bridge is intentional here — if so, that cleanup (and confirming PROFILE_EXTENSION_FORM covers everything NAU_ACCOUNTS_CC_VISIBLE_FIELDS-based filtering did) needs to land together with this change, not as a follow-up.

Context from fccn/nau-technical#954 — this doesn't change the assessment, and may make this PR unnecessary right now

I checked the tracking issue and its two companion PRs (nau-openedx-extensions#159, nau-tutor-configs#318) to see if anything there changes NAU's config in a way that avoids the scenarios above. It doesn't — nau-tutor-configs#318 leaves REGISTRATION_EXTENSION_FORM exactly as-is and never sets PROFILE_EXTENSION_FORM, so the failure scenarios reproduced above are exactly what would run in production once Phase 1 ships, with or without this PR.

More importantly: the existing bridge's field visibility is controlled by a different setting, NAU_ACCOUNTS_CC_VISIBLE_FIELDS (default ["employment_situation", "nif", "allow_newsletter"]), which neither #159 nor #318 update. The bridge's write path has no field allow-list at all, so nuts/cae4 already save correctly through it today; only the visibility in the account-settings UI/API needs nuts/cae4 added to NAU_ACCOUNTS_CC_VISIBLE_FIELDS — a one-line config change in nau-tutor-configs, with none of this PR's regression risk.

Given that, I don't think this needs to be merged urgently to unblock Phase 1 — the same practical outcome (nuts/cae4 editable via account settings) looks achievable today via that config change, using the mechanism that's already proven to work. I'd suggest decoupling the two: ship Phase 1 via the small NAU_ACCOUNTS_CC_VISIBLE_FIELDS addition, and let this backport get the fixes above plus a full test pass before merging on its own timeline.

(Side note: #318's description states as verified testing evidence that "fields are editable in the account settings MFE and persist correctly" — given the above, that's most likely exercising the existing bridge, not this PR in its current state. Worth clarifying with @efortish which code path was actually under test.)

Minor / non-blocking, in addition to Manuel's LOW findings

• The hardcoded assumption that the extended-profile model's user FK is named user (Manuel flagged this in api.py/serializers.py) also appears a third time: forms.py:115, get_extended_profile_form()'s extended_profile_model.objects.get(user=user).
• serializers.py's new get_extended_profile() docstring claims values are "converted to string", but the model-backed branch returns model_to_dict(profile_obj) unmodified — e.g. a BooleanField would come back as a Python bool, not a string. Low risk for NAU today (all NauUserExtendedModel fields used here are CharField/TextField/BooleanField), but worth tightening on shared platform code.
• Worth flagging for whoever eventually wires PROFILE_EXTENSION_FORM for NAU: any field in the custom form's Meta.fields becomes writable via the account-settings API with no separate allow-list — for NauUserExtendedForm that includes data_authorization (the privacy-policy consent flag). Confirm that's intentional before enabling PROFILE_EXTENSION_FORM, since it would let a user silently re-flip consent post-registration without re-showing the policy text.