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
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ type MoneyRequestReportListProps = {
/** List of transactions belonging to this report */
transactions?: OnyxTypes.Transaction[];

/** Whether there is a pending delete transaction */
hasPendingDeletionTransaction?: boolean;

/** List of transactions that arrived when the report was open */
newTransactions: OnyxTypes.Transaction[];

Expand Down Expand Up @@ -122,6 +125,7 @@ function MoneyRequestReportActionsList({
violations,
hasNewerActions,
hasOlderActions,
hasPendingDeletionTransaction,
showReportActionsLoadingState,
}: MoneyRequestReportListProps) {
const styles = useThemeStyles();
Expand Down Expand Up @@ -746,6 +750,7 @@ function MoneyRequestReportActionsList({
report={report}
transactions={transactions}
newTransactions={newTransactions}
hasPendingDeletionTransaction={hasPendingDeletionTransaction}
reportActions={reportActions}
violations={violations}
hasComments={reportHasComments}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ type MoneyRequestReportTransactionListProps = {
/** List of transactions belonging to one report */
transactions: OnyxTypes.Transaction[];

/** Whether there is a pending delete transaction */
hasPendingDeletionTransaction?: boolean;

/** List of transactions that arrived when the report was open */
newTransactions: OnyxTypes.Transaction[];

Expand Down Expand Up @@ -122,6 +125,7 @@ function MoneyRequestReportTransactionList({
violations,
hasComments,
isLoadingInitialReportActions: isLoadingReportActions,
hasPendingDeletionTransaction = false,
scrollToNewTransaction,
}: MoneyRequestReportTransactionListProps) {
useCopySelectionHelper();
Expand All @@ -141,8 +145,8 @@ function MoneyRequestReportTransactionList({
const session = useSession();

const hasPendingAction = useMemo(() => {
return transactions.some(getTransactionPendingAction);
}, [transactions]);
return hasPendingDeletionTransaction || transactions.some(getTransactionPendingAction);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain why we need this condition? Doesn't transactions.some(getTransactionPendingAction) cover pending delete?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dukenv0307 we are already filtering out deleted transactions here

const reportTransactions = useMemo(() => getAllNonDeletedTransactions(allReportTransactions, reportActions), [allReportTransactions, reportActions]);

Copy link
Contributor

Choose a reason for hiding this comment

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

but we just need to grey out only if the total can't be recalculated, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@trjExpensify is suggesting to follow the pattern we use for new expense creation and we grey it out until the BE clears the pending action currently, no matter the condition of the recalculation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry @FitseTLT, I'm still quite confused.

Based on the confirmation here, in offline mode, we want to grey out the amount only when the total can't be recalculated.
image

But with the current implementation, we always grey out even though the amount is updated (IOU currency is the same as expense currency)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dukenv0307 I am trying to follow the current pattern we are using. On main, when adding a new expense with the same currency as the iou report it is greyed out even online until the BE response clears the pending field.

const hasPendingAction = useMemo(() => {
return transactions.some(getTransactionPendingAction);
}, [transactions]);

<Text style={[shouldUseNarrowLayout ? styles.mnw64p : styles.mnw100p, styles.textAlignRight, styles.textBold, hasPendingAction && styles.opacitySemiTransparent]}>

}, [hasPendingDeletionTransaction, transactions]);

const {selectedTransactionIDs, setSelectedTransactions, clearSelectedTransactions} = useSearchContext();
const isMobileSelectionModeEnabled = useMobileSelectionMode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe
}, [unfilteredReportActions]);

const {transactions: reportTransactions, violations: allReportViolations} = useTransactionsAndViolationsForReport(reportID);
const hasPendingDeletionTransaction = Object.values(reportTransactions ?? {}).some((transaction) => transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
const transactions = useMemo(() => getAllNonDeletedTransactions(reportTransactions, reportActions), [reportTransactions, reportActions]);

const visibleTransactions = transactions?.filter((transaction) => isOffline || transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
Expand Down Expand Up @@ -227,6 +228,7 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe
report={report}
policy={policy}
transactions={visibleTransactions}
hasPendingDeletionTransaction={hasPendingDeletionTransaction}
newTransactions={newTransactions}
reportActions={reportActions}
violations={allReportViolations}
Expand Down
2 changes: 1 addition & 1 deletion src/hooks/useSelectedTransactionsActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function useSelectedTransactionsActions({
return;
}

deleteMoneyRequest(transactionID, action, duplicateTransactions, duplicateTransactionViolations, false, deletedTransactionIDs);
deleteMoneyRequest(transactionID, action, duplicateTransactions, duplicateTransactionViolations, false, deletedTransactionIDs, selectedTransactionIDs);
deletedTransactionIDs.push(transactionID);
});
clearSelectedTransactions(true);
Expand Down
5 changes: 3 additions & 2 deletions src/libs/IOUUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ function updateIOUOwnerAndTotal<TReport extends OnyxInputOrEntry<Report>>(
isDeleting = false,
isUpdating = false,
isOnHold = false,
unHeldAmount = amount,
): TReport {
// For the update case, we have calculated the diff amount in the calculateDiffAmount function so there is no need to compare currencies here
if ((currency !== iouReport?.currency && !isUpdating) || !iouReport) {
Expand All @@ -109,12 +110,12 @@ function updateIOUOwnerAndTotal<TReport extends OnyxInputOrEntry<Report>>(
if (actorAccountID === iouReport.ownerAccountID) {
iouReportUpdate.total += isDeleting ? -amount : amount;
if (!isOnHold) {
iouReportUpdate.unheldTotal += isDeleting ? -amount : amount;
iouReportUpdate.unheldTotal += isDeleting ? -unHeldAmount : unHeldAmount;
}
} else {
iouReportUpdate.total += isDeleting ? amount : -amount;
if (!isOnHold) {
iouReportUpdate.unheldTotal += isDeleting ? amount : -amount;
iouReportUpdate.unheldTotal += isDeleting ? unHeldAmount : -unHeldAmount;
}
}

Expand Down
5 changes: 5 additions & 0 deletions src/libs/MoneyRequestReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ function getAllNonDeletedTransactions(transactions: OnyxCollection<Transaction>,
if (!transaction) {
return false;
}

if (transaction?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) {
return true;
}

const action = getIOUActionForTransactionID(reportActions, transaction.transactionID);
return !isDeletedParentAction(action) && (reportActions.length === 0 || !isDeletedAction(action));
});
Expand Down
49 changes: 32 additions & 17 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@
};

let allPersonalDetails: OnyxTypes.PersonalDetailsList = {};
Onyx.connect({

Check warning on line 678 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand Down Expand Up @@ -716,13 +716,13 @@
};

let allBetas: OnyxEntry<OnyxTypes.Beta[]>;
Onyx.connect({

Check warning on line 719 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.BETAS,
callback: (value) => (allBetas = value),
});

let allTransactions: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({

Check warning on line 725 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -736,7 +736,7 @@
});

let allTransactionDrafts: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({

Check warning on line 739 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_DRAFT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -745,7 +745,7 @@
});

let allTransactionViolations: NonNullable<OnyxCollection<OnyxTypes.TransactionViolations>> = {};
Onyx.connect({

Check warning on line 748 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -759,7 +759,7 @@
});

let allDraftSplitTransactions: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({

Check warning on line 762 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -768,7 +768,7 @@
});

let allNextSteps: NonNullable<OnyxCollection<OnyxTypes.ReportNextStep>> = {};
Onyx.connect({

Check warning on line 771 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.NEXT_STEP,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -777,14 +777,14 @@
});

let allPolicyCategories: OnyxCollection<OnyxTypes.PolicyCategories> = {};
Onyx.connect({

Check warning on line 780 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY_CATEGORIES,
waitForCollectionCallback: true,
callback: (val) => (allPolicyCategories = val),
});

const allPolicies: OnyxCollection<OnyxTypes.Policy> = {};
Onyx.connect({

Check warning on line 787 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY,
callback: (val, key) => {
if (!key) {
Expand Down Expand Up @@ -817,7 +817,7 @@
});

let allReports: OnyxCollection<OnyxTypes.Report>;
Onyx.connect({

Check warning on line 820 in src/libs/actions/IOU.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand Down Expand Up @@ -7807,7 +7807,13 @@
* @param reportAction - The reportAction of the transaction in the IOU report
* @return the url to navigate back once the money request is deleted
*/
function prepareToCleanUpMoneyRequest(transactionID: string, reportAction: OnyxTypes.ReportAction, shouldRemoveIOUTransactionID = true, transactionIDsPendingDeletion?: string[]) {
function prepareToCleanUpMoneyRequest(
transactionID: string,
reportAction: OnyxTypes.ReportAction,
shouldRemoveIOUTransactionID = true,
transactionIDsPendingDeletion?: string[],
selectedTransactionIDs?: string[],
) {
// STEP 1: Get all collections we're updating
const iouReportID = isMoneyRequestAction(reportAction) ? getOriginalMessage(reportAction)?.IOUReportID : undefined;
const iouReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${iouReportID}`] ?? null;
Expand Down Expand Up @@ -7862,38 +7868,46 @@
const currency = getCurrency(transaction);
const updatedReportPreviewAction: Partial<OnyxTypes.ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW>> = cloneDeep(reportPreviewAction ?? {});
updatedReportPreviewAction.pendingAction = shouldDeleteIOUReport ? CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE : CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE;
if (iouReport && isExpenseReport(iouReport)) {

const transactionPendingDelete = transactionIDsPendingDeletion?.map((id) => allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`]);
const selectedTransactions = selectedTransactionIDs?.map((id) => allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`]);
const canEditTotal = !selectedTransactions?.some((trans) => getCurrency(trans) !== iouReport?.currency);
const isExpenseReportType = isExpenseReport(iouReport);
const amountDiff = getAmount(transaction, isExpenseReportType) + (transactionPendingDelete?.reduce((prev, curr) => prev + getAmount(curr, isExpenseReportType), 0) ?? 0);
const unheldAmountDiff =
getAmount(transaction, isExpenseReportType) + (transactionPendingDelete?.reduce((prev, curr) => prev + (!isOnHold(curr) ? getAmount(curr, isExpenseReportType) : 0), 0) ?? 0);

if (iouReport && isExpenseReportType) {
updatedIOUReport = {...iouReport};

if (typeof updatedIOUReport.total === 'number' && currency === iouReport?.currency) {
if (typeof updatedIOUReport.total === 'number' && currency === iouReport?.currency && canEditTotal) {
// Because of the Expense reports are stored as negative values, we add the total from the amount
const amountDiff = getAmount(transaction, true);
updatedIOUReport.total += amountDiff;

if (!transaction?.reimbursable && typeof updatedIOUReport.nonReimbursableTotal === 'number') {
updatedIOUReport.nonReimbursableTotal += amountDiff;
const nonReimbursableAmountDiff =
getAmount(transaction, true) + (transactionPendingDelete?.reduce((prev, curr) => prev + (!curr?.reimbursable ? getAmount(curr, true) : 0), 0) ?? 0);
updatedIOUReport.nonReimbursableTotal += nonReimbursableAmountDiff;
}

if (!isTransactionOnHold) {
if (typeof updatedIOUReport.unheldTotal === 'number') {
updatedIOUReport.unheldTotal += amountDiff;
updatedIOUReport.unheldTotal += unheldAmountDiff;
}

if (!transaction?.reimbursable && typeof updatedIOUReport.unheldNonReimbursableTotal === 'number') {
updatedIOUReport.unheldNonReimbursableTotal += amountDiff;
const unheldNonReimbursableAmountDiff =
getAmount(transaction, true) +
(transactionPendingDelete?.reduce((prev, curr) => prev + (!isOnHold(curr) && !curr?.reimbursable ? getAmount(curr, true) : 0), 0) ?? 0);
updatedIOUReport.unheldNonReimbursableTotal += unheldNonReimbursableAmountDiff;
}
}
}
} else {
updatedIOUReport = updateIOUOwnerAndTotal(
iouReport,
reportAction.actorAccountID ?? CONST.DEFAULT_NUMBER_ID,
getAmount(transaction, false),
currency,
true,
false,
isTransactionOnHold,
);
updatedIOUReport =
iouReport && !canEditTotal
? {...iouReport}
: updateIOUOwnerAndTotal(iouReport, reportAction.actorAccountID ?? CONST.DEFAULT_NUMBER_ID, amountDiff, currency, true, false, isTransactionOnHold, unheldAmountDiff);
}

if (updatedIOUReport) {
Expand Down Expand Up @@ -8175,6 +8189,7 @@
violations: OnyxCollection<OnyxTypes.TransactionViolations>,
isSingleTransactionView = false,
transactionIDsPendingDeletion?: string[],
selectedTransactionIDs?: string[],
) {
if (!transactionID) {
return;
Expand All @@ -8194,7 +8209,7 @@
transactionViolations,
iouReport,
reportPreviewAction,
} = prepareToCleanUpMoneyRequest(transactionID, reportAction, false, transactionIDsPendingDeletion);
} = prepareToCleanUpMoneyRequest(transactionID, reportAction, false, transactionIDsPendingDeletion, selectedTransactionIDs);

const urlToNavigateBack = getNavigationUrlOnMoneyRequestDelete(transactionID, reportAction, isSingleTransactionView);

Expand Down
2 changes: 2 additions & 0 deletions src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) {
// OpenReport will be called each time the user scrolls up the report a bit, clicks on report preview, and then goes back.
const isLinkedMessagePageReady = isLinkedMessageAvailable && (reportActions.length - indexOfLinkedMessage >= CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT || doesCreatedActionExists());
const {transactions: allReportTransactions, violations: allReportViolations} = useTransactionsAndViolationsForReport(reportIDFromRoute);
const hasPendingDeletionTransaction = Object.values(allReportTransactions ?? {}).some((transaction) => transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);

const reportTransactions = useMemo(() => getAllNonDeletedTransactions(allReportTransactions, reportActions), [allReportTransactions, reportActions]);
// wrapping in useMemo because this is array operation and can cause performance issues
Expand Down Expand Up @@ -882,6 +883,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) {
{!!report && shouldDisplayMoneyRequestActionsList && !shouldWaitForTransactions ? (
<MoneyRequestReportActionsList
report={report}
hasPendingDeletionTransaction={hasPendingDeletionTransaction}
policy={policy}
reportActions={reportActions}
transactions={visibleTransactions}
Expand Down
75 changes: 75 additions & 0 deletions tests/actions/IOUTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ import type {InvoiceTestData} from '../data/Invoice';
import createRandomPolicy, {createCategoryTaxExpenseRules} from '../utils/collections/policies';
import createRandomPolicyCategories from '../utils/collections/policyCategory';
import createRandomPolicyTags from '../utils/collections/policyTags';
import createRandomReportAction from '../utils/collections/reportActions';
import {createRandomReport} from '../utils/collections/reports';
import createRandomTransaction from '../utils/collections/transaction';
import getOnyxValue from '../utils/getOnyxValue';
Expand Down Expand Up @@ -4320,6 +4321,80 @@ describe('actions/IOU', () => {
});
});

describe('bulk deleteMoneyRequest', () => {
it('update IOU report total properly for bulk deletion of expenses', async () => {
const expenseReport: Report = {
...createRandomReport(11),
type: CONST.REPORT.TYPE.EXPENSE,
total: 30,
currency: CONST.CURRENCY.USD,
unheldTotal: 20,
unheldNonReimbursableTotal: 20,
};
const transaction1: Transaction = {
...createRandomTransaction(1),
amount: 10,
comment: {hold: '123'},
currency: CONST.CURRENCY.USD,
reportID: expenseReport.reportID,
reimbursable: true,
};
const moneyRequestAction1: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> = {
...createRandomReportAction(1),
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
childReportID: '1',
originalMessage: {
IOUReportID: expenseReport.reportID,
amount: transaction1.amount,
currency: transaction1.currency,
type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
},
message: undefined,
previousMessage: undefined,
};
const transaction2: Transaction = {...createRandomTransaction(2), amount: 10, currency: CONST.CURRENCY.USD, reportID: expenseReport.reportID, reimbursable: false};
const moneyRequestAction2: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> = {
...createRandomReportAction(2),
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
childReportID: '2',
originalMessage: {
IOUReportID: expenseReport.reportID,
amount: transaction2.amount,
currency: transaction2.currency,
type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
},
message: undefined,
previousMessage: undefined,
};
const transaction3: Transaction = {...createRandomTransaction(3), amount: 10, currency: CONST.CURRENCY.USD, reportID: expenseReport.reportID, reimbursable: false};

await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction1.transactionID}`, transaction1);
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction2.transactionID}`, transaction2);
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction3.transactionID}`, transaction3);
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport);

const selectedTransactionIDs = [transaction1.transactionID, transaction2.transactionID];
deleteMoneyRequest(transaction1.transactionID, moneyRequestAction1, {}, {}, undefined, [], selectedTransactionIDs);
deleteMoneyRequest(transaction2.transactionID, moneyRequestAction2, {}, {}, undefined, [transaction1.transactionID], selectedTransactionIDs);

await waitForBatchedUpdates();

const report = await new Promise<OnyxEntry<Report>>((resolve) => {
const connection = Onyx.connect({
key: `${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`,
callback: (val) => {
Onyx.disconnect(connection);
resolve(val);
},
});
});

expect(report?.total).toBe(10);
expect(report?.unheldTotal).toBe(10);
expect(report?.unheldNonReimbursableTotal).toBe(10);
});
});

describe('submitReport', () => {
it('correctly submits a report', () => {
const amount = 10000;
Expand Down
Loading