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
2.99k stars 2.5k forks source link

[$250] IOS - After editing split expense amount weird flash and reloads at the top #41174

Open m-natarajan opened 2 weeks ago

m-natarajan commented 2 weeks 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!


Version Number: 1.4.67-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: @roryabraham Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1714161321481359

Action Performed:

  1. Create a group
  2. Add messages to the chat such that there is at least one full screen of chat messages.
  3. Scan a receipt to split a bill in the group.
  4. Manually set the amount on the bill split

Expected Result:

The amount should update, and nothing else should happen.

Actual Result:

The amount updates, then the report "flashes" and reloads at the top, then scrolls down to the bottom, without you touching anything.

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/82bec8e2-0c90-42c9-9b47-4b398a09bc7f

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0174f92c2a5d39efca
  • Upwork Job ID: 1784657619645915136
  • Last Price Increase: 2024-05-05
Issue OwnerCurrent Issue Owner: @roryabraham
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @laurenreidexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 2 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~0174f92c2a5d39efca

melvin-bot[bot] commented 2 weeks ago

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

eh2077 commented 2 weeks ago

Posted on slack to attract proposals

eh2077 commented 2 weeks ago

Not overdue, still waiting for proposals

charles-liang commented 1 week ago

Proposal

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

After editing split expense amount weird flash and reloads at the top

What is the root cause of that problem?

As shown in the attachment, the CompleteSplitBill returns data where the most recent previousReportActionID is 0. However, in the local data, this ID corresponds to the number of the previous order. This discrepancy causes the reportActions to refresh when the network response is received, leading to a page refresh.

As shown in the first code snippet, by adding debugging code on line 794, you will notice that the previousReportActionID changes from an initial value to 0 and then reverts back to the original value. The debugging line of code would look like this:

console.log(
'ReportScreen', 
prevProps.sortedAllReportActions[0].previousReportActionID, nextProps.sortedAllReportActions[0].previousReportActionID);

https://github.com/Expensify/App/blob/ab1e924bf0d4f928a9006f5bdd6b9da60346b1df/src/pages/home/ReportScreen.tsx#L787-L815

Due to the changes in previousReportActionID, line 268 of the ReportActionsUtils.getContinuousReportActionChain function returns different values, causing changes in reportActions and subsequently triggering a redraw of the screen.

https://github.com/Expensify/App/blob/ab1e924bf0d4f928a9006f5bdd6b9da60346b1df/src/pages/home/ReportScreen.tsx#L264-L269

condition in Line 317

https://github.com/Expensify/App/blob/ab1e924bf0d4f928a9006f5bdd6b9da60346b1df/src/libs/ReportActionsUtils.ts#L290-L344

request.txt response.json

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

Fix Backend return the expect value

What alternative solutions did you explore? (Optional)

Or add condition in ReportActionsUtils.ts#L317 except previousReportActionID equal 0

melvin-bot[bot] commented 1 week ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

eh2077 commented 1 week ago

@charles-liang Thanks for your proposal. Your RCA is correct and I agreed with you this issue should be fixed by the backend. StartSplitBill returns the correct previousReportActionID

image

While CompleteSplitBill returns wrong previousReportActionID.

image

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 1 week ago

Current assignee @roryabraham is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

roryabraham commented 1 week ago

ok, I keep seeing a number of issues with incorrect previousReportActionID generated in the back-end

melvin-bot[bot] commented 1 week ago

Current assignee @eh2077 is eligible for the Internal assigner, not assigning anyone new.

melvin-bot[bot] commented 6 days ago

@roryabraham @laurenreidexpensify @eh2077 this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

eh2077 commented 5 days ago

I think @roryabraham will take care of this from the backend. Once the backend fix is done, we can retest it to determine if it's good to close the issue.

arielgreen commented 5 days ago

@roryabraham can you confirm if you're going to be working on this or if it needs a hot pick label?

roryabraham commented 1 day ago

I'm still working on this, though it won't necessarily be back-end. I'm writing up a thorough P/S in this doc

roryabraham commented 1 day ago

Requested retest: https://expensify.slack.com/archives/C9YU7BX5M/p1715988615903839

kavimuru commented 1 day ago

Bug is still able to reproduce.

https://github.com/Expensify/App/assets/43996225/6b0da52e-bf4a-458b-a4eb-6504d7c5dc77