Skip to content

feat: use extended profile model in account settings#37119

Open
BryanttV wants to merge 33 commits intoopenedx:masterfrom
eduNEXT:bav/use-extended-profile-model-in-account-settings
Open

feat: use extended profile model in account settings#37119
BryanttV wants to merge 33 commits intoopenedx:masterfrom
eduNEXT:bav/use-extended-profile-model-in-account-settings

Conversation

@BryanttV
Copy link
Contributor

@BryanttV BryanttV commented Aug 4, 2025

Description

This PR enhances the extended profile fields functionality, enabling the use of custom extended profile models in account settings with improved consistency, atomicity, and error handling.

Currently, when we have extended profile fields associated with the REGISTRATION_EXTENSION_FORM setting, it works in this way:

  1. During Registration: Additional custom fields are stored in both the meta field of the UserProfile model AND in the custom model associated with the form. See the registration code.
  2. In Account Settings (before this PR): Custom fields are only fetched from and stored in the meta field of the UserProfile model, ignoring the custom model entirely.

What This PR Changes

This PR introduces several improvements:

  1. New Setting PROFILE_EXTENSION_FORM: Introduces a new Django setting PROFILE_EXTENSION_FORM that supersedes the deprecated REGISTRATION_EXTENSION_FORM. The new setting enables:

    • More accurate naming (profile extension vs registration extension)
    • Enhanced capabilities for managing extended profile fields
    • Data storage in dedicated model (not just UserProfile.meta)
    • Ability to update extended profile fields via Account Settings API
      Migration Path: Sites currently using REGISTRATION_EXTENSION_FORM (deprecated) will continue working with the old behavior (data stored in UserProfile.meta, no Account Settings updates). To get the new capabilities, migrate to PROFILE_EXTENSION_FORM.
  2. Dual Storage: Extended profile fields are now stored in both the custom model (when configured) and the meta field, maintaining parity with the registration process.

  3. Atomic Updates: Both meta and custom model updates are performed within a single database transaction. If either operation fails, both are rolled back to prevent partial/inconsistent saves.

  4. Consistent Reading: Extended profile data is read from the custom model when PROFILE_EXTENSION_FORM is configured with a model, falling back to meta only when no model is configured.

Important Behavior Notes

  • Migration Consideration: When PROFILE_EXTENSION_FORM is configured with a model, extended profile data is read only from that model. If a user has existing data in meta but no corresponding model record, their extended profile fields will appear empty until their profile is updated (which will then create the model record and populate it from meta).

  • Validation: When a form is configured, all extended profile updates go through that form's validation logic, including any model-level validators.

Testing instructions

  1. Create a Tutor environment.

  2. Create a mount of edx-platform with the changes in this PR.

    tutor mounts add edx-platform
  3. Add the following property to the LMS Django Admin > Site Configurations > local.openedx.io:8000

    {
        "extended_profile_fields": [
            "nickname",
            "interests",
            "wants_newsletter",
            "favorite_language"
        ]
    }
  4. Install this app that includes the custom extra fields mentioned above: https://github.com/bryanttv/custom-extra-fields. You can use this setting in your config.yml

    OPENEDX_EXTRA_PIP_REQUIREMENTS:
    - git+https://github.com/bryanttv/custom-extra-fields.git
  5. Create a tutor inline plugin and enable it with tutor plugins enable custom-settings

    # custom-settings.yml
    name: custom-settings
    version: 1.0.0
    patches:
      openedx-common-settings: |
        PROFILE_EXTENSION_FORM = "custom_extra_fields.forms.CustomExtraFieldsForm"
  6. Run tutor dev launch

  7. Create a user in the platform.

  8. Go to {lms_domain}/api/user/v1/accounts/{username} in your browser. Be sure to use the application/merge-patch+json Media type in the DRF UI.

Test Cases

1. No Form (PROFILE_EXTENSION_FORM = None) and no extended profile fields

The response includes the extended_profile key as an empty list. This list cannot be updated because no fields are defined in the extended_profile_fields setting, and there is no extended form either.

{
    "extended_profile": []
}

2. No Form (PROFILE_EXTENSION_FORM = None) and with extended profile fields

The fields defined in the extended_profile_fields setting are returned and can be updated. However, these fields do not have any validation, meaning any value can be assigned to them. Data is stored in the UserProfile.meta JSON field.

{
    "extended_profile": [
        {
            "field_name": "nickname",
            "field_value": "any_value_allowed"
        },
        {
            "field_name": "interests",
            "field_value": "Programming"
        },
        {
            "field_name": "wants_newsletter",
            "field_value": "true"
        },
        {
            "field_name": "favorite_language",
            "field_value": "spanish"
        }
    ]
}

3. With Form (PROFILE_EXTENSION_FORM = "myapp.forms.ExtendedForm") and no extended profile fields

Fields defined in the form will be displayed in the legacy registration form if they are marked as required. However, if the extended_profile_fields property in the Site Configuration is not defined, no fields will be returned from the endpoint.

Still, the defined fields can be updated and will be validated according to the logic in the form. Updates are transactional: both meta and the custom model (if configured) are updated atomically, or both fail together.

  • If the form is associated with a model, additional validations from the model will be executed, and its fields will be updated as well. Data for the extended profile fields will also be extracted from that model.
{
    "extended_profile": []
}

4. With Form (PROFILE_EXTENSION_FORM = "myapp.forms.ExtendedForm") and with extended profile fields

When both settings are defined, the fields listed in extended_profile_fields are returned, each with its corresponding validation as defined in the form.

  • If the form is associated with a model, additional validations from that model will be applied, and its fields will be updated accordingly. Data for the extended profile fields will be sourced from the model (not from meta).
  • Updates are atomic: if validation fails or the save operation errors, no partial data is persisted.
{
    "extended_profile": [
        {
            "field_name": "nickname",
            "field_value": "codewanderer"
        },
        {
            "field_name": "interests",
            "field_value": "Programming"
        },
        {
            "field_name": "wants_newsletter",
            "field_value": "true"
        },
        {
            "field_name": "favorite_language",
            "field_value": "python"
        }
    ]
}

5. Testing Atomic Rollback (Error Handling)

To verify that failed updates don't leave partial data:

  1. Configure a custom form with validation that can fail (e.g., a field with choices)
  2. Attempt to update extended profile fields with invalid data
  3. Verify that an AccountUpdateError is raised
  4. Verify that neither the meta field nor the custom model was updated (rollback worked)

Deadline

None

Demo

extended-profile-fields-in-account-settings.mp4

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 4, 2025
@openedx-webhooks
Copy link

openedx-webhooks commented Aug 4, 2025

Thanks for the pull request, @BryanttV!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@openedx-webhooks
Copy link

Thanks for the pull request, @BryanttV!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Update the status of your PR

Your PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate.


Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@github-project-automation github-project-automation bot moved this to Needs Triage in Contributions Aug 4, 2025
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch 2 times, most recently from 12561c1 to dc1cee2 Compare August 5, 2025 17:35
@BryanttV BryanttV marked this pull request as ready for review August 5, 2025 18:10
@BryanttV BryanttV requested review from a team as code owners August 5, 2025 18:10
@BryanttV BryanttV requested a review from mariajgrimaldi August 5, 2025 18:11
@mphilbrick211 mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Aug 5, 2025
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from dc1cee2 to 04cdfb7 Compare August 6, 2025 22:53
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch 2 times, most recently from dae3705 to 065faee Compare August 11, 2025 17:48
@efortish efortish self-requested a review September 5, 2025 18:38
Copy link
Contributor

@efortish efortish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the contribution @BryanttV
Everything looks good, I followed your instructions step by step and it went great!

Can we add some edge-case tests suit for the new features in api.py and registration_form.py?

@mphilbrick211 mphilbrick211 moved this from Ready for Review to In Eng Review in Contributions Sep 16, 2025
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from 065faee to 06c8680 Compare September 30, 2025 01:17
@openedx-webhooks openedx-webhooks added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Sep 30, 2025
@BryanttV
Copy link
Contributor Author

Thanks for the review, @efortish! I will be working on the unit tests for this functionality.

Comment on lines +602 to +615
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 {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section made me question something: is the profile meta and the extended profile model out of date at some point? Should they be in sync?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every time we update the extended profile fields, they will be updated in both the model and the meta, so they should always be synchronized.

_send_email_change_requests_if_needed(update, user)


def _get_and_validate_extended_profile_form(update: dict, user, field_errors: dict) -> Optional[forms.Form]:
Copy link
Member

@deborahgu deborahgu Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super happy about having form specific code in api.py. For that matter, I'm also not super happy about having form validators in api.py. That seems like form and view and validator code to me. api.py should be dealing with abstract data models, in my opinion.

If you can point to prior art where we do this, especially recent prior art, I'm willing to change my mind about that, but this really seems like, well...not dependency inversion, but dependency reversal. Your form.py should import from the API, not vice versa.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @deborahgu, thanks for your comment! I made some changes according to your suggestions. Could you check again, please?

@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from 06c8680 to 4e25200 Compare November 11, 2025 23:10
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch 2 times, most recently from f234f96 to b6b08c9 Compare November 21, 2025 17:43
@BryanttV BryanttV marked this pull request as draft November 21, 2025 20:26
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from f15cab3 to 750e00a Compare November 24, 2025 18:46
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from b9b26cc to cc79b5d Compare March 3, 2026 22:39
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from cc79b5d to 350b9c1 Compare March 3, 2026 23:03
@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from 9ea2a62 to cecbb7c Compare March 3, 2026 23:24
Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BryanttV here are the rest of the comments from the AI assisted review. Let me know if anything is unclear.


if not field_name:
logger.warning("Missing field_name in extended_profile field_data: %s", field_data)
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Entries where field_value is null are silently skipped. A client sending {"field_name": "title", "field_value": null} expecting to clear a field will get no error and no change. If clearing a field isn't supported, this should return a validation error. If it is supported (or intentionally deferred to the model form), that should be documented here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can see, the current behavior accepts any value, including null. Should we keep this behavior? I would say yes, and let the associated form handle the validation. Commit: 53a9b59

extended_profile_fields_data (dict): Extended profile field data to populate the form
user (User): User instance to associate with the extended profile

Returns:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says "The validated form instance", but this function can return an invalid form when is_valid() fails — the invalid form is returned alongside the errors dict. The caller handles this correctly (view returns 400 when form_errors is non-empty), but the docstring is misleading. Consider saying "The form instance (may be invalid if field_errors is non-empty)" or returning None when the form is invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that’s correct. I updated the docstring to make it clearer. Commit: a0c077a

"""
field_errors = {}

try:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code: get_extended_profile_model() catches all its own exceptions internally and always returns None on failure — it never raises. This except ImportError branch will never execute and can be removed.

# Simplify to just:
extended_profile_model = get_extended_profile_model()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, applied! Commit: 8379ee2

field_errors[field_name] = {
"developer_message": f"Error in extended profile field {field_name}: {first_error}",
"user_message": str(first_error),
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This returns the invalid form to the caller when validation fails (see above on the docstring issue). Since the caller uses form_errors to gate on whether to proceed, the invalid form object itself is harmless today — but it's a subtle footgun if any future caller doesn't check form_errors first.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree, changes applied! Commit: 4c5750d

extended_profile = extended_profile_form.save(commit=False)
if not hasattr(extended_profile, "user") or extended_profile.user is None:
extended_profile.user = user_profile.user
# Now persist the instance with the user field properly set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errors during the model save are caught and logged but not re-raised. Because the meta write (user_profile.save() above) already committed outside this savepoint, a failure here leaves meta updated but the model record stale — a silent partial save.

This appears intentional given test_update_extended_profile_form_save_error explicitly asserts the meta update still succeeds when the form save fails. If so, the docstring's Note should be updated to say so explicitly, e.g. "If the model save fails, the error is logged and the meta update is preserved (not rolled back)."

This also has the side effect that the meta values and the values in the model will get out of sync. Since we've now updated the code to read only from the model, this could end up hiding information we had intended to expose to the user. I don't necessarily want us to update the extended profile model on read but perhaps it wolud be good to merge the data from meta and the extended model on read (prioritizing the value in the model) to not lose data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the best approach is to include both operations within the same transaction so there’s no risk of the values getting out of sync. Commit: aaa1327

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says the function "falls back to user_profile.meta" but that fallback (lines below) only happens when no model is configured at all. If a model is configured but the user has no record yet, the code returns {} — there is no fallback to meta.

This means users who have existing data in user_profile.meta but no model record yet will see their extended profile fields disappear as soon as REGISTRATION_EXTENSION_FORM is set. If this is the intended migration behaviour, it should be documented explicitly here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the docstring. Commit: 0307656


try:
extended_profile_fields_data = json.loads(user_profile.meta)
except ValueError:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching AttributeError here is overly broad. Since extended_profile_model is guaranteed non-None at this point (guarded by if extended_profile_model: above), an AttributeError on .DoesNotExist shouldn't happen for a well-formed Django model. This broad catch could silently mask unrelated bugs. Consider catching only extended_profile_model.DoesNotExist.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied, commit: 1636f4b

@BryanttV
Copy link
Contributor Author

BryanttV commented Mar 4, 2026

Hi @feanil, thanks for your additional comments!

I updated the implementation to include the new PROFILE_EXTENSION_FORM setting, and I addressed your latest comments. Could you take another look? Thanks!

I also marked the REGISTRATION_EXTENSION_FORM setting as deprecated. Do you think we should go ahead and open the DEPR ticket now?

@BryanttV BryanttV requested a review from feanil March 4, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Status: In Eng Review

Development

Successfully merging this pull request may close these issues.

7 participants