Skip to content

[SE-4888] Cherry-pick Fix use a registration field order when using a registration extension - #419

Merged
gabor-boros merged 1 commit into
opencraft-release/koa.3from
gabor/cherry-pick-field-order
Oct 1, 2021
Merged

[SE-4888] Cherry-pick Fix use a registration field order when using a registration extension#419
gabor-boros merged 1 commit into
opencraft-release/koa.3from
gabor/cherry-pick-field-order

Conversation

@gabor-boros

@gabor-boros gabor-boros commented Oct 1, 2021

Copy link
Copy Markdown

This PR cherry-picks https://github.com/edx/edx-platform/pull/26633

Test instructions

  1. Navigate here
  2. Validate that the text " I hereby confirm receiving updates on new courses, content and new learning initiatives " follows "Password" field.

@gabor-boros gabor-boros changed the title Cherry-pick Fix use a registration field order when using a registration extension [SE-4888] Cherry-pick Fix use a registration field order when using a registration extension Oct 1, 2021

@tinuademargaret tinuademargaret left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@gabor How can I test this?


custom_form = get_registration_extension_form()
if custom_form:
custom_form_field_names = [field_name for field_name, field in custom_form.fields.items()]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we could have done something like this here
custom_form_field_names = list(custom_form.fields.keys())
what do you think?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I agree, this could be simplified, though it is already merged on edX master and possibly I wouldn't make an unnecessary code drift.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sure! sounds reasonable.

@gabor

gabor commented Oct 1, 2021

Copy link
Copy Markdown

@gabor How can I test this?

you probably need @gabor-boros 😄

@gabor-boros

Copy link
Copy Markdown
Author

@gabor How can I test this?

@tinumide Give me some monies and I'll update the description 😊

@tinuademargaret

tinuademargaret commented Oct 1, 2021

Copy link
Copy Markdown

@gabor How can I test this?

you probably need @gabor-boros 😄

So sorry to bother you

@tinuademargaret

Copy link
Copy Markdown

@gabor How can I test this?

@tinumide Give me some monies and I'll update the description 😊

would you mind 2 BTCs?

@gabor-boros

Copy link
Copy Markdown
Author

would you mind 2 BTCs?

I meant minutes, but my autocorrect won. Sorry 😅 🤦

@tinuademargaret tinuademargaret left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

  • I tested this PR according to the test instructions and confirmed that it functions well
  • I read through the code
  • I checked for accessibility issues NA
  • Includes documentation NA


custom_form = get_registration_extension_form()
if custom_form:
custom_form_field_names = [field_name for field_name, field in custom_form.fields.items()]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sure! sounds reasonable.

@tinuademargaret

Copy link
Copy Markdown

@gabor-boros thanks for the good work 👍.. This is good to go

@gabor-boros
gabor-boros merged commit b98bcd0 into opencraft-release/koa.3 Oct 1, 2021
@gabor-boros
gabor-boros deleted the gabor/cherry-pick-field-order branch October 1, 2021 13:18
@gabor-boros

Copy link
Copy Markdown
Author

Thank you @tinumide for the quick review!

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