-
Notifications
You must be signed in to change notification settings - Fork 3k
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-10-27] [$500] IOU Category - Search message shows 'Invalid email' when typing email address in search field #28078
Comments
Triggered auto assignment to @Christinadobrzyn ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.IOU Category - Search message shows 'Invalid email' when typing email address in search field What is the root cause of that problem?App/src/libs/OptionsListUtils.js Lines 1476 to 1485 in 5aa7f6d
Let's see the logic to get header message, we are checking if the search value includes "@" we will display errorMessageInvalidEmail instead of noResultsFound What changes do you think we should make in order to solve the problem?In this case, we don't search email and phone number App/src/libs/OptionsListUtils.js Lines 1477 to 1485 in 5aa7f6d
So this logic to redandunt. We need to pass one more params called includeEmailAndPhoneNumber to getHeaderMessage function and only apply logic to check email and phone if this param is true What alternative solutions did you explore? (Optional) |
Not sure how to make a Category, asking QA - https://expensify.slack.com/archives/C9YU7BX5M/p1695681312874909 |
still trying to get Categories to work on my c.dobrzyn+45@expensifail.com account. Asking if anyone else has this issue? https://expensify.slack.com/archives/C01SKUP7QR0/p1695845916425979 |
This comment was marked as outdated.
This comment was marked as outdated.
@Christinadobrzyn I still can reproduce this bug on the latest main by enabling permission in the codebase. You can ping C+ team or internal engineer to help verify the bug |
@Christinadobrzyn This is an example of category issue: #28079 |
@s77rt It seems @Christinadobrzyn can't access the category feature. Because you are C+ Contributor of another issue that relates to category #28517. Could you help to verify this is a bug? |
Thanks @DylanDylann! I'll reopen and assign a C+ since you can still see it's happening. |
Job added to Upwork: https://www.upwork.com/jobs/~0131655c00ab293751 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.IOU Category - Search message shows 'Invalid email' when typing email address in search field What is the root cause of that problem?To determine the email, we only use the presence of @ in the line App/src/libs/OptionsListUtils.js Line 1480 in 5aa7f6d
What changes do you think we should make in order to solve the problem?But what is email? leters => @ => leters => . => leters (Plus numbers. But I'm not sure that they can be used in every point in my scheme) So we can use this regex and then we will check string is valid or not using isValidEmail As a result
App/src/libs/OptionsListUtils.js Lines 1480 to 1482 in 5aa7f6d
But actually it would be great Screen.Recording.2023-10-06.at.17.57.07.movWhat alternative solutions did you explore? (Optional) |
@ZhenjaHorbach, I'm also interested in the project. By the way the regular expression you suggested will work for validation only if there is no punctuation in the TLD.
It's better to use: /\S+@\S+\.\S+/ This will return the whole email on a match. And not partial using the Hope this helps. |
📣 @ChrisCates! 📣
|
Contributor details |
✅ Contributor details stored successfully. Thank you for contributing to Expensify! |
@rushatgabhane this PR is ready for review |
approved |
🎯 ⚡️ Woah @rushatgabhane / @DylanDylann, great job pushing this forwards! ⚡️ The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉
On to the next one 🚀 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.87-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-10-27. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
I believe we just need the BZ checklist here so we can proceed with the payments |
Payouts due: Issue Reporter: NA - reporter is Applause Eligible for 50% #urgency bonus? Y - #28078 (comment) Upwork job is here. @rushatgabhane could you let us know about a regression test? |
nudge @rushatgabhane about a regression test, thanks! |
@MariaHCD, @rushatgabhane, @Christinadobrzyn, @DylanDylann Eep! 4 days overdue now. Issues have feelings too... |
Dming @rushatgabhane to see if we need a regression test |
|
Created a manual request https://staging.new.expensify.com/r/8819237970145454 |
awesome! I think this can be closed. |
$750 payment approved for @rushatgabhane based on this comment. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Issue found when executing PR #27459
Action Performed:
Expected Result:
The message below the search field will display 'No results found'..
Actual Result:
The message below the search field displays 'invalid email' when Category search does not involve email
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.73-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
Bug6211688_20230923_190412.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: