[$250] Track expense - App shows skeleton when saving track expense edit for the second time onward

[izarutskaya]

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.65-0 Reproducible in staging?: Y Reproducible in production?: N Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Track expense.
3. Track a manual expense to self.
4. Go to transaction thread.
5. Click Amount.
6. Edit the amount and save it.
7. Click Amount.
8. Edit the amount and save it.

Expected Result:

App will not show skeleton when saving track expense edit for the second time.

Actual Result:

App shows skeleton when saving track expense edit for the second time onward.

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/f16ac6b0-17e1-4aa1-8e0f-e1b630a066e0

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01072100fbd6058c80
• Upwork Job ID: 1783235013921456128
• Last Price Increase: 2024-05-01

[melvin-bot[bot]]

Triggered auto assignment to @bfitzexpensify (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]]

Triggered auto assignment to @cristipaval (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions[bot]]

: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.

[izarutskaya]

@bfitzexpensify I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[izarutskaya]

We think this issue might be related to the #vip-vsb.

[izarutskaya]

Production

https://github.com/Expensify/App/assets/115492554/89e37075-95e2-4aaf-8d10-fb74d9551886

[mountiny]

Removing a blocker label as it does the work and user is not blocker and this is mainly ui issue cc @thienlnam

[mountiny]

@allroundexperts @shubham1206agra @s77rt this might be related to https://github.com/Expensify/App/pull/39956 your PR

[allroundexperts]

This was a known issue that we discussed during PR review as well. @shubham1206agra mentioned that it was happening on prod as well.

[thienlnam]

Yeah, we're not entirely sure why this is happening - going to open up externally so someone can investigate

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01072100fbd6058c80

[melvin-bot[bot]]

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

[nkdengineer]

Proposal

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

App shows skeleton when saving track expense edit for the second time onward.

What is the root cause of that problem?

When building the modified expense report action here, we're not setting optimistic previousReportActionID to the reportActionID of the latest report action.

So when we get the continuous report action list here, it fails to navigate further because previousReportActionID is empty and is not equal to the next report action.

So in here we're able to get only 1 report action which is the newly added modified expense, so hasCreatedAction here will be false (even though we do have the created report action).

In here, we're checking hasCreatedAction in order to avoid calling getOlderActions unnecessarily. In this case hasCreatedAction is false so it will still trigger getOlderActions, leading to the Skeleton loading.

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

When building the modified expense report action here, set the optimistic previousReportActionID to the reportActionID of the latest report action.

What alternative solutions did you explore? (Optional)

Above, we can optionally check other modified expense/system report action case to make sure the same issue doesn't happen.

Another solution: Here, early return the sortedReportActions if id is not provided. I think the continuous report action list retrieval is only relevant for linked comment case (where id will be provided). If it's not linked comment like in this case, we can just use the sortedReportActions.

if (!id) {
   return sortedReportActions;
}

[shubham1206agra]

@thienlnam Maybe this is a BE issue, as we don't set previousReportActionID anywhere in FE.

[nkdengineer]

IMO we should set it, optimistic data should be consistent with BE data, it's simple and fixes this issue well

[mananjadhav]

@thienlnam Maybe this is a BE issue, as we don't set previousReportActionID anywhere in FE.

@thienlnam Can you confirm this? Do we still fix this from FE.

[thienlnam]

Yeah right now we don't modify previousReportActionID anywhere in the FE. I'm guessing this is because an optimistic and actual reportActionID do not correspond so we're always trying to load a previous reportActionID that doesn't exist.

We should have a larger discussion about adding this in the FE to solve issues like this. We've talked about it briefly. This is also something that might not be a problem if we handled this correctly in the BE

[melvin-bot[bot]]

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

[thienlnam]

Going to make this internal now, unless we have a strong reason to start adding the previousReportActionID in the FE

[melvin-bot[bot]]

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