eduNEXT / eox-core

eox-core is a plugin to extend the core functionality in Open edX
GNU Affero General Public License v3.0
15 stars 9 forks source link

feat: Make extra registration fields optional in during edxapp account creation #222

Closed magajh closed 1 year ago

magajh commented 2 years ago

Description

In this PR we make some changes to the create_edxapp_user() in order to make the extra registration fields optional when registering a new user un edxapp.

Three conditions are checked in order to be able to skip these extra registration fields:

  1. The skip_extra_registration_fields field is sent in the data for the account creation as True.
  2. The user making the request is staff.
  3. The data received in the parameters does not contain any of the REGISTRATION_EXTRA_FIELDS configured for the microsite.

In case any of these conditions are not met, the function will request the extra registration fields to be sent for the creation of the new user.

Why are these changes necessary?

In PR #223 we are implementing an endpoint that creates a new OAuth Application. We need to create a user in the platform to set it as the owner of the new edxapp application and want to use the create_edxapp_user() function for this. However, this function requires to send the extra registration fields configured for the site during the account creation, so we are making these changes to be able to register a user only with the basic information such as fullname, username and email, without having to worry about other registration fields.

Testing instructions

The best way to test this change is through the endpoint that creates an Oauth application, implemented in PR #223, since we are not sending any of the extra registration fields during the creation of the user that will be the owner of the application. On the contrary, if you try to send only these fields

{
    "username": "mariecurie",
    "email": "mariecurie@example.com",
    "fullname": "Marie Curie",
}

In the endpoint that handles the creation of a new user, you will get form errors for not sending the extra registration fields.

Checklist for Merge

Alec4r commented 2 years ago

@magajh Maybe we need new unit tests or update the unit tests? if not, This looks good for me.

magajh commented 1 year ago

@Alec4r I initially thought about updating the tests, but then I checked and there aren't any for the backend modules. I think this is ready for merge