-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2023-09-20] [$1000] Workspace - Incorrect Behavior When Inviting Existing User with Capitalized Email #25636
Comments
ProposalPlease re-state the problem that we are trying to solve in this issue.Incorrect Behavior When Inviting Existing User with Capitalized Email What is the root cause of that problem?App/src/pages/workspace/WorkspaceInvitePage.js Lines 187 to 196 in c435734
We don't lowercase the search value before validating it What changes do you think we should make in order to solve the problem?we should lowercase searchvalue before validating like this
Result |
ProposalPlease re-state the problemThere is a difference of error messages between uppercase and lowercase emails. What is the root cause of this problem?We are not converting the
What changes should be made in order to solve this problem?We should convert the const searchValue = searchTerm.trim().toLowerCase(); This is approach is being used in other places as well, so I think we will be consistent in that approach. App/src/libs/OptionsListUtils.js Line 638 in ed2c287
What other alternative approach did you explore? (Optional)None |
ProposalPlease re-state the problem that we are trying to solve in this issue.Emails of workspace members when typed in capital letters in the Invite search input shows 'No results found' instead of showing "user already a member of the workspace" What is the root cause of that problem?We are directly matching the trimmed App/src/pages/workspace/WorkspaceInvitePage.js Lines 187 to 193 in 63d87ba
So already existing emails do not get matched because they are in lower case and the search string includes capital letters. What changes do you think we should make in order to solve the problem?We should change this mentioned above
to const searchValue = searchTerm.trim().toLowerCase(); Now even Expensify emails are matched case-insensitively. What alternative solutions did you explore? (Optional)We can instead not convert directly the const searchValue = searchTerm.trim();
if (!userToInvite && CONST.EXPENSIFY_EMAILS.includes(searchValue.toLowerCase())) {
return translate('messages.errorMessageInvalidEmail');
}
if (!userToInvite && excludedUsers.includes(searchValue.toLowerCase())) {
return translate('messages.userIsAlreadyMemberOfWorkspace', {login: searchValue, workspace: policyName});
} Slight difference with this is the header message is shown in the same case as the search value with this solution whereas with the first solution the header message shows the lower case email. ResultcapitalLetterEmailSearchInWorkspaceInvitePage.movcapitalLetterEmailSearchInWorkspaceInvitePage2.mov |
Triggered auto assignment to @muttmuure ( |
Bug0 Triage Checklist (Main S/O)
|
@muttmuure Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@muttmuure Still overdue 6 days?! Let's take care of this! |
Job added to Upwork: https://www.upwork.com/jobs/~010c78f3bb410f312f |
Current assignee @muttmuure is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee ( |
Added external |
So we've got three duplicate proposals here, all with the correct RCA and solution. @DylanDylann's proposal happens to be the first so we can go with that! 🎀👀🎀 C+ reviewed |
Triggered auto assignment to @techievivek, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @jjcoffee 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @DylanDylann You have been assigned to this job! |
📣 @Habben101 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app! |
|
@jjcoffee, @techievivek, @muttmuure, @DylanDylann Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@muttmuure Gentle bump for payment here. Also, I think this may be due the timeliness bonus as it was only delayed due to the merge freeze (see this comment). |
Gentle bump @muttmuure ^ |
Gentle bump @muttmuure ^^ |
Getting to this now |
Offers sent, including urgency bonus |
@muttmuure Thanks, offer accepted! |
@muttmuure Thanks, I've accepted the offer. |
@muttmuure are we done with the payment here? Can we close the issue now? |
DMed Matt to look into it. |
Handling |
Just waiting on @DylanDylann to accept, then we can close out |
@muttmuure I've accepted, thank you! |
Paid! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
The description should clearly state that the user is already a member of the workspace
Actual Result:
Instead of the expected message, it displays "No results found" when inviting the same user with capitalized email letters
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.56-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
CapitalizedInvitation.mp4
Recording.2949.mp4
Expensify/Expensify Issue URL:
Issue reported by: @Habben101
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691797768663379
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: