Give batch uploading users more information when an email address in their batch is invalid #2297
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mike29736
force-pushed
the
give-batch-uploading-users-more-information
branch
from
407f376
to
9b78962
Compare
floehopper
suggested changes
Aug 7, 2023
There was a problem hiding this comment.
I think this is definitely an improvement, but I've added a comment inline suggesting we might be able to make the error message more actionable by including the first invalid email address. What do you think...?
I'm happy to chat about it if that would be easier.
The batch invite process creates a User record based on a BatchInvitationUser. Until now, the email address that those two records share wasn't validated until the User record was created. The validation rules that the User model employs are provided by Devise (https://github.com/heartcombo/devise/blob/v4.9.2/lib/devise/models/validatable.rb#L31-L33). Unfortunately Devise doesn't appear to be able to offer these rules in isolation, only as a package deal along with validation rules for other columns that aren't applicable here, so I've duplicated the relevant ones of those email rules in our model class. (Note: `email_regexp`/`Devise.email_regexp` is set by us in config/initializers/devise.rb)
The BatchInvitation and its BatchInvitationUsers are still saved, just in a different order now.
Until now, one invalid email address in a batch upload (CSV) would raise an ambiguous error and give the user no information about what went wrong. Now, if the batch encounters a validation problem, it returns to the original form as before but also displays a more helpful error message. This is a small improvement which avoids some of the complexities of what might be even more helpful error reports (e.g. listing one or all of the invalid email addresses).
mike29736
force-pushed
the
give-batch-uploading-users-more-information
branch
from
9b78962
to
789b799
Compare
floehopper
approved these changes
Aug 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://trello.com/c/HOwVtQLy/184-give-batch-uploading-users-more-information-when-an-email-address-in-their-batch-is-invalid
Until now, one invalid email address in a batch upload (CSV) would raise
an ambiguous error and give the user no information about what went
wrong.
Now, if the batch encounters a validation problem, it returns to the
original form as before but also displays a more helpful error message.
Though, as I've implemented it, the message could be more helpful. But see for yourself: