Skip to content
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.

Sign up for GitHub

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

fix: loading indicator display when deleting vague distance Expense #51940

Merged
merged 21 commits into from
Dec 11, 2024
Merged

fix: loading indicator display when deleting vague distance Expense #51940

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1fff908
fix: loading indicator display when deleting vague distance Expense
truph01 Nov 4, 2024
41f25da
merge main
truph01 Nov 7, 2024
0fab945
merge main
truph01 Nov 11, 2024
fcf7f3f
remove InteractionManager.runAfterInteractions
truph01 Nov 11, 2024
65b343b
fix: remove unused lib
truph01 Nov 11, 2024
10fb70b
fix: revert remove InteractionManager.runAfterInteractions
truph01 Nov 11, 2024
3ac6241
merge main
truph01 Nov 24, 2024
38b088c
merge main
truph01 Dec 2, 2024
d332cb7
fix: clear action first then navigate back then clear rest
truph01 Dec 3, 2024
12a0a68
fix: add isWeb logic
truph01 Dec 3, 2024
70fb37d
fix: revert unused change
truph01 Dec 3, 2024
173a51e
fix: lint
truph01 Dec 3, 2024
8353a82
fix: lint
truph01 Dec 3, 2024
3ecf918
merge main
truph01 Dec 6, 2024
a90580a
fix: move all update logic to cleanUpMoneyRequest
truph01 Dec 6, 2024
d71f1eb
fix: lint
truph01 Dec 6, 2024
4149884
fix: use promise
truph01 Dec 6, 2024
5117bec
fix: lint
truph01 Dec 6, 2024
9d570a1
fix: remove param
truph01 Dec 6, 2024
cc1ef7f
merge main
truph01 Dec 9, 2024
f104ffd
fix: remove variable and remove unused export
truph01 Dec 9, 2024
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
17 changes: 14 additions & 3 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {useCallback, useMemo} from 'react';
import {View} from 'react-native';
import {InteractionManager, View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import * as Expensicons from '@components/Icon/Expensicons';
Expand All @@ -21,6 +21,7 @@ import useViolations from '@hooks/useViolations';
import type {ViolationField} from '@hooks/useViolations';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import DistanceRequestUtils from '@libs/DistanceRequestUtils';
import getPlatform from '@libs/getPlatform';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as PolicyUtils from '@libs/PolicyUtils';
import {isTaxTrackingEnabled} from '@libs/PolicyUtils';
Expand Down Expand Up @@ -468,8 +469,18 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals
return;
}
if (parentReportAction) {
const urlToNavigateBack = IOU.cleanUpMoneyRequest(transaction?.transactionID ?? linkedTransactionID, parentReportAction, true);
Navigation.goBack(urlToNavigateBack);
const isWeb = getPlatform() === CONST.PLATFORM.WEB;
const {urlToNavigateBack} = IOU.prepareToCleanUpMoneyRequest(transaction?.transactionID ?? linkedTransactionID, parentReportAction, true);
IOU.cleanUpMoneyRequest(transaction?.transactionID ?? linkedTransactionID, parentReportAction, true, true);
if (isWeb) {
Navigation.goBack(urlToNavigateBack);
}
InteractionManager.runAfterInteractions(() => {
if (!isWeb) {
Navigation.goBack(urlToNavigateBack);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@truph01 Is there any specific reason for handling web differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During testing, I noticed differences in processing speed in Onyx and navigation between web versus native platforms. Therefore, I need to account for the specific platform in this case.

IOU.cleanUpMoneyRequest(transaction?.transactionID ?? linkedTransactionID, parentReportAction, true, false);
});
return;
}
}
Expand Down
41 changes: 22 additions & 19 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5587,9 +5587,10 @@ function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxT
* @param transactionID - The transactionID of IOU
* @param reportAction - The reportAction of the transaction in the IOU report
* @param isSingleTransactionView - whether we are in the transaction thread report
* @param onlyCleanupActions - whether we only clear the reportActions data
* @return the url to navigate back once the money request is deleted
*/
function cleanUpMoneyRequest(transactionID: string, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false) {
function cleanUpMoneyRequest(transactionID: string, reportAction: OnyxTypes.ReportAction, isSingleTransactionView = false, onlyCleanupActions = false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of calling cleanUpMoneyRequest twice with onlyCleanupActions toggled between true and false, it would be more efficient to manage the Onyx updates within a single invocation of cleanUpMoneyRequest. The method should first prepare and apply updates relevant to report actions using Onyx.update(onyxReportActionsUpdates). After navigating back with Navigation.goBack(urlToNavigateBack);, it should then prepare and apply the remaining updates with Onyx.update(onyxRemainingUpdates). This change eliminates the need for cleanUpMoneyRequest to return urlToNavigateBack, as navigation can be handled directly after the initial Onyx update, simplifying the flow and enhancing readability. A comment should be added within the code to clarify the sequence of operations.

const {
shouldDeleteTransactionThread,
shouldDeleteIOUReport,
Expand All @@ -5606,27 +5607,28 @@ function cleanUpMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repo
// build Onyx data

// Onyx operations to delete the transaction, update the IOU report action and chat report action
const onyxUpdatesActions: OnyxUpdate[] = [];
truph01 marked this conversation as resolved.
Show resolved Hide resolved
const onyxUpdates: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.SET,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: null,
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport?.reportID}`,
value: {
[reportAction.reportActionID]: shouldDeleteIOUReport
? null
: {
pendingAction: null,
},
},
},
];
onyxUpdatesActions.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport?.reportID}`,
value: {
[reportAction.reportActionID]: shouldDeleteIOUReport
? null
: {
pendingAction: null,
},
},
});

if (reportPreviewAction?.reportActionID) {
onyxUpdates.push({
onyxUpdatesActions.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport?.reportID}`,
value: {
Expand Down Expand Up @@ -5663,12 +5665,12 @@ function cleanUpMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repo
}

// added operations to update IOU report and chat report
onyxUpdatesActions.push({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport?.reportID}`,
value: updatedReportAction,
});
onyxUpdates.push(
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport?.reportID}`,
value: updatedReportAction,
},
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.reportID}`,
Expand Down Expand Up @@ -5719,7 +5721,7 @@ function cleanUpMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repo
);
}

Onyx.update(onyxUpdates);
Onyx.update(onlyCleanupActions ? onyxUpdatesActions : onyxUpdates);

return urlToNavigateBack;
}
Expand Down Expand Up @@ -8550,6 +8552,7 @@ export {
completePaymentOnboarding,
payInvoice,
payMoneyRequest,
prepareToCleanUpMoneyRequest,
truph01 marked this conversation as resolved.
Show resolved Hide resolved
putOnHold,
replaceReceipt,
requestMoney,
Expand Down
Loading