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.03k stars 2.54k forks source link

Search-Expense details updated are not shown immediately #42254

Open izarutskaya opened 2 weeks ago

izarutskaya 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.74 Reproducible in staging?: Y Reproducible in production?: Unable to check Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

  1. Launch app
  2. Tap fab -- submit expense
  3. Create a expense with an user
  4. Navigate to lhn
  5. Tap profile -- troubleshoot -- new search
  6. Tap on the expense
  7. In IOU details page, change amount
  8. Note in search page, amount not updated
  9. Tap on the expense
  10. Enter merchant and note in search page, merchant also not updated
  11. Tap chat icon and from profile -- troubleshoot -- new search revisit page
  12. Note now updated amount and merchant is shown

Expected Result:

In search page, expense details updated must be shown immediately.

Actual Result:

In search page, expense details updated are not shown immediately, shown only while revisiting page.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/Expensify/App/assets/115492554/3f6144b9-708e-4dfa-b9e7-58131e2e54bb

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @abdulrahuman5196
melvin-bot[bot] commented 2 weeks ago

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

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

melvin-bot[bot] commented 2 weeks ago

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

  1. If you find which PR caused the issue/bug, you can reassign it to the person responsible for it.
    • If the author is OOO or won’t get online before the daily deploy is due, you are responsible for finding the best fix/path forward. Don’t hesitate to ask for help!
  2. Try to reproduce the issue, if the bug is on production, remove the DeployBlocker label but stay assigned to fix it (or find out which PR broke it to get help from the author).
    • You can adjust the urgency of the issue to better represent the gravity of the bug.
    • If the issue is super low priority, feel free to un-assign yourself.
    • Be careful with PHP warnings, sometimes it is more complex than just adding a null coalescing operator as they might be uncovering some bigger bug.
    • If it was a one-off issue that requires no action (for example, Bedrock was down or it is a duplicated issue), you can close it.

Remember rule #2: Never un-assign yourself from a real DeployBlocker unless you are 100% sure someone else is assigned and will take care of it.

github-actions[bot] commented 2 weeks 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.
izarutskaya commented 2 weeks ago

@dylanexpensify 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 commented 2 weeks ago

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

youssef-lr commented 2 weeks ago

can we check again if this is reproducible in prod?

kavimuru commented 2 weeks ago

@youssef-lr we don't have "testing preference" in production. So, we can't check it.

image

dylanexpensify commented 2 weeks ago

@youssef-lr checking in on this, need anything from me so far?

youssef-lr commented 2 weeks ago

Nothing yet @dylanexpensify, thanks! I'm trying to reproduce this now.

youssef-lr commented 2 weeks ago

Demoting this given that this only affect the new search page which is only accessible from staging

melvin-bot[bot] commented 1 week ago

@youssef-lr, @dylanexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

dylanexpensify commented 1 week ago

sounds good!

melvin-bot[bot] commented 5 days ago

@youssef-lr, @dylanexpensify Eep! 4 days overdue now. Issues have feelings too...

youssef-lr commented 4 days ago

Not overdue.

melvin-bot[bot] commented 4 days ago

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

melvin-bot[bot] commented 4 days ago

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

dominictb commented 3 days ago

Proposal

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

In search page, expense details updated are not shown immediately, shown only while revisiting page.

What is the root cause of that problem?

  1. In TransactionListItemRow here, it's using the item from the search result as is, so when the transaction is updated, the update is not yet reflected in the search result, this cause the information shown to be outdated.

  2. When memoizing the result of the cell like in here, it's checking only the keyForList equality, so when the keyForList does not change but the other props like amount changes, the component will not update

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

  1. In TransactionListItemRow, use useOnyx to get the latest value by item.transactionID, and use those values in priority, if there're fields that don't exist in the Onyx transaction like from, to, use the search result values for them (they cannot be updated anyway so no issue)

  2. When memoizing, check other relevant props too, not just keyForList. I.e for TotalCell we should check for amount, currency, ... equality too

What alternative solutions did you explore? (Optional)

When optimistically update the expense transaction, update the search result snapshot that contains that transaction too.

dylanexpensify commented 3 days ago

@abdulrahuman5196 to review!

melvin-bot[bot] commented 2 days ago

@youssef-lr @abdulrahuman5196 @dylanexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!