Skip to content

revert: restore the NAU account settings extension points - #48

Open
efortish wants to merge 1 commit into
nau/teak.masterfrom
nau/restore-account-extension-points
Open

revert: restore the NAU account settings extension points#48
efortish wants to merge 1 commit into
nau/teak.masterfrom
nau/restore-account-extension-points

Conversation

@efortish

Copy link
Copy Markdown
Contributor

Reverts 2602e05, which removed the three extension points the account settings path called:

Extension point Where What it does
NAU_STUDENT_ACCOUNT_CONTEXT_EXTENSION settings_views.account_settings_context Renders the NAU fields on the account page
NAU_STUDENT_SERIALIZER_CONTEXT_EXTENSION serializers.UserReadOnlySerializer Returns their current values
NAU_STUDENT_ACCOUNT_PARTIAL_UPDATE api.update_account_settings Saves them

run_extension_point itself was never removed and is still live (lms/djangoapps/certificates/views/webview.py uses it), so this only puts the three call sites back.

Why

The account page is the only place a learner can fill or correct the NAU extended profile fields (nif, employment_situation, nuts, cae4) after registering.

ARTE (nau-technical#954) makes those fields part of the data NAU has to collect, and a course can block access until they are filled. Right now a learner hits that block and has nowhere to go: registration is the only path that writes to the model, and they already registered.

Why not the upstream replacement

Upstream replaced this pattern with PROFILE_EXTENSION_FORM (openedx/openedx-platform#37119), which landed after the Teak cut. We backported it in #47 and found two problems that make it unusable as it stands:

It writes every field on the extension model. That includes the ten cc_* fields populated from Autenticação Gov and the data_authorization privacy consent. get_registration_extension_form() serves both registration and account settings from the same setting, so one Meta.fields governs both paths and narrowing it for the account page also narrows registration, where data_authorization is required.

A partial update blanks everything else. The form is bound with the submitted data plus the existing row, so any field absent from the request validates as empty and is saved as empty. Sending only nuts through update_account_settings, on a real account:

antes    nif:123456789  cc_nif:987654321  cc_first_name:Kel  consent:True
despues  nif:None       cc_nif:None       cc_first_name:''   consent:False

Neither problem exists on this path. get_fields() in the plugin filters by the NAU_ACCOUNTS_CC_VISIBLE_FIELDS allowlist, so cc_* and the consent flag are never exposed unless someone lists them, and partial_update only setattrs the fields that actually arrive, so editing one field leaves the rest alone.

Not enough on its own

The plugin side was removed to match and has to come back too. That is in nau-openedx-extensions#159, which restores custom_registration_form/context_extender.py and the settings that point at it, and adds nuts and cae4 to the allowlist. Site configuration for NAU_ACCOUNTS_CC_VISIBLE_FIELDS goes in nau-tutor-configs#318.

With only this PR merged nothing changes: run_extension_point returns early when the setting is unset.

How to test

  1. Deploy this together with the plugin branch from nau-openedx-extensions#159.
  2. Set NAU_ACCOUNTS_CC_VISIBLE_FIELDS to ["employment_situation", "nif", "allow_newsletter", "nuts", "cae4"].
  3. Open the account page as a learner. The five fields render, choice fields as dropdowns.
  4. Change one of them and save. Check NauUserExtendedModel for that user: the edited field changed, the others kept their values, data_authorization untouched.
  5. Remove nif from the allowlist and reload. The field disappears from the page and from the serializer payload.

What happens to #47

It stays open for now. If this lands, the PROFILE_EXTENSION_FORM backport is not needed for ARTE and we would rather close it than carry a fork divergence, but that is worth deciding together.

…lds on the accounts page""

This restores the three extension points the account settings path used to
call: NAU_STUDENT_ACCOUNT_CONTEXT_EXTENSION to render the NAU fields,
NAU_STUDENT_SERIALIZER_CONTEXT_EXTENSION to return their current values, and
NAU_STUDENT_ACCOUNT_PARTIAL_UPDATE to save them.

We need them back because the account page is the only place a learner can
fill or correct the NAU extended profile fields (nif, employment_situation,
nuts, cae4) after registering. ARTE (nau-technical#954) requires those fields
and lets a course block access until they are filled, so a learner who is
blocked today has nowhere to go and fill them.

The upstream replacement for this, PROFILE_EXTENSION_FORM (openedx PR 37119),
landed after the Teak cut. We tried backporting it in #47 and found two
problems that make it unusable as it stands:

1. It writes every field on the extension model, including the ten cc_* fields
   that come from Autenticacao Gov and the data_authorization privacy consent.
   There is no way to narrow that for the account page without also narrowing
   registration, because one setting serves both paths.
2. The form is bound with the submitted data plus the existing row, so any
   field not present in the request is treated as empty and saved as empty.
   Sending only "nuts" wipes nif, cc_nif, cc_first_name and flips
   data_authorization to False. Verified against a real account.

The extension points have neither problem. get_fields() in the plugin filters
by the NAU_ACCOUNTS_CC_VISIBLE_FIELDS allowlist, so cc_* and the consent flag
are never exposed unless someone lists them, and partial_update only setattrs
the fields that actually arrive, so an edit to one field leaves the rest
alone.

This reverts commit 2602e05.
@efortish efortish changed the title Restore the NAU account settings extension points revert: restore the NAU account settings extension points Sep 11, 2026
efortish added a commit to fccn/nau-openedx-extensions that referenced this pull request Sep 11, 2026
Restores context_extender.py and the settings that point at it, undoing
2ae1d9d. We removed them when the platform side was reverted in the fork
(fccn/openedx-platform 2602e05ebf), which left registration as the only way to
write to NauUserExtendedModel. That is a problem now that a course can block
access until the fields are filled: a learner who already registered has
nowhere to go. The platform side comes back in fccn/openedx-platform#48.

nuts and cae4 join the default NAU_ACCOUNTS_CC_VISIBLE_FIELDS so the four ARTE
fields are all editable.

Two changes on top of the plain revert:

partial_update now checks the allowlist before writing. It used to setattr
whatever field_name arrived in the request, so an account PATCH could set any
field on the model, including the ten cc_* fields that come from Autenticacao
Gov and the data_authorization consent flag. get_fields already filtered the
read side, this makes the write side match.

Its body moved out of the finally block. It was only there to share the
instance lookup, and with an early return in it any exception raised by the
save would have been swallowed.

SCORMXBLOCK_ASYNC_THRESHOLD stays. 2ae1d9d happened to replace that line, so
reverting it automatically would have taken the setting with it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant