You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2024-01-26] [$500] Settings - Red dot remains after error is cleared #33021
Version Number: 1.4.12-0 Reproducible in staging?: Y Reproducible in production?: Y 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 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:
changed the title [-]All - Settings - Red dot remains after error is cleared[/-][+][$500] All - Settings - Red dot remains after error is cleared[/+]on Dec 13, 2023
Please re-state the problem that we are trying to solve in this issue.
All - Settings - Red dot remains after error is cleared
What is the root cause of that problem?
In deleteContactMethod, we are clearing errorFields.deletedLogin but we aren't clearing errorFields.addedLogin which is a part of the failureData of addNewContactMethodAndNavigate.
When we delete a contact method, we expect a green dot to show up while the deletion is being processed, this is added intentionally by using pendingFields.deletedLogin: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE as shown below in the code, however this green dot is override by red dot because of errorFields.addedLogin
What changes do you think we should make in order to solve the problem?
Logic on both ends should be kept consistent so I would suggest we implement the same checks in userUtils and call it. This will maintain the DRY principle as well as is easier to maintain.
What alternative solutions did you explore? (Optional)
📣 @sfurqan2! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
Hi! I just wanted to call it out here - I'm closing out #34758 as not a bug, but I'm pretty sure it means that QA was never able to successfully QA this issue. It would be great if y'all could make sure this works as expected on this 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:
[@ntdiary] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: No need
[@ntdiary] Determine if we should create a regression test for this bug. No need
[@ntdiary] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. No need
[@michaelhaxhiu] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
I think this is a small edge case we overlooked during the development of the contact details page, so it's okay to just leave a comment in the original PR, no regression test is needed. :)
Activity
[-]All - Settings - Red dot remains after error is cleared[/-][+][$500] All - Settings - Red dot remains after error is cleared[/+]melvin-bot commentedon Dec 13, 2023
Triggered auto assignment to @michaelhaxhiu (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.melvin-bot commentedon Dec 13, 2023
Job added to Upwork: https://www.upwork.com/jobs/~012ec660a67cb6d223
melvin-bot commentedon Dec 13, 2023
Bug0 Triage Checklist (Main S/O)
Platformsin OP are ✅)melvin-bot commentedon Dec 13, 2023
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary (
External)AmjedNazzal commentedon Dec 13, 2023
Proposal
Please re-state the problem that we are trying to solve in this issue.
All - Settings - Red dot remains after error is cleared
What is the root cause of that problem?
In
deleteContactMethod, we are clearingerrorFields.deletedLoginbut we aren't clearingerrorFields.addedLoginwhich is a part of thefailureDataofaddNewContactMethodAndNavigate.When we delete a contact method, we expect a green dot to show up while the deletion is being processed, this is added intentionally by using
pendingFields.deletedLogin: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETEas shown below in the code, however this green dot is override by red dot because oferrorFields.addedLoginApp/src/libs/actions/User.js
Lines 186 to 200 in fe2d0d1
What changes do you think we should make in order to solve the problem?
We should clear that errorField on contact method deletion so the red dot doesn't show up during the deletion process
Result
Screen.Recording.2023-12-14.at.2.31.09.AM.mov
sfurqan2 commentedon Dec 14, 2023
Proposal
Please re-state the problem that we are trying to solve in this issue.
Red dot remains after error is cleared when network connection is simulating slow network requests
What is the root cause of that problem?
The root cause of the problem is that in Contact Page there is an additional check which hides new contact requests with pendingAction "delete".
However, this check is missing from userUtils "hasLoginListError" function.
What changes do you think we should make in order to solve the problem?
Logic on both ends should be kept consistent so I would suggest we implement the same checks in userUtils and call it. This will maintain the DRY principle as well as is easier to maintain.
What alternative solutions did you explore? (Optional)
N/A
melvin-bot commentedon Dec 14, 2023
📣 @sfurqan2! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:
Format:
sfurqan2 commentedon Dec 14, 2023
Contributor details
Your Expensify account email: sfurqan1999@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0163c31a5a31c72f3f
58 remaining items
dangrous commentedon Jan 29, 2024
Hi! I just wanted to call it out here - I'm closing out #34758 as not a bug, but I'm pretty sure it means that QA was never able to successfully QA this issue. It would be great if y'all could make sure this works as expected on this issue!
danieldoglas commentedon Jan 31, 2024
@ntdiary let's do the bugzero checklist here
michaelhaxhiu commentedon Jan 31, 2024
Next step is with @ntdiary to do the regression steps
ntdiary commentedon Feb 1, 2024
I think this is a small edge case we overlooked during the development of the contact details page, so it's okay to just leave a comment in the original PR, no regression test is needed. :)
melvin-bot commentedon Feb 1, 2024
Triggered auto assignment to @miljakljajic (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.danieldoglas commentedon Feb 1, 2024
cc @miljakljajic can you please proceed with the payment on this issue? Thanks!
miljakljajic commentedon Feb 2, 2024
Paid and paid!