-
Notifications
You must be signed in to change notification settings - Fork 4k
fix: offline merge visibility for track distance expenses #77862
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
574ebd5
7228ff6
c6e12e1
db84cff
6929a80
1dc0349
afdc078
a8397a4
42c6138
84bb0b3
747fc64
5e8a6aa
a88f620
a1d812a
1baf83f
28e1958
b5a1560
3872fca
6481605
e57575d
ea95f53
0fa13c8
8179697
57eef21
104414d
00b1920
48b9ca4
570bae4
2604eeb
ad4a9da
a571e6c
e21d81a
35b913d
2ebf5ca
2605993
039f40d
6fb5718
d27a04f
81d8f01
b358dbf
b05089f
3a97fe7
648fba6
7955f3d
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 |
|---|---|---|
|
|
@@ -7,13 +7,12 @@ | |
| import type {ValueOf} from 'type-fest'; | ||
| import type {LocaleContextProps, LocalizedTranslate} from '@components/LocaleContextProvider'; | ||
| import usePrevious from '@hooks/usePrevious'; | ||
| import {isHarvestCreatedExpenseReport, isPolicyExpenseChat} from '@libs/ReportUtils'; | ||
| import CONST from '@src/CONST'; | ||
| import IntlStore from '@src/languages/IntlStore'; | ||
| import type {TranslationPaths} from '@src/languages/types'; | ||
| import ONYXKEYS from '@src/ONYXKEYS'; | ||
| import ROUTES from '@src/ROUTES'; | ||
| import type {Card, OnyxInputOrEntry, OriginalMessageIOU, PersonalDetails, Policy, PrivatePersonalDetails, ReportMetadata, ReportNameValuePairs} from '@src/types/onyx'; | ||
| import type {Card, OnyxInputOrEntry, OriginalMessageIOU, PersonalDetails, Policy, PrivatePersonalDetails, ReportMetadata, ReportNameValuePairs, Transaction} from '@src/types/onyx'; | ||
| import type { | ||
| JoinWorkspaceResolution, | ||
| OriginalMessageChangeLog, | ||
|
|
@@ -43,6 +42,12 @@ | |
| import Parser from './Parser'; | ||
| import {arePersonalDetailsMissing, getEffectiveDisplayName, getPersonalDetailByEmail, getPersonalDetailsByIDs} from './PersonalDetailsUtils'; | ||
| import {getPolicy, isPolicyAdmin as isPolicyAdminPolicyUtils} from './PolicyUtils'; | ||
| // This cycle import is safe because the functions imported here don't create initialization-time dependencies. | ||
| // ReportActionsUtils imports utility functions from ReportUtils, and ReportUtils imports utility functions from ReportActionsUtils. | ||
| // Some functions use module-level variables (e.g., allReports, allReportActions) that are initialized asynchronously via Onyx.connect(), so there's no circular dependency during module initialization. | ||
| // eslint-disable-next-line import/no-cycle | ||
| import {getReportOrDraftReport, isExpenseReport, isHarvestCreatedExpenseReport, isPolicyExpenseChat} from './ReportUtils'; | ||
| // eslint-disable-next-line import/no-cycle | ||
| import type {getReportName, OptimisticIOUReportAction, PartialReportAction} from './ReportUtils'; | ||
| import StringUtils from './StringUtils'; | ||
| import {getReportFieldTypeTranslationKey} from './WorkspaceReportFieldUtils'; | ||
|
|
@@ -66,7 +71,7 @@ | |
| type MemberChangeMessageElement = MessageTextElement | MemberChangeMessageUserMentionElement | MemberChangeMessageRoomReferenceElement; | ||
|
|
||
| let allReportActions: OnyxCollection<ReportActions>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, | ||
| waitForCollectionCallback: true, | ||
| callback: (actions) => { | ||
|
|
@@ -78,7 +83,7 @@ | |
| }); | ||
|
|
||
| let allReports: OnyxCollection<Report>; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -87,13 +92,13 @@ | |
| }); | ||
|
|
||
| let isNetworkOffline = false; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.NETWORK, | ||
| callback: (val) => (isNetworkOffline = val?.isOffline ?? false), | ||
| }); | ||
|
|
||
| let deprecatedCurrentUserAccountID: number | undefined; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.SESSION, | ||
| callback: (value) => { | ||
| // When signed out, value is undefined | ||
|
|
@@ -1523,6 +1528,82 @@ | |
| return !!IOUTransactionID && reportTransactionIDs.includes(IOUTransactionID); | ||
| }; | ||
|
|
||
| /** | ||
| * Checks if a transaction has a valid pending action for expense report filtering. | ||
| * When online, transactions with ADD or UPDATE pending action are included (to keep them visible during optimistic edits). | ||
| * When offline, transactions with ADD pending action are included. | ||
| */ | ||
| function hasValidPendingActionForExpenseReport(transaction: Transaction, isOffline: boolean): boolean { | ||
| return ( | ||
| !transaction.pendingAction || | ||
| transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD || | ||
| (!isOffline && transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE) | ||
| ); | ||
| } | ||
|
Comment on lines
+1536
to
+1542
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.
The new Useful? React with 👍 / 👎.
Author
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. Fixed: Updated |
||
|
|
||
| /** | ||
| * Filters transactions for an expense report and returns their transaction IDs. | ||
| * When online, excludes transactions with pending actions (except ADD when offline). | ||
| * | ||
| * @param allTransactions - Collection of all transactions from Onyx | ||
| * @param iouReportID - The expense report ID to filter transactions for | ||
| * @param transactionID - Optional specific transaction ID to include if it matches the report | ||
| * @param isOffline - Whether the app is currently offline (affects which pending actions are considered valid) | ||
| * @returns Array of transaction IDs that belong to the expense report | ||
| */ | ||
| function getExpenseReportTransactionIDs(allTransactions: OnyxCollection<Transaction>, iouReportID: string, transactionID: string | undefined, isOffline: boolean): string[] { | ||
| if (!iouReportID) { | ||
| return []; | ||
| } | ||
|
|
||
| const expenseReportTransactions = Object.values(allTransactions ?? {}).filter((transaction) => { | ||
| if (!transaction) { | ||
| return false; | ||
| } | ||
| const matchesReportID = transaction.reportID === iouReportID; | ||
| return matchesReportID && hasValidPendingActionForExpenseReport(transaction, isOffline); | ||
| }); | ||
|
|
||
| const transactionIDsToCheck = expenseReportTransactions.map((transaction) => transaction?.transactionID).filter((id): id is string => !!id); | ||
|
|
||
| if (transactionID) { | ||
| const transactionInOnyx = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; | ||
| if (transactionInOnyx && transactionInOnyx.reportID === iouReportID && !transactionIDsToCheck.includes(transactionID)) { | ||
| if (hasValidPendingActionForExpenseReport(transactionInOnyx, isOffline)) { | ||
| transactionIDsToCheck.push(transactionID); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return transactionIDsToCheck; | ||
| } | ||
|
|
||
| /** | ||
| * Determines which transaction IDs should be checked for an IOU action. | ||
| */ | ||
| function getTransactionIDsForIOUAction(reportAction: ReportAction, reportTransactionIDs: string[], allTransactions: OnyxCollection<Transaction>, isOffline: boolean): string[] { | ||
| if (!isMoneyRequestAction(reportAction)) { | ||
| return reportTransactionIDs; | ||
| } | ||
|
|
||
| const originalMessage = getOriginalMessage(reportAction); | ||
| const iouReportID = originalMessage?.IOUReportID; | ||
| const transactionID = originalMessage?.IOUTransactionID; | ||
|
|
||
| if (!iouReportID || !transactionID) { | ||
| return reportTransactionIDs; | ||
| } | ||
|
|
||
| const iouReport = getReportOrDraftReport(iouReportID); | ||
| const isIOUReportExpense = isExpenseReport(iouReport); | ||
|
|
||
| if (isIOUReportExpense) { | ||
| return getExpenseReportTransactionIDs(allTransactions, iouReportID, transactionID, isOffline); | ||
| } | ||
|
|
||
| return reportTransactionIDs; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the report action for the transaction thread associated with a report by iterating over the reportActions and identifying the IOU report actions. | ||
| * Returns a report action if there is exactly one transaction thread for the report, and undefined otherwise. | ||
|
|
@@ -3331,11 +3412,11 @@ | |
| return connectionName ? translate('report.actions.type.removedConnection', {connectionName}) : ''; | ||
| } | ||
|
|
||
| function getRenamedAction(translate: LocalizedTranslate, reportAction: OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.RENAMED>>, isExpenseReport: boolean, actorName?: string) { | ||
| function getRenamedAction(translate: LocalizedTranslate, reportAction: OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.RENAMED>>, isExpenseReportParam: boolean, actorName?: string) { | ||
|
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. Why did we rename it?
Author
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. Naming conflict fix. |
||
| const originalMessage = getOriginalMessage(reportAction); | ||
| return translate('newRoomPage.renamedRoomAction', { | ||
| actorName, | ||
| isExpenseReport, | ||
| isExpenseReport: isExpenseReportParam, | ||
| oldName: originalMessage?.oldName ?? '', | ||
| newName: originalMessage?.newName ?? '', | ||
| }); | ||
|
|
@@ -3712,6 +3793,7 @@ | |
| getAllReportActions, | ||
| getCombinedReportActions, | ||
| getDismissedViolationMessageText, | ||
| getExpenseReportTransactionIDs, | ||
| getFirstVisibleReportActionID, | ||
| getIOUActionForReportID, | ||
| getIOUActionForTransactionID, | ||
|
|
@@ -3744,6 +3826,7 @@ | |
| getSortedReportActions, | ||
| getSortedReportActionsForDisplay, | ||
| getTextFromHtml, | ||
| getTransactionIDsForIOUAction, | ||
| getTrackExpenseActionableWhisper, | ||
| getWhisperedTo, | ||
| hasRequestFromCurrentAccount, | ||
|
|
||
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.
Let's add the explanation when we want to disable lint rule
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.
Done! I'll also explore the possibility of getting rid of these cycles in a separate proposal.
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.
Can you try
@libs/ReportUtils? We used before without any lint error so I think we can keep the same