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 2023-09-12] [HOLD for payment 2023-09-11] [$1000] IOU - Currency edit in confirmation page causes missing header and wrong redirection #26586

Closed
6 tasks done
lanitochka17 opened this issue Sep 2, 2023 · 38 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 2, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Issue found when executing PR #26520

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to + > Request money > Enter an amount > Select a payer
  3. On confirmation page, click on the Amount
  4. Edit the amount only > Click Next
  5. On the confirmation page, click on the Amount again
  6. Change the currency
  7. Click Next

Expected Result:

In Step 6, Amount header does not disappear
In Step 7, user is redirected to the confirmation page

Actual Result:

In Step 6, Amount header disappears
In Step 7, user is redirected to the payer selection page. This is a different behavior observed in Step 4 when user is redirected to the confirmation page when the amount is changed

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to + > Request money
  3. Enter an amount
  4. Change the currency

Expected Result:

The amount is preserved and does not change

Actual Result:

The amount is not preserved and it changes to zero

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.62-1

Reproducible in staging?: Yes

Reproducible in production?: No

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

Bug6186258_20230902_233538.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010711a82a596d6dcc
  • Upwork Job ID: 1698168980758937600
  • Last Price Increase: 2023-09-03
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Sep 2, 2023
@OSBotify
Copy link
Contributor

OSBotify commented Sep 2, 2023

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@melvin-bot
Copy link

melvin-bot bot commented Sep 2, 2023

Triggered auto assignment to @marcaaron (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@napster125
Copy link
Contributor

napster125 commented Sep 3, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Currency edit in confirmation page causes missing header and wrong redirection

What is the root cause of that problem?

470c54a caused regression.

Navigation.goBack(`${props.route.params.backTo}?currency=${option.currencyCode}`, true);

In this code snippet, the goBack function with true as the second parameter replaces the modal without considering the previous state. This leads to unexpected behavior, such as the issue we've encountered and the resetting of text input.

What changes do you think we should make in order to solve the problem?

We should use Navigation.navigate function instead of goBack function.

Navigation.navigate(`${props.route.params.backTo}?currency=${option.currencyCode}`);

What alternative solutions did you explore? (Optional)

N/A

@shubham1206agra
Copy link
Contributor

@marcaaron If you have no problem, we should combine this with #26587

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented Sep 3, 2023

This was probably caused by #25707

We need to solve this my Monday and it's weekend. I gonna make this external to get immediate proposals and PRs.

@hayata-suenaga hayata-suenaga added the External Added to denote the issue can be worked on by a contributor label Sep 3, 2023
@melvin-bot melvin-bot bot changed the title IOU - Currency edit in confirmation page causes missing header and wrong redirection [$500] IOU - Currency edit in confirmation page causes missing header and wrong redirection Sep 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

Job added to Upwork: https://www.upwork.com/jobs/~010711a82a596d6dcc

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

Triggered auto assignment to @sophiepintoraetz (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@hayata-suenaga
Copy link
Contributor

hayata-suenaga commented Sep 3, 2023

@marcaaron If you have no problem, we should combine this with #26587

Brought issue reported in with #26587 and combined with this one

@hayata-suenaga hayata-suenaga changed the title [$500] IOU - Currency edit in confirmation page causes missing header and wrong redirection [$1000] IOU - Currency edit in confirmation page causes missing header and wrong redirection Sep 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 3, 2023

Upwork job price has been updated to $1000

@napster125
Copy link
Contributor

@hayata-suenaga could you please take a look here? I can make PR ASAP.

@hayata-suenaga
Copy link
Contributor

@napster125 could you kindly post a video showing navigation still works after your changes and then we can proceed with your PR

@napster125
Copy link
Contributor

napster125 commented Sep 3, 2023

of course!

mobile-safari.mp4

@hayata-suenaga
Copy link
Contributor

@napster125

also do you think this one is also somehow related to this issue?

@sophiepintoraetz
Copy link
Contributor

Providing there's no regressions, I'm correct in saying that $1500 is due to @napster125?

@mountiny
Copy link
Contributor

mountiny commented Sep 4, 2023

@sophiepintoraetz thats right and that should be the only payment here, thanks!

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Sep 4, 2023
@melvin-bot melvin-bot bot changed the title [$1000] IOU - Currency edit in confirmation page causes missing header and wrong redirection [HOLD for payment 2023-09-11] [$1000] IOU - Currency edit in confirmation page causes missing header and wrong redirection Sep 4, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Sep 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.62-4 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-09-11. 🎊

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.

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

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:

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

@sophiepintoraetz
Copy link
Contributor

Offer sent - I'll release payment on 11 Sep

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Sep 5, 2023
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2023-09-11] [$1000] IOU - Currency edit in confirmation page causes missing header and wrong redirection [HOLD for payment 2023-09-12] [HOLD for payment 2023-09-11] [$1000] IOU - Currency edit in confirmation page causes missing header and wrong redirection Sep 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 5, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.63-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 2023-09-12. 🎊

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.

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

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:

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

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 Daily KSv2 labels Sep 11, 2023
@sophiepintoraetz
Copy link
Contributor

Going to move this to weekly while we work out the payment situation.

@melvin-bot melvin-bot bot removed the Overdue label Sep 13, 2023
@sophiepintoraetz sophiepintoraetz added Weekly KSv2 and removed Daily KSv2 labels Sep 13, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 21, 2023
@sophiepintoraetz
Copy link
Contributor

Actually, I'm going to close this one out and create a new one with the appropriate issue, seeing as payment is the only thing left here.

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 Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants