Remove SearchTransaction > isActionLoading #74404
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -617,6 +617,7 @@ function getSuggestedSearchesVisibility( | |
| * Returns a list of properties that are common to every Search ListItem | ||
| */ | ||
| function getTransactionItemCommonFormattedProperties( | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| transactionItem: SearchTransaction, | ||
| from: OnyxTypes.PersonalDetails, | ||
| to: OnyxTypes.PersonalDetails, | ||
|
|
@@ -766,12 +767,14 @@ function isAmountTooLong(amount: number, maxLength = 8): boolean { | |
| return Math.abs(amount).toString().length >= maxLength; | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| function isTransactionAmountTooLong(transactionItem: TransactionListItemType | SearchTransaction | OnyxTypes.Transaction) { | ||
| // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||
| const amount = Math.abs(transactionItem.modifiedAmount || transactionItem.amount); | ||
| return isAmountTooLong(amount); | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| function isTransactionTaxAmountTooLong(transactionItem: TransactionListItemType | SearchTransaction | OnyxTypes.Transaction) { | ||
| // it won't matter if pass true or false as second argument to getTaxAmount here because isAmountTooLong function uses Math.abs on the returned value of getTaxAmount | ||
| const taxAmount = getTaxAmount(transactionItem, false); | ||
|
|
@@ -785,6 +788,7 @@ function getWideAmountIndicators(data: TransactionListItemType[] | TransactionGr | |
| let isAmountWide = false; | ||
| let isTaxAmountWide = false; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| const processTransaction = (transaction: TransactionListItemType | SearchTransaction) => { | ||
| isAmountWide ||= isTransactionAmountTooLong(transaction); | ||
| isTaxAmountWide ||= isTransactionTaxAmountTooLong(transaction); | ||
|
|
@@ -908,6 +912,7 @@ function getIOUReportName(data: OnyxTypes.SearchResults['data'], reportItem: Tra | |
|
|
||
| function getTransactionViolations( | ||
| allViolations: OnyxCollection<OnyxTypes.TransactionViolation[]>, | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| transaction: SearchTransaction, | ||
| currentUserEmail: string, | ||
| ): OnyxTypes.TransactionViolation[] { | ||
|
|
@@ -1016,6 +1021,7 @@ function getTransactionsSections( | |
| currentAccountID: number | undefined, | ||
| currentUserEmail: string, | ||
| formatPhoneNumber: LocaleContextProps['formatPhoneNumber'], | ||
| isActionLoadingSet: ReadonlySet<string> | undefined, | ||
| ): TransactionListItemType[] { | ||
| const shouldShowMerchant = getShouldShowMerchant(data); | ||
| const doesDataContainAPastYearTransaction = shouldShowYear(data); | ||
|
|
@@ -1040,7 +1046,9 @@ function getTransactionsSections( | |
| const report = data[`${ONYXKEYS.COLLECTION.REPORT}${transactionItem.reportID}`] as SearchReport | undefined; | ||
|
|
||
| let shouldShow = true; | ||
|
|
||
| const isActionLoading = isActionLoadingSet?.has(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${transactionItem.reportID}`); | ||
| if (queryJSON && !isActionLoading) { | ||
| if (queryJSON.type === CONST.SEARCH.DATA_TYPES.EXPENSE) { | ||
| const status = queryJSON.status; | ||
| if (Array.isArray(status)) { | ||
|
|
@@ -1109,10 +1117,15 @@ function getTransactionsSections( | |
| * Retrieves all transactions associated with a specific report ID from the search data. | ||
|
|
||
| */ | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| function getTransactionsForReport(data: OnyxTypes.SearchResults['data'], reportID: string): SearchTransaction[] { | ||
| return ( | ||
| Object.entries(data) | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| .filter(([key, value]) => isTransactionEntry(key) && (value as SearchTransaction)?.reportID === reportID) | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| .map(([, value]) => value as SearchTransaction) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1227,6 +1240,7 @@ function getActions( | |
| } | ||
|
|
||
| const allActions: SearchTransactionAction[] = []; | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| let allReportTransactions: SearchTransaction[]; | ||
| if (isReportEntry(key)) { | ||
| allReportTransactions = getTransactionsForReport(data, report.reportID); | ||
|
|
@@ -1497,7 +1511,9 @@ function getReportSections( | |
| const actions = reportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportItem.reportID}`]; | ||
|
|
||
| let shouldShow = true; | ||
|
|
||
| const isActionLoading = isActionLoadingSet?.has(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportItem.reportID}`); | ||
|
Contributor
There was a problem hiding this comment. ❌ Logic Error (context)Critical bug: The condition logic was inverted incorrectly. The original code checked if the report IS loading: if (queryJSON && isActionLoadingSet?.has(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportItem.reportID}`))But the refactored code checks if it's NOT loading: if (queryJSON && !isActionLoading)This reverses the intended behavior and will cause reports to be hidden when they're loading, and shown when they shouldn't be filtered. Fix: Change line 1543 from: if (queryJSON && !isActionLoading) {to: if (queryJSON && isActionLoading) {
Member
Author
There was a problem hiding this comment. The original logic actually checked if not loading. It was changed here but I think it was a mistake cc @DylanDylann |
||
| if (queryJSON && !isActionLoading) { | ||
| if (queryJSON.type === CONST.SEARCH.DATA_TYPES.EXPENSE) { | ||
| const status = queryJSON.status; | ||
|
|
||
|
|
@@ -1749,7 +1765,7 @@ function getSections({ | |
| } | ||
| } | ||
|
|
||
| return getTransactionsSections(data, currentSearch, currentAccountID, currentUserEmail, formatPhoneNumber, isActionLoadingSet); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1925,6 +1941,7 @@ function isSearchResultsEmpty(searchResults: SearchResults, groupBy?: SearchGrou | |
| return !Object.keys(searchResults?.data).some( | ||
| (key) => | ||
| key.startsWith(ONYXKEYS.COLLECTION.TRANSACTION) && | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| (searchResults?.data[key as keyof typeof searchResults.data] as SearchTransaction)?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE, | ||
| ); | ||
| } | ||
|
|
@@ -2432,6 +2449,7 @@ function getColumnsToShow( | |
| }; | ||
|
|
||
| const {moneyRequestReportActionsByTransactionID} = Array.isArray(data) ? {} : createReportActionsLookupMaps(data); | ||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| const updateColumns = (transaction: OnyxTypes.Transaction | SearchTransaction) => { | ||
| const merchant = transaction.modifiedMerchant ? transaction.modifiedMerchant : (transaction.merchant ?? ''); | ||
| if ((merchant !== '' && merchant !== CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT) || isScanning(transaction)) { | ||
|
|
@@ -2470,6 +2488,7 @@ function getColumnsToShow( | |
| columns[CONST.REPORT.TRANSACTION_LIST.COLUMNS.FROM] = true; | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| const toFieldValue = getToFieldValueForTransaction(transaction as SearchTransaction, report, data.personalDetailsList, reportAction); | ||
| if (toFieldValue.accountID && toFieldValue.accountID !== currentAccountID && !columns[CONST.REPORT.TRANSACTION_LIST.COLUMNS.TO]) { | ||
| columns[CONST.REPORT.TRANSACTION_LIST.COLUMNS.TO] = !!report && !isOpenReport(report); | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
❌ Logic Error (context)
Critical bug: The condition logic was inverted incorrectly. The original code checked if the transaction IS loading (
if (queryJSON && !transactionItem.isActionLoading)), meaning it would apply status filtering when the transaction is NOT loading.The refactored code also checks
if (queryJSON && !isActionLoading), but the logic differs from the original in thegetReportSectionsfunction at line 1543. To maintain consistency with the original behavior:if (queryJSON && isActionLoading)Based on the original
getReportSectionscode checkingisActionLoadingSet?.has(...)(meaning IS loading), there's an inconsistency that needs clarification. Please verify the intended behavior across both functions.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.
The original condition checked if not loading and that's still the case with the new logic.