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.53k stars 2.88k forks source link

[HOLD for payment 2024-10-31][$250] [Search v2.3] Add Onyx optimistic and failure data #49439

Closed lakchote closed 3 days ago

lakchote commented 1 month ago

See https://github.com/Expensify/App/pull/48566#issuecomment-2348617805

You should also optimistically display the saved search.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021836663116286679327
  • Upwork Job ID: 1836663116286679327
  • Last Price Increase: 2024-09-19
  • Automatic offers:
    • nkdengineer | Contributor | 104037632
Issue OwnerCurrent Issue Owner: @hoangzinh
melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

nkdengineer commented 1 month ago

Proposal

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

Add Onyx optimistic and failure data

What is the root cause of that problem?

The Save search button is not greyed out when offline, but instead takes you to the You appear to be offline page which is probably fine for now, but blocking the button tap would be the standard thing to do here I think!

The save search button is not disabled when we offline

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/pages/Search/AdvancedSearchFilters.tsx#L320

Deleting a saved search has no offline feedback (I'd expect the deleted search to show up greyed out)

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/libs/actions/Search.ts#L61

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

  1. The Save search button is not greyed out when offline, but instead takes you to the You appear to be offline page which is probably fine for now, but blocking the button tap would be the standard thing to do here I think!

We can add disabled={isOffline} for the save search button. Or if we want to create optimistic data and failure data for ONYXKEYS.SAVED_SEARCHES with pendingAction as add here with key is queryJSON.hash, name is queryJSON.saveSearchName and query is queryJSON.inputQuery. Then we can also add an error infailureData to display the error

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/pages/Search/AdvancedSearchFilters.tsx#L320

  1. When we delete the saved search here, we can add pendingAction in optimistic data of this saved search as delete and reset it in failureData and we can also add an error if the API fails

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/libs/actions/Search.ts#L61

Then we can add pendingAction field as item.pendingAction here. We can also add error field to display the error and onPendingActionDismiss to clear the error if we want

https://github.com/Expensify/App/blob/f7a4aa13979ae71631df79a6ac181c2e9b08f17f/src/pages/Search/SearchTypeMenu.tsx#L109

What alternative solutions did you explore? (Optional)

NA

lakchote commented 1 month ago

Added @allroundexperts experts as the C+ since he's available now to review, to move this forward.

allroundexperts commented 1 month ago

@nkdengineer's proposal looks good enough to me. I think we want to go with the optimistic data creation in this case. @lakchote can confirm this.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 1 month ago

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

lakchote commented 1 month ago

Let's go with optimistic data creation, yes.

@nkdengineer's proposal LGTM.

melvin-bot[bot] commented 1 month ago

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

nkdengineer commented 1 month ago

@allroundexperts The PR is here.

melvin-bot[bot] commented 3 weeks ago

This issue has not been updated in over 15 days. @lakchote, @luacmartins, @allroundexperts, @nkdengineer eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

nkdengineer commented 3 weeks ago

@lakchote This issue is ready for payment.

melvin-bot[bot] commented 1 week ago

Issue is ready for payment but no BZ is assigned. @johncschuster you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

lakchote commented 5 days ago

friendly bump @johncschuster, please process payment for @nkdengineer thanks!

johncschuster commented 5 days ago

Thanks for the bump! My bookmark for outstanding payments seems to not include the Awaiting payment label. I'll take care of this now.

johncschuster commented 5 days ago

Payment has been issued. Thank you!

luacmartins commented 5 days ago

Are we good to close this issue now?

allroundexperts commented 3 days ago

@johncschuster Can we please have a payment summary for this? Thanks!