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

[HOLD for payment 2024-04-09] [Simplified Collect][Members] Polish items for Transfer owner flow #39005

Closed
luacmartins opened this issue Mar 26, 2024 · 12 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2

Comments

@luacmartins
Copy link
Contributor

luacmartins commented Mar 26, 2024

cc @burczu Coming from this PR, there are a few polish items that need to be handled:

  1. Wrong error flashes momentarily
error_flash.mov
  1. We're showing an error screen on success after adding a debit card
error_page_on_success.mov
  1. We're showing three decimals for USD currency

three_decimals

@luacmartins luacmartins added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 26, 2024
@luacmartins luacmartins self-assigned this Mar 26, 2024
Copy link

melvin-bot bot commented Mar 26, 2024

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

@burczu
Copy link
Contributor

burczu commented Mar 27, 2024

Hey @luacmartins, please assign me to this issue as this is a follow up issue for my recently merged feature. Thank you!

@burczu
Copy link
Contributor

burczu commented Mar 27, 2024

  1. Disable Transfer owner button if user is offline since we can only go through this flow while online.

@luacmartins I'm surprised because offline state is handled from the very beginning of working on this issue:

Screenshot 2024-03-27 at 08 39 13

Could you provide your steps to reproduce it? Maybe you've found some edge case scenario?

@luacmartins
Copy link
Contributor Author

Oh maybe, the App wasn't actually offline when I checked. Seems like this is already done, so let's move on to the other items in the list!

@burczu
Copy link
Contributor

burczu commented Mar 27, 2024

Oh maybe, the App wasn't actually offline when I checked. Seems like this is already done, so let's move on to the other items in the list!

Yeah - I've actually done all of it today (you can check on the draft PR). Tomorrow morning I'll fill the checklist, add videos and publish it.

@luacmartins luacmartins added the Reviewing Has a PR in review label Mar 27, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Mar 28, 2024
luacmartins added a commit that referenced this issue Mar 28, 2024
…ransfer-owner-flow

#39005 - Polish items for Transfer owner flow
@luacmartins
Copy link
Contributor Author

PR merged

@luacmartins
Copy link
Contributor Author

Found another bug on the API level, put up a PR with a fix

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Apr 2, 2024
@melvin-bot melvin-bot bot changed the title [Simplified Collect][Members] Polish items for Transfer owner flow [HOLD for payment 2024-04-09] [Simplified Collect][Members] Polish items for Transfer owner flow Apr 2, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Apr 2, 2024
Copy link

melvin-bot bot commented Apr 2, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Apr 2, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.58-8 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 2024-04-09. 🎊

For reference, here are some details about the assignees on this issue:

  • @burczu does not require payment (Contractor)

Copy link

melvin-bot bot commented Apr 2, 2024

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:

  • [@luacmartins] The PR that introduced the bug has been identified. Link to the PR:
  • [@luacmartins] 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:
  • [@luacmartins] 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:
  • [@burczu] Determine if we should create a regression test for this bug.
  • [@burczu] 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.
  • [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@luacmartins
Copy link
Contributor Author

No need for BZ checklist, this was a new feature in implementation.

@luacmartins
Copy link
Contributor Author

I think we're good to close this one, since I reviewed and did the checklist for all PRs linked given that it involved mocking API responses and it was hard for external contributors to test it. Please reopen if I missed something.

@github-project-automation github-project-automation bot moved this from Polish to Done in [#whatsnext] #wave-collect Apr 2, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

3 participants