Skip to content

fix: Validation error for "empty" Batch Enrollment, Batch Beta Tester Addition fields is absent - #32684

Merged
abdullahwaheed merged 2 commits into
openedx:masterfrom
DmytroAlipov:fix-error-for-empty-list-enrollment-master
Mar 21, 2024
Merged

fix: Validation error for "empty" Batch Enrollment, Batch Beta Tester Addition fields is absent#32684
abdullahwaheed merged 2 commits into
openedx:masterfrom
DmytroAlipov:fix-error-for-empty-list-enrollment-master

Conversation

@DmytroAlipov

Copy link
Copy Markdown
Contributor

Description

[Instructor Dashboard > Membership] Validation error for "empty" Batch Enrollment, Batch Beta Tester Addition fields is absent

Steps to Reproduce:

  1. Go to the Instructor Dashboard > Membership
  2. Leave input field for Batch Enrollment empty
  3. Click on [Enroll] button

screen_14

screen_15

Another bug found:

  • if you add invalid users to Batch Enrollment:
screen_12
  • if you add invalid users to Batch Beta Tester Addition:
screen_13

Now everything works correctly:

screen_16 screen_17

@openedx-webhooks

openedx-webhooks commented Jul 7, 2023

Copy link
Copy Markdown

Thanks for the pull request, @DmytroAlipov! 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.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 7, 2023
@DmytroAlipov
DmytroAlipov force-pushed the fix-error-for-empty-list-enrollment-master branch 2 times, most recently from 9dc8985 to 36115db Compare July 13, 2023 05:57
@DmytroAlipov DmytroAlipov changed the title fix: Validation error for "empty" Batch Enrollment, Batch Beta Tester Addition fields is absent WIP: fix: Validation error for "empty" Batch Enrollment, Batch Beta Tester Addition fields is absent Jul 18, 2023
@DmytroAlipov
DmytroAlipov force-pushed the fix-error-for-empty-list-enrollment-master branch from 36115db to a168884 Compare July 18, 2023 11:02
@DmytroAlipov DmytroAlipov changed the title WIP: fix: Validation error for "empty" Batch Enrollment, Batch Beta Tester Addition fields is absent fix: Validation error for "empty" Batch Enrollment, Batch Beta Tester Addition fields is absent Jul 18, 2023
@mphilbrick211

Copy link
Copy Markdown

hi @DmytroAlipov! Is this ready for review?

@DmytroAlipov

Copy link
Copy Markdown
Contributor Author

@mphilbrick211 yes ;)

@mphilbrick211

Copy link
Copy Markdown

Hi @openedx/fed-bom! Is this something you could help review/merge for us? Thanks!

@abdullahwaheed

Copy link
Copy Markdown
Contributor

sure @mphilbrick211 we'll review it

@abdullahwaheed
abdullahwaheed self-requested a review August 8, 2023 07:28
@mphilbrick211 mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Aug 23, 2023

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.

Suggested change
renderList(gettext('This field must not be blank'), [])
renderList(gettext('This field must not be blank'), []);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Suggested change
gettext('Users must create and activate their account before they can be promoted to beta tester.')
gettext('Users must create and activate their account before they can be promoted to beta tester.');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@abdullahwaheed I disagree, this gettext is inside unshift

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Perhaps you meant ; on the next line

Comment on lines 715 to 721

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.

Suggested change
invalidIdentifier.length === 0 &&
errors.length === 0 &&
enrolled.length === 0 &&
allowed.length === 0 &&
autoenrolled.length === 0 &&
notenrolled.length === 0 &&
notunenrolled.length === 0
invalidIdentifier.length === 0
&& errors.length === 0
&& enrolled.length === 0
&& allowed.length === 0
&& autoenrolled.length === 0
&& notenrolled.length === 0
&& notunenrolled.length === 0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Suggested change
renderList(gettext('This field must not be blank'), [])
renderList(gettext('This field must not be blank'), []);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@DmytroAlipov
DmytroAlipov force-pushed the fix-error-for-empty-list-enrollment-master branch from a168884 to 71034b8 Compare September 6, 2023 17:40
@DmytroAlipov

Copy link
Copy Markdown
Contributor Author

I don't understand why the CMS test failed. The backed part is not affected at all in this PR.

@DmytroAlipov

Copy link
Copy Markdown
Contributor Author

In the Palm branch, everything was successful for the same PR

@mphilbrick211 mphilbrick211 removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Sep 6, 2023
@mphilbrick211 mphilbrick211 added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Sep 18, 2023
@DmytroAlipov
DmytroAlipov force-pushed the fix-error-for-empty-list-enrollment-master branch 2 times, most recently from 890bbed to e78a1c7 Compare October 4, 2023 09:36
@DmytroAlipov
DmytroAlipov force-pushed the fix-error-for-empty-list-enrollment-master branch from e78a1c7 to 35f0844 Compare October 4, 2023 10:13
@mphilbrick211

Copy link
Copy Markdown

Hi @abdullahwaheed! Are you able to merge this for us?

@mphilbrick211

Copy link
Copy Markdown

Hi @abdullahwaheed! Are you able to merge this for us?

Hi @abdullahwaheed - friendly follow-up on this :)

@mphilbrick211 mphilbrick211 removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Nov 16, 2023
@mphilbrick211

Copy link
Copy Markdown

Hi @abdullahwaheed! Are you able to merge this for us?

Hi @abdullahwaheed - friendly follow-up on this :)

Hi @abdullahwaheed and @openedx/2u-fed-bom! Would someone able to please merge this for us?

@abdullahwaheed
abdullahwaheed merged commit 79f6223 into openedx:master Mar 21, 2024
@openedx-webhooks

Copy link
Copy Markdown

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

@DmytroAlipov
DmytroAlipov deleted the fix-error-for-empty-list-enrollment-master branch March 21, 2024 09:38
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

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

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants