Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/hooks/useSearchHighlightAndScroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,11 +383,13 @@ function extractReportActionIDsFromSearchResults(searchResultsData: Partial<Sear
}

/**
* Whether a transaction that the current search results already display has changed.
* Whether a transaction change invalidates what the current search results show.
*
* Onyx keeps one value object per collection member and only replaces the ones it writes, so an identity check is
* enough to spot an edit. A refetch triggered from here can't feed itself, because a Search response only writes
* snapshot keys and never touches the transaction collection.
*
* A move counts too: the report row owes its count and total to the snapshot, so a transaction never on screen still invalidates it.
*/
function hasChangedTransactionInSearchResults(
transactions: OnyxCollection<Transaction>,
Expand All @@ -399,11 +401,17 @@ function hasChangedTransactionInSearchResults(
return false;
}

const isReportInSearchResults = (reportID: string | undefined) => !!reportID && !!searchResultsData[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];

const changedTransactionIDs: string[] = [];
for (const [key, transaction] of Object.entries(transactions ?? {})) {
if (!transaction?.transactionID || !previousTransactionKeys.has(key) || previousTransactions?.[key] === transaction) {
const previousTransaction = previousTransactions?.[key];
if (!transaction?.transactionID || !previousTransactionKeys.has(key) || previousTransaction === transaction) {
continue;
}
if (transaction.reportID !== previousTransaction?.reportID && (isReportInSearchResults(transaction.reportID) || isReportInSearchResults(previousTransaction?.reportID))) {
return true;
}
changedTransactionIDs.push(transaction.transactionID);
}

Expand Down
20 changes: 20 additions & 0 deletions src/pages/DynamicReportChangeWorkspacePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import ActivityIndicator from '@components/ActivityIndicator';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import {useSession} from '@components/OnyxListItemProvider';
import ScreenWrapper from '@components/ScreenWrapper';
import {useSearchQueryContext, useSearchResultsContext} from '@components/Search/SearchContext';
import SelectionList from '@components/SelectionList';
import type {WorkspaceListItemType} from '@components/SelectionList/ListItem/types';
import UserListItem from '@components/SelectionList/ListItem/UserListItem';
Expand All @@ -19,6 +20,7 @@ import useParentReportAction from '@hooks/useParentReportAction';
import usePermissions from '@hooks/usePermissions';
import useReportIsArchived from '@hooks/useReportIsArchived';
import useReportTransactions from '@hooks/useReportTransactions';
import useSearchShouldCalculateTotals from '@hooks/useSearchShouldCalculateTotals';
import useShouldSuppressPromotionalUI from '@hooks/useShouldSuppressPromotionalUI';
import useThemeStyles from '@hooks/useThemeStyles';
import useWorkspaceList from '@hooks/useWorkspaceList';
Expand All @@ -38,6 +40,7 @@ import {
isSettled,
isWorkspaceEligibleForReportChange,
} from '@libs/ReportUtils';
import refreshSearchAfterReportAction from '@libs/SearchRefreshUtils';
import {shouldRestrictUserBillableActions} from '@libs/SubscriptionUtils';
import {hasAppliedCommuterExclusion, isManualDistanceRequest, isOdometerDistanceRequest} from '@libs/TransactionUtils';

Expand Down Expand Up @@ -110,6 +113,20 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace
isOdometerDistanceRequest: hasOdometerDistanceRequest,
});
const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector});
const {currentSearchQueryJSON, currentSearchKey} = useSearchQueryContext();
const {currentSearchResults} = useSearchResultsContext();
const shouldCalculateTotals = useSearchShouldCalculateTotals(currentSearchKey, currentSearchQueryJSON?.hash, true);

// The snapshot keeps the report row after a workspace change, and only the server can tell whether it still matches the query.
const refreshSearch = () => {
refreshSearchAfterReportAction({
currentSearchQueryJSON,
currentSearchKey,
shouldCalculateTotals,
isOffline,
isLoading: !!currentSearchResults?.search?.isLoading,
Comment thread
BartekObudzinski marked this conversation as resolved.
});
Comment on lines +116 to +128

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not going to lead to many extra Search calls?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, it's one call per workspace pick. It only runs when you tap a row in that list, and if a search for the same hash and offset is already in flight, search method throws it away anyway

};

const selectPolicy = (policyID?: string) => {
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
Expand Down Expand Up @@ -140,6 +157,7 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace
if (!invite?.policyExpenseChatReportID) {
moveIOUReportToPolicy(report, policy, reportPreviewAction, getCurrencyDecimals, false, reportTransactions);
}
refreshSearch();
return;
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
}
Expand Down Expand Up @@ -169,6 +187,7 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace
isTrackIntentUser,
reportTransactions,
});
refreshSearch();
return;
}

Expand All @@ -189,6 +208,7 @@ function DynamicReportChangeWorkspacePage({report}: DynamicReportChangeWorkspace
isTrackIntentUser,
reportTransactions,
});
refreshSearch();
};

const {data, shouldShowNoResultsFoundMessage, shouldShowSearchInput} = useWorkspaceList({
Expand Down
54 changes: 54 additions & 0 deletions tests/unit/useSearchHighlightAndScrollTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,60 @@ describe('useSearchHighlightAndScroll', () => {
expect(search).not.toHaveBeenCalled();
});

it('should trigger search when a transaction moves into a report the results display', () => {
const movedTransaction = createMock<Transaction>({transactionID: '99', reportID: '5'});
const initialProps = createMock<UseSearchHighlightAndScroll>({
...baseProps,
searchResults: {
...baseProps.searchResults,
data: {
report_2: {reportID: '2'},
},
},
transactions: {transactions_99: movedTransaction},
previousTransactions: {transactions_99: movedTransaction},
});

const {rerender} = renderHook((props: UseSearchHighlightAndScroll) => useSearchHighlightAndScroll(props), {
initialProps,
});

const updatedProps = createMock<UseSearchHighlightAndScroll>({
...initialProps,
transactions: {transactions_99: {transactionID: '99', reportID: '2'}},
});

rerender(updatedProps);
expect(search).toHaveBeenCalledWith({queryJSON: baseProps.queryJSON, searchKey: undefined, offset: 0, shouldCalculateTotals: false, isLoading: false});
});

it('should not trigger search when a transaction moves between reports the results do not display', () => {
const movedTransaction = createMock<Transaction>({transactionID: '99', reportID: '5'});
const initialProps = createMock<UseSearchHighlightAndScroll>({
...baseProps,
searchResults: {
...baseProps.searchResults,
data: {
report_2: {reportID: '2'},
},
},
transactions: {transactions_99: movedTransaction},
previousTransactions: {transactions_99: movedTransaction},
});

const {rerender} = renderHook((props: UseSearchHighlightAndScroll) => useSearchHighlightAndScroll(props), {
initialProps,
});

const updatedProps = createMock<UseSearchHighlightAndScroll>({
...initialProps,
transactions: {transactions_99: {transactionID: '99', reportID: '7'}},
});

rerender(updatedProps);
expect(search).not.toHaveBeenCalled();
});

it('should trigger the deferred search once Search is active again, after previousTransactions caught up', () => {
const transaction = createMock<Transaction>({transactionID: '1', amount: 100});
const editedTransaction = createMock<Transaction>({transactionID: '1', amount: 250});
Expand Down
Loading