Skip to content

Use registration field order when using a registration extension form - #26633

Merged
waheedahmed merged 1 commit into
openedx:masterfrom
igobranco:igobranco/registration-field-order-with-form
Sep 23, 2021
Merged

Use registration field order when using a registration extension form#26633
waheedahmed merged 1 commit into
openedx:masterfrom
igobranco:igobranco/registration-field-order-with-form

Conversation

@igobranco

@igobranco igobranco commented Feb 19, 2021

Copy link
Copy Markdown
Contributor

Description

On registration screen, make it possible to change the REGISTRATION_FIELD_ORDER when using a custom REGISTRATION_EXTENSION_FORM.

If you are managing an open edx installation probably you need to ask more information about your users on the registration form. For that you use a registration extension form. But when you configure that extension form you no longer can change the ordering of presentation of each field on the registration form. This pull request tries to fix that specific thing.

This change has impact for Developers and Operators that manage a custom open edx installation.

Example, with this configuration, we can

REGISTRATION_EXTRA_FIELDS:
    city: required
    confirm_email: hidden
    country: required
    gender: optional
    goals: optional
    honor_code: required
    level_of_education: optional
    mailing_address: hidden
    year_of_birth: optional
    terms_of_service: hidden
    data_authorization: required # custom registration field
    employment_situation: optional # custom registration field
    allow_newsletter: optional # custom registration field

REGISTRATION_FIELD_ORDER:
- email
- name
- username
- password
- data_authorization # custom registration field
- honor_code
- city
- country
- allow_newsletter # custom registration field
- gender
- year_of_birth
- level_of_education
- employment_situation # custom registration field
- goals

Required fields:
image
Optional fields:
image

@openedx-webhooks

openedx-webhooks commented Feb 19, 2021

Copy link
Copy Markdown

Thanks for the pull request, @igobranco! I've created OSPR-5632 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@natabene

Copy link
Copy Markdown
Contributor

@igobranco Thank you for the contribution! When you have a chance, could you please send in a signed Contributor Agreement mentioned in the earlier comment? You only need to do this once and we can then review all your future code contributions. Thanks

@natabene

Copy link
Copy Markdown
Contributor

@igobranco We now have the same agreement available in Docusign here, so you don't need to print. Please sign when you have a chance.

@natabene

Copy link
Copy Markdown
Contributor

@igobranco Thank you for joining the Open edX community! We have added your user to our records and can now review your code going forward.

@natabene

Copy link
Copy Markdown
Contributor

jenkins run all

@natabene

Copy link
Copy Markdown
Contributor

@igobranco Could you please look into the failing tests?

@natabene

natabene commented Jun 1, 2021

Copy link
Copy Markdown
Contributor

jenkins run all

@natabene

natabene commented Jun 1, 2021

Copy link
Copy Markdown
Contributor

@wajeeha-khalid Could you please provide early feedback once you have a chance?

@wajeeha-khalid

Copy link
Copy Markdown
Contributor

@waheedahmed ^^ review required
I don't see any product concern on this as far as it does not impact the default order of fields

@igobranco

Copy link
Copy Markdown
Contributor Author

@natabene and @waheedahmed thank you with your information. So I'm going to try to fix the tests and update this pull request.
On previous months, I hadn't have much time to review this PR. I'm kind of new to django, so I had a problems running the tests, but now I think that could run them.

@igobranco

Copy link
Copy Markdown
Contributor Author

jenkins run all

1 similar comment
@natabene

natabene commented Jun 2, 2021

Copy link
Copy Markdown
Contributor

jenkins run all

@arch-bom-gocd-alerts

Copy link
Copy Markdown

📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build.

We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master.

This branch has been detected to not have commit 2e33565 as an ancestor. Here's how to see for yourself:

git merge-base --is-ancestor 2e335653 igobranco/registration-field-order-with-form && echo "You're all set" || echo "Please rebase onto master or merge master to your branch"

If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal).

@igobranco

Copy link
Copy Markdown
Contributor Author

Hi @natabene finally the jenkins didn't end with my errors or other errors ! I've been fetching from upstream a couple of time because the last builds stopped with completed strange errors. Can you help so anyone from edX could validate the changes ?

@natabene

Copy link
Copy Markdown
Contributor

@waheedahmed Could you validate please?

@waheedahmed

Copy link
Copy Markdown
Contributor

@natabene sure, taking a look.

@waheedahmed waheedahmed left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@natabene I have verified and tested these changes locally and looks good to me.

@waheedahmed

Copy link
Copy Markdown
Contributor

@igobranco Thank you for making these changes! Please squash your commits and make sure your branch is rebased with the latest master.

@gabor-boros

Copy link
Copy Markdown
Contributor

Hey @igobranco, are you still interested in this change? If not, I'd be happy to continue squashing and rebasing as we need this change too.

cc: @waheedahmed

@waheedahmed

Copy link
Copy Markdown
Contributor

@natabene Are we good to move forward and merge this PR or any further step is required here?

@igobranco
igobranco force-pushed the igobranco/registration-field-order-with-form branch from 53e8301 to baf0207 Compare September 22, 2021 14:00
@natabene

Copy link
Copy Markdown
Contributor

@waheedahmed If the author @igobranco is ready, no problem from my side.

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@igobranco

Copy link
Copy Markdown
Contributor Author

@waheedahmed I've updated the PR: rebased and squashed the commits.

cc @gabor-boros

@waheedahmed

Copy link
Copy Markdown
Contributor

Thank you @natabene and @igobranco!

Merging it now. Thanks again, everyone!

@waheedahmed
waheedahmed merged commit 6f0255b into openedx:master Sep 23, 2021
@openedx-webhooks

Copy link
Copy Markdown

@igobranco 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@alfredchavez

alfredchavez commented Oct 26, 2021

Copy link
Copy Markdown
Contributor

@gabor-boros could you port this changes to lilac.master? as part of BB-4877
CC: @arbrandes for review

gabor-boros pushed a commit to open-craft/openedx-platform that referenced this pull request Nov 15, 2021
gabor-boros added a commit to open-craft/openedx-platform that referenced this pull request Nov 15, 2021
…to registration form (#441)

* feat: add `optional-exposed` extra field type to registration form

This defines optional extra fields that are not hidden under the toggle on the
registration page.

(cherry picked from commit 230795f)
Signed-off-by: Gabor Boros <gabor.brs@gmail.com>

* Fix use a registration field order when using a registration extension form (openedx#26633)

(cherry-picked from commit 6f0255b)

Co-authored-by: Ivo Branco <igobranco@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants