Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable custom_reg_form capability #256

Merged
merged 1 commit into from
Jan 12, 2024
Merged

Conversation

johanseto
Copy link
Contributor

@johanseto johanseto commented Nov 20, 2023

Description

Enable the custom_reg_form. This is a feature of upstream openedx, and maybe for people like opencraft would like.
The field could be used using the REGISTRATION_EXTENSION_FORM setting. https://github.com/openedx/edx-platform/blob/master/openedx/core/djangoapps/user_authn/views/register.py#L205

Useful information to include:
This is like a cherrypick of this PR: #256
Also, some logic has to be updated.
To know how this work with more explanation check this PR.
#253

Testing instructions

#253

Additional information

[Include anything else that will help reviewers and consumers understand the change.

Checklist for Merge

  • Tested in a remote environment
  • Updated documentation
  • Rebased master/main
  • Squashed commits

@andrey-canon andrey-canon requested review from Alec4r, MaferMazu and a team November 23, 2023 15:16
@johanseto johanseto force-pushed the jlc/add-custom-reg-form branch 5 times, most recently from 8f0dd25 to 7cc9505 Compare November 23, 2023 15:56
The capability could be used enabled using the `REGISTRATION_EXTENSION_FORM` setting.

This is a feature of upstream openedx, the platform has that capability of registration
extension models and maybe for people like opencraft would like.

refs: #253
@MaferMazu
Copy link
Contributor

Hello, @johanv26, thanks for this feature. Only for better understanding:

  • What is the current behavior around the custom registration forms?
  • How does this PR modify the current behavior?
  • Why do we want this?

@MaferMazu MaferMazu mentioned this pull request Nov 23, 2023
7 tasks
@MaferMazu
Copy link
Contributor

I tested it by adding in my tenant the following:

"EDNX_CUSTOM_REGISTRATION_FIELDS": [
        {
            "label": "arabicname",
            "name": "arabic_name",
            "type": "text"
        },
        {
            "label": "Organization name",
            "name": "org_name",
            "type": "text"
        }
    ],
   "extended_profile_fields": [
        "org_name",
        "arabic_name"
    ]
}

And then I made a request to create a user... and it created the user with the info in the meta... but I didn't need to set the REGISTRATION_EXTENSION_FORM. Is it the expected behavior?

cc @johanv26

@johanseto
Copy link
Contributor Author

Hey @MaferMazu I think that is the expected behavior.
What is the current behavior around the custom registration forms?
Custom reg form is something related extension in order to save extra custom fields defined in a model. This extra data would be saved in that custom_reg_form model.
This is an example.
eg one case use other field called arabic_name and is saved in a custom_reg_info model.
image

How does this PR modify the current behavior?
If you dont use the setting REGISTRATION_EXTENSION_FORM the post flow would be the same.

https://github.com/openedx/edx-platform/blob/open-release/palm.4/openedx/core/djangoapps/user_authn/views/registration_form.py#L298-L305
That function return None and then the function do_create_account
https://github.com/openedx/edx-platform/blob/open-release/palm.4/common/djangoapps/student/helpers.py#L674
has that default value.

Why do we want this?
Custom reg form is a feature that edx-platform upstream support. So if Someone has it and wants to create a user using eox-core, this feature allows him also to save the data in the custom_reg_form model if they have configured REGISTRATION_EXTENSION_FORM and the main function do_create_account hast that default value.
https://github.com/openedx/edx-platform/blob/open-release/palm.4/common/djangoapps/student/helpers.py#L674

"EDNX_CUSTOM_REGISTRATION_FIELDS": [
        {
            "label": "arabicname",
            "name": "arabic_name",
            "type": "text"
        }
]

Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

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

Thanks, @johanv26, looks good to me.

@bra-i-am bra-i-am self-requested a review December 1, 2023 14:04
Copy link
Contributor

@bra-i-am bra-i-am left a comment

Choose a reason for hiding this comment

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

Working as expected!

@bra-i-am
Copy link
Contributor

Hi everyone, should this PR be merged right now or is there something lacking?

@luisfelipec95
Copy link
Contributor

Hello @johanseto, should this PR be merged right now or is there something lacking?

@andrey-canon
Copy link
Contributor

Hi @luisfelipec95, @bra-i-am, this pr is ready and there is nothing to add, however we are not aware of your merging process (bumpversion, tags, merge commit ... etc) that's why we prefer that you merge this

@MaferMazu MaferMazu merged commit 05f2756 into master Jan 12, 2024
4 checks passed
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.

7 participants