-
Notifications
You must be signed in to change notification settings - Fork 4k
Patch Search snapshot when submitting a tracked expense (offline Spend > Expenses) #97618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
cc2cf3c
32adcb9
d9f0bb2
2547d03
6f11102
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import type {SearchQueryJSON} from '@components/Search/types'; | ||
|
|
||
| import {isExpenseReport, isOptimisticPersonalDetail} from '@libs/ReportUtils'; | ||
| import {buildSearchQueryJSON, buildSearchQueryString, getCurrentSearchQueryJSON, getFilterFromQuery} from '@libs/SearchQueryUtils'; | ||
| import {buildCannedSearchQuery, buildSearchQueryJSON, buildSearchQueryString, getCurrentSearchQueryJSON, getFilterFromQuery} from '@libs/SearchQueryUtils'; | ||
| import {getSuggestedSearches, isEligibleForStatus} from '@libs/SearchUIUtils'; | ||
|
|
||
| import CONST from '@src/CONST'; | ||
|
|
@@ -113,6 +113,30 @@ function shouldOptimisticallyUpdateSearch( | |
| return shouldOptimisticallyUpdateByStatus && validSearchTypes && matchesFilterQuery; | ||
| } | ||
|
|
||
| /** | ||
| * The default Spend > Expenses and Reports pages render from the canned suggested-search snapshots | ||
| * (`type:expense` / `type:expense_report`). Those hashes are normally added to SEARCH_QUERY_BY_HASH | ||
| * only as a side effect of the `search()` action when the page is actually opened (see Search.ts). | ||
| * If the user never opened the page before going offline, that hash is absent from the map, so the | ||
| * fan-out loop in `getSearchOnyxUpdate` never patches the snapshot the page reads and it stays empty. | ||
| * | ||
| * These canned queries are deterministic and don't depend on a visit, so we register their hashes here | ||
| * and let the existing loop patch them exactly as it would after a visit. This is cheap: the query | ||
| * strings are trivial to build and `buildSearchQueryJSON` is internally cached. | ||
| */ | ||
| function getDefaultSearchQueriesByHash(): Record<string, string> { | ||
| const defaultQueryStrings = [buildCannedSearchQuery(), buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT})]; | ||
| const queriesByHash: Record<string, string> = {}; | ||
| for (const queryString of defaultQueryStrings) { | ||
| const queryJSON = buildSearchQueryJSON(queryString); | ||
| if (!queryJSON) { | ||
| continue; | ||
| } | ||
| queriesByHash[queryJSON.hash] = queryString; | ||
| } | ||
| return queriesByHash; | ||
| } | ||
|
|
||
| function getSearchOnyxUpdate({ | ||
| participant, | ||
| transaction, | ||
|
|
@@ -193,6 +217,12 @@ function getSearchOnyxUpdate({ | |
| key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${queryJSON.hash}` as const, | ||
| value: { | ||
| search: { | ||
| // `hash` is what makes an optimistically-created snapshot renderable. The Search page gates | ||
| // rendering on `isSearchDataLoaded`, which requires `snapshot.search.hash === queryJSON.hash`. | ||
| // When the page was visited before, `search()` already stamped this hash on the snapshot, so a | ||
| // partial MERGE renders; on a never-visited page the snapshot is created by this MERGE, so it | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ CONSISTENCY-16 (docs)This comment uses a semicolon to join two independent clauses. Comments should read as plain, natural sentences, so a semicolon that separates two complete thoughts should be split into two separate sentences. Split the clause into two sentences, e.g.: // When the page was visited before, `search()` already stamped this hash on the snapshot, so a
// partial MERGE renders. On a never-visited page the snapshot is created by this MERGE, so it
// must carry the hash itself or `isSearchDataLoaded` stays false and the page shows "Nothing to show".Reviewed at: 6f11102 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency. |
||
| // must carry the hash itself or `isSearchDataLoaded` stays false and the page shows "Nothing to show". | ||
| hash: queryJSON.hash, | ||
| type: queryJSON.type, | ||
| hasResults: true, | ||
| isLoading: false, | ||
|
|
@@ -252,6 +282,8 @@ function getSearchOnyxUpdate({ | |
| key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${groupTransactionsQueryJSON.hash}` as const, | ||
| value: { | ||
| search: { | ||
| // See the note above: the per-member snapshot must carry its own hash to be renderable when created optimistically. | ||
| hash: groupTransactionsQueryJSON.hash, | ||
| type: groupTransactionsQueryJSON.type, | ||
| offset: 0, | ||
| hasMoreResults: false, | ||
|
|
@@ -277,7 +309,10 @@ function getSearchOnyxUpdate({ | |
| // This catches cases like creating an expense from a chat while a `from:<me>` filter or | ||
| // `groupBy:from` view is loaded but not the currently active search. The hash→query map is | ||
| // stored in a dedicated Onyx key (not on the snapshot) so SEARCH API responses can't wipe it. | ||
| const queryByHash = getSearchQueryByHash(); | ||
| // The deterministic default canned hashes (see getDefaultSearchQueriesByHash) are merged in so the | ||
| // default Spend > Expenses / Reports pages are patched even when they were never visited. Onyx-recorded | ||
| // queries take precedence so a real visited entry is never shadowed by the canned default. | ||
| const queryByHash = {...getDefaultSearchQueriesByHash(), ...getSearchQueryByHash()}; | ||
| for (const [hashString, queryString] of Object.entries(queryByHash)) { | ||
| if (!queryString) { | ||
| continue; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ CONSISTENCY-17 (docs)
The phrase "fan-out" is AI-generated jargon that rarely appears in engineer-written code. It should be replaced with plain, direct language.
Rewrite the comment to drop "fan-out", e.g.:
Reviewed at: 6f11102 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.