Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.36k stars 2.79k forks source link

[HOLD for payment 2023-08-10] [HOLD for payment 2023-08-08] Pressing Enter key on Split bill page doesn't navigate to next step #23950

Closed kavimuru closed 1 year ago

kavimuru commented 1 year ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Click on 'FAB' menu
  2. Click on 'Split bill' button
  3. Enter amount & press 'Enter' key

    Expected Result:

    User should be navigated to the next step

    Actual Result:

    User is not navigated to the next step

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

Version Number: 1.3.48-0 Reproducible in staging?: y Reproducible in production?: n 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

https://github.com/Expensify/App/assets/43996225/05a0531a-66e2-407a-b832-fdc084a64671

https://github.com/Expensify/App/assets/43996225/0ddafca2-2b55-4658-b8f4-f18b2c8e37cf

Expensify/Expensify Issue URL: Issue reported by: @natnael-guchima Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690829632233769

View all open jobs on GitHub

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

OSBotify commented 1 year ago

:wave: 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[bot] commented 1 year ago

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

getusha commented 1 year ago

@marcaaron did some debugging, i think the root cause of this issue is from onyx, the update of the amount is delayed and initially it's 0 so it will navigate us back from MoneyRequestParticipantsPage here:

https://github.com/Expensify/App/blob/1fa4b723d8ef649afa56780b52ef453a39bbc9fc/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js#L77-L79

and there is onyx version upgrade 5 days ago deployed to staging NoQA] Perf/onyx upgrade use cache directly #23749

Pujan92 commented 1 year ago

@hannojg This seems to persist after react-native-onyx change https://github.com/Expensify/App/pull/23749 https://github.com/Expensify/react-native-onyx/pull/280 , cache logic(cahcedState) implementation might be the cause for this. With the earlier version 1.0.52 issue is not reproducible.

Specifically in this case, we don't want to render immediately with the cached data as we depend on the updated one which is still in the execution. https://github.com/Expensify/react-native-onyx/pull/280/files#diff-364fd7349a1af7b52091427c5a0734da8ff56057fbc7fe2be10734a06c97264cR52-R53

getusha commented 1 year ago

@Pujan92 what a coincidence! 😄

marcaaron commented 1 year ago

@Pujan92 what a coincidence! 😄

@getusha Is that sarcasm? If so, not sure if I understood you properly or what the "coincidence" you are referring to is. Thanks!

getusha commented 1 year ago

@getusha Is that sarcasm? If so, not sure if I understood you properly or what the "coincidence" you are referring to is. Thanks!

@marcaaron It's just the fact that @Pujan92 and i commented almost the exact same time.

hannojg commented 1 year ago

I am currently running into an issue where I can't open the split bill page due to this crash. is it just me or everyone?

Screenshot 2023-08-01 at 11 12 53

hannojg commented 1 year ago

Try creating a new fresh account in a new browser instance. I think the crash is reproducible

parasharrajat commented 1 year ago

Yes, it happening to all.

Beamanator commented 1 year ago

Ya that looks like this issue: https://github.com/Expensify/App/issues/23951 (aah looks like you found it already)

hannojg commented 1 year ago

Okay, however with these changes, there is still the issue that when pressing the Enter-Key,

  1. You will open a second instance of the same page (split bill)
  2. On the second press you get to the next page

There was a separate issue for that, not sure if that's also due to the onyx changes will double check!

hannojg commented 1 year ago
melvin-bot[bot] commented 1 year ago

⚠️ 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.

hannojg commented 1 year ago

Fixed also with this PR:

melvin-bot[bot] commented 1 year ago

⚠️ 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.

Beamanator commented 1 year ago

Looks like this was fixed by https://github.com/Expensify/App/pull/24003! Not a deploy blocker anymore

melvin-bot[bot] commented 1 year ago

⚠️ 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.

melvin-bot[bot] commented 1 year ago

⚠️ 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.

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.48-5 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-08-08. :confetti_ball:

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.

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

melvin-bot[bot] commented 1 year ago

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:

melvin-bot[bot] commented 1 year ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.3.49-3 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-08-10. :confetti_ball:

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.

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

melvin-bot[bot] commented 1 year ago

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:

Natnael-Guchima commented 1 year ago

Hello @marcaaron, no reporting bonus is issued here.

Natnael-Guchima commented 1 year ago

A gentle bump for reporting payment @isabelastisser

isabelastisser commented 1 year ago

@Natnael-Guchima I sent you an offer in Upwork now. Please accept it and I will process the payment. Thanks!

Natnael-Guchima commented 1 year ago

I have accepted the offer. Thanks @isabelastisser