Skip to content

Conversation

@BryanttV
Copy link
Contributor

@BryanttV BryanttV commented Aug 6, 2025

Description

This PR is a copy of this PR in upstream. For the time being, the changes will be applied directly in this fork until the upstream merge is performed.

Related Issues

https://github.com/fccn/nau-technical/issues/394

Related PRS

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. Follow the steps in this PR to use the Account MFE with the extended profile fields, but instead of using the custom-form-app, we will use the nau-openedx-extensions app.

  4. Add the following setting to the Site Configurations > local.edly.io:8000

    {
        "extended_profile_fields": [
            "employment_situation",
            "allow_newsletter",
            "nif"
        ]
    }
  5. Install this branch of nau-openedx-extensions. You can use this setting in your config.yml of Tutor.

    OPENEDX_EXTRA_PIP_REQUIREMENTS:
    - git+https://github.com/fccn/nau-openedx-extensions.git@bav/add-nif-in-custom-reg-form
  6. Create a tutor inline plugin and enable it with tutor plugins enable nau-settings

    # nau-settings.yml
    name: nau-settings
    version: 1.0.0
    patches:
      openedx-common-settings: |
        REGISTRATION_EXTENSION_FORM = "nau_openedx_extensions.custom_registration_form.forms.NauUserExtendedForm"
  7. Run tutor dev launch

  8. Create a user in the platform.

  9. Go to the Account MFE in http://apps.local.edly.io:1997/account/

  10. Edit the NAU Custom fields (Employment Situation, NIF, and Allow Newsletter) in the Profile Information

  11. Check the new values stored in the NAU User Extended Model in the Django admin.

Demo

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

@BryanttV BryanttV force-pushed the bav/use-extended-profile-model-in-account-settings branch from 7f54242 to 2962a32 Compare August 11, 2025 19:58
@BryanttV BryanttV marked this pull request as ready for review August 11, 2025 22:57
@sandroscosta
Copy link

@igobranco I'm testing this locally but I would like your opinion on this PR.

@igobranco
Copy link
Member

@BryanttV I couldn't make it work on the MFE on the Redwood release.
Is this suppose to work? Shouldn't this require more patches to make the extended profile work on Redwood?
If yes, IMO we should just focus on upstream. Then on Teak backport was is still missing.

@BryanttV The focus should be always push to upstream. Until the PR openedx/edx-platform#37119 is merged upstream, IMO we shouldn't try to change it downstream.
We already have a forked customization, I don't want to from 1 forked to other. The important is to change upstream to lower on the long run the effort of upgrades.

@BryanttV Please review the upstream PR: openedx/edx-platform#37119

@paolacruzedunext can you ask @mariajgrimaldi to review that PR upstream?

Detailing on my tests:

I created: https://github.com/fccn/nau-tutor-configs/pull/197

I used both branches:

  • edx-platform -> bav/use-extended-profile-model-in-account-settings
  • nau-openedx-extensions -> bav/add-nif-in-custom-reg-form

I could make it still receive the extended_profile information on Account API.
The response of this API call http://lms.local.nau.fccn.pt:8000/api/user/v1/accounts/ibranco

    "extended_profile": [
        {
            "field_name": "employment_situation",
            "field_value": "Student"
        },
        {
            "field_name": "allow_newsletter",
            "field_value": false
        },
        {
            "field_name": "nif",
            "field_value": "828407487"
        }
    ],

IMO the problem is the MFE that it isn't working.

But the UI didn't show more fields:
image

Profile MFE

I could make it work the profile MFE on Redwood.
I think we don't have any custom code in it.
So we could activate it.
image

Account MFE

I found out that the account MFE has some brand, header, footer incompatibility on Readwood, the MFE won't start on runtime.

CC @sandroscosta

@sandroscosta
Copy link

sandroscosta commented Sep 26, 2025

@BryanttV
With the MFE it's working on my machine, even though it duplicates the field, probably because I didn't remove the previous extension or code. I'm still doing some extra tests with the MFE to check if my result is similar to @igobranco.

There are four points me and @igobranco agreed upon, after some quick chat over Teams:

  • We need to have this working on Teak for Authn, Profile and Account MFEs, always with the code that was merged upstream. We don't want to replicate this code in our instance, if it's already implemented in the official MFE.
  • We should backport it to Teak if we need to. For Teak, we think that our best option is to leave the legacy django screens behind and go all in with the MFEs, while guaranteeing that they give us the same functionality as the latter.
  • We need to have this functionality working on a multilang environment, so we need to have a way to translate field names and messages on the MFEs. If you need to, I'll create a ticket specific to this one subject.
  • We have to guarantee that custom messages are being shown when an error occurs, and not the default "something went wrong", as for some of these fields, context is incredibly important.

There are two tickets related to this

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.

4 participants