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

[HS-1211838] Allow editing incomplete registrations #858

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

canac
Copy link
Contributor

@canac canac commented Aug 22, 2024

We received a HelpScout ticket from an admin wanting to change the registration type on incomplete registrations. Because some required fields hadn't been filled out yet, the edit registration modal wasn't letting admins save the registration. To resolve that, this PR removes the ng-disabled check and lets admins always update registrations. Note that unlike Save, Save & Set as Completed still requires all questions to have valid questions.

I also had to set allowInvalid on ng-model-options for email address fields. Without it, data corruption is possible (see screen capture). To reproduce, create an incomplete registration, change the registrant type, and set the email address to an invalid value. At this point, the field is failing validation and ng-model sets the value to undefined. Saving the answer on the server without a value attribute will cause the email to be stored with a null value in the database. Then if you try to change the type, you will get an error about the email address being null.

To avoid this, I'm enabling allowInvalid on email addresses when admins are editing registrations so that invalid email addresses will still update ng-model and by rejected during the server-side email address validation. I'm not adding allowInvalid to any other fields because no other fields have server-side validation like email addresses do.

If a registration contains an invalid email address that was added before server-side email validation was added in May 2023 (see API PR), then admins won't be able to change the registrant type without fixing the invalid email address. I believe that that is an acceptable trade-off because legacy registrations with invalid email addresses would be over a year old by now.

Feel free to test in my Promo Test event: https://stage.eventregistrationtool.com/eventRegistrations/ec6ac5f9-1df0-4b69-b09d-f85ce39001ef

@canac canac added the On Staging Will be merged to the staging branch by Github Actions label Aug 22, 2024
@canac canac requested a review from dr-bizz August 22, 2024 19:55
Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

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

Great work around and PR description. Good work 🔥

@canac canac merged commit 8857ffd into master Aug 26, 2024
12 of 14 checks passed
@canac canac deleted the hs-1211838-edit-incomplete-registration branch August 26, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On Staging Will be merged to the staging branch by Github Actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants