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

[$1000] IOU - Total amount is flashing 3 times after back online #12792

Closed
kbecciv opened this issue Nov 16, 2022 · 39 comments
Closed

[$1000] IOU - Total amount is flashing 3 times after back online #12792

kbecciv opened this issue Nov 16, 2022 · 39 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@kbecciv
Copy link

kbecciv commented Nov 16, 2022

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 #12488

Action Performed:

  1. Launch the app
  2. Log in with any account
  3. Send a money request in USD while being online, e.g. $100.
  4. Send a second money request in a different currency, e.g. 100 AED.
  5. Go offline, cancel the request in AED
  6. Go online.

Expected Result:

The total is updated correctly without changing 3 times

Actual Result:

Total amount is flashing 3 times after back online

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.2.28.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

RPReplay_Final1668620180.1.MP4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 16, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 16, 2022

Triggered auto assignment to @maddylewis (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@maddylewis
Copy link
Contributor

maddylewis commented Nov 17, 2022

Managing a BUG issue:

  • The bug is not a duplicate report of an existing GH
  • The bug is reproducible, following the reproduction steps 
  • The GH template is filled out as fully as possible -- this means the GH body and title are clear (ie. could an external contributor understand it and work on it?)
  • The GH was created by an Expensify employee or a QA tester.
  • If the bug is an OldDot issue, you should decide whether it is widespread enough to justify fixing or if it is better to wait for reunification.
  • If there is a Slack link in the OP of the GH (i.e. from #expensify-open-source), review that for more info and check for any discussion about NOT fixing the issue.
  • Decide if the GH should be resolved by an External contributor or Internal engineer:
  • Add the External / Internal label

BUG GHs should remain Daily until closed or until the associated PR has been deployed to production and the GH title has been updated with "[HOLD for payment]" where payment is due in 7 days if no regressions.

@melvin-bot
Copy link

melvin-bot bot commented Nov 21, 2022

@maddylewis Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Nov 21, 2022
@youssef-lr youssef-lr self-assigned this Nov 21, 2022
@maddylewis maddylewis added the External Added to denote the issue can be worked on by a contributor label Nov 21, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 21, 2022

Current assignee @maddylewis is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Nov 21, 2022

Job added to Upwork: https://www.upwork.com/jobs/~01ef6af9374cf4e5ec

@melvin-bot
Copy link

melvin-bot bot commented Nov 21, 2022

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 21, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 21, 2022

Current assignee @youssef-lr is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title IOU - Total amount is flashing 3 times after back online [$1000] IOU - Total amount is flashing 3 times after back online Nov 21, 2022
@maddylewis
Copy link
Contributor

This issue is mentioned here - #12779.

Added external label to get a fix proposed 👍

@melvin-bot melvin-bot bot removed the Overdue label Nov 21, 2022
@trjExpensify
Copy link
Contributor

@youssef-lr @mountiny the root cause of this is the same as this issue, isn't it?

I don't think we should have separate issues up for this, as we discuss and look at finding a resolution to optimistic foreign currency requests.

@mountiny
Copy link
Contributor

Yes I agree, we should probably keep #12781 (comment) open as there is already lots of discussion. Removing the help wanted for now so contirbutors dont necesarily spend time on this once, this is better to be internal and I think we should close this issue in favour of the linked one.

And make sure that the testing steps from this one are covered in PR fixing #12781 (comment) as well.

@trjExpensify or @youssef-lr if you agree, please feel free to close this issue.

@mountiny mountiny removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 21, 2022
@trjExpensify
Copy link
Contributor

I agree with that. Will leave this open for @maddylewis, so she can kill the Upwork job when she gets a sec.

@devpuffer0807
Copy link

This is very challengeable problem.
I am very interested about this problem.
Could I work in backend and mobile project?

@devpuffer0807
Copy link

Also could I join your dev slack channel?
pufferdev0807@gmail.com (This is my email address)

@devpuffer0807
Copy link

I've some questions for this issue.
Please contact me asap.

@mountiny
Copy link
Contributor

mountiny commented Nov 22, 2022

Hey @pufferdev0807! Our back end is not open source at the moment, however, you can find all the information about how to contribute to this app or how to get access to the slack channel here https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md

Thank you!

@mountiny mountiny added Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor labels Nov 22, 2022
@mountiny
Copy link
Contributor

For this particular issue, it is better to be handled internally @pufferdev0807. Feel free to look at issues with Help Wanted label

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Nov 28, 2022
@melvin-bot melvin-bot bot changed the title [$1000] IOU - Total amount is flashing 3 times after back online [HOLD for payment 2022-12-05] [$1000] IOU - Total amount is flashing 3 times after back online Nov 28, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 28, 2022

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.32-2 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 2022-12-05. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot
Copy link

melvin-bot bot commented Nov 28, 2022

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:

  • [@Santhosh-Sellavel / @youssef-lr] The PR that introduced the bug has been identified. Link to the PR:
  • [@Santhosh-Sellavel / @youssef-lr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@Santhosh-Sellavel / @youssef-lr] 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:
  • [@maddylewis] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 5, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 12, 2022

@youssef-lr, @maddylewis, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

@maddylewis
Copy link
Contributor

@youssef-lr - let me know where we're at with this checklist whenever you have a chance. thanks!

  • [@Santhosh-Sellavel / @youssef-lr] The PR that introduced the bug has been identified. Link to the PR:
  • [@Santhosh-Sellavel / @youssef-lr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@Santhosh-Sellavel / @youssef-lr] 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:
  • [@maddylewis] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

@melvin-bot
Copy link

melvin-bot bot commented Dec 14, 2022

@youssef-lr, @maddylewis, @Santhosh-Sellavel Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot
Copy link

melvin-bot bot commented Dec 16, 2022

@youssef-lr, @maddylewis, @Santhosh-Sellavel Still overdue 6 days?! Let's take care of this!

@maddylewis
Copy link
Contributor

@youssef-lr + @Santhosh-Sellavel - can you provide an update on the status of this one? thanks!

@Santhosh-Sellavel
Copy link
Collaborator

@youssef-lr Please check, I'm not aware of this. cc: @mountiny I saw you as reviewer.

@mountiny
Copy link
Contributor

cc @youssef-lr as engineer assigned to this.

@youssef-lr
Copy link
Contributor

@maddylewis Sorry I missed your comment, I'm gonna update the checklist right away.

@Santhosh-Sellavel Santhosh-Sellavel removed their assignment Dec 23, 2022
@Santhosh-Sellavel
Copy link
Collaborator

Unassign me as I did nothing here!

@mallenexpensify mallenexpensify changed the title [HOLD for payment 2022-12-05] [$1000] IOU - Total amount is flashing 3 times after back online [$1000] IOU - Total amount is flashing 3 times after back online Dec 24, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 26, 2022

@youssef-lr, @maddylewis Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@maddylewis
Copy link
Contributor

@youssef-lr - just checking in here to make sure im not missing anything. lmk, if im good to move forward with adding a regression test and ill do that after the holiday break. thanks!

@mountiny mountiny removed the Awaiting Payment Auto-added when associated PR is deployed to production label Dec 29, 2022
@mountiny
Copy link
Contributor

Removed the waiting payment label as that was a lie.

@youssef-lr bumping the checklist 🙇

@melvin-bot
Copy link

melvin-bot bot commented Jan 3, 2023

@youssef-lr, @maddylewis Whoops! This issue is 2 days overdue. Let's get this updated quick!

@mountiny mountiny self-assigned this Jan 4, 2023
@mountiny
Copy link
Contributor

mountiny commented Jan 4, 2023

Assigning since Youssef should be ooo for 2 weeks so I will finish this one up later

@mountiny
Copy link
Contributor

mountiny commented Jan 5, 2023

I think this was not really a regression, we had discovered a new usecase for the IOUs. However, this because more clear after making all the IOUs optimistic and after this change #12488

Commented on the PR #12488 (comment)

I think there is no specific test to add to this situation. This is kind of related to any optimistic action and if when coming back online there is some odd behaviour that should be flagged by QA. Nothing to add here, I think we can close it

@mountiny mountiny closed this as completed Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

9 participants