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
2 changes: 1 addition & 1 deletion src/hooks/useSearchBulkActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2060,7 +2060,7 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) {

const firstTransactionKey = selectedTransactionsKeys.at(0);
const firstTransactionMeta = firstTransactionKey ? selectedTransactions[firstTransactionKey] : undefined;
const canShowDeleteAction = shouldShowDeleteOption(selectedTransactions, currentSearchResults?.data, selectedReports, queryJSON?.type);
const canShowDeleteAction = shouldShowDeleteOption(selectedTransactions, currentSearchResults?.data, accountID, selectedReports, queryJSON?.type);

const isSplittable = !!firstTransactionMeta?.canSplit;
const isAlreadySplit = !!firstTransactionMeta?.hasBeenSplit;
Expand Down
18 changes: 11 additions & 7 deletions src/libs/ReportSecondaryActionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,8 @@ function isChangeWorkspaceAction(report: Report, policies: OnyxCollection<Policy
return hasAvailablePolicies && canEditReportPolicy(report, reportPolicy) && !isExportedUtils(reportActions, report);
}

function isDeleteAction(report: Report, reportTransactions: Transaction[], reportActions?: ReportAction[]): boolean {
return canDeleteMoneyRequestReport(report, reportTransactions, reportActions ?? []);
function isDeleteAction(report: Report, reportTransactions: Transaction[], currentUserAccountID: number, reportActions?: ReportAction[]): boolean {
return canDeleteMoneyRequestReport(report, reportTransactions, reportActions ?? [], currentUserAccountID);
}

function shouldShowEditSplitInDeleteAction(
Expand All @@ -688,6 +688,7 @@ function shouldShowEditSplitInDeleteAction(
reportActions: ReportAction[] | undefined,
originalTransaction: OnyxEntry<Transaction>,
isProduction: boolean,
currentUserAccountID: number,
): boolean {
if (reportTransactions.length !== 1) {
return false;
Expand All @@ -699,7 +700,10 @@ function shouldShowEditSplitInDeleteAction(
}

const isSelfDMSplit = isSelfDMReportUtils(report);
return shouldRedirectDeleteToSplitExpenseEdit(reportTransaction, originalTransaction, isSelfDMSplit, isProduction) && isDeleteAction(report, reportTransactions, reportActions);
return (
shouldRedirectDeleteToSplitExpenseEdit(reportTransaction, originalTransaction, isSelfDMSplit, isProduction) &&
isDeleteAction(report, reportTransactions, currentUserAccountID, reportActions)
);
}

function isRetractAction(report: Report, policy?: Policy): boolean {
Expand Down Expand Up @@ -1051,7 +1055,7 @@ function getSecondaryReportActions({

if (
isSplitAction(report, reportTransactions, originalTransaction, currentUserLogin, currentUserAccountID, policy, parentReport, isProduction) &&
!shouldShowEditSplitInDeleteAction(report, reportTransactions, reportActions, originalTransaction, isProduction)
!shouldShowEditSplitInDeleteAction(report, reportTransactions, reportActions, originalTransaction, isProduction, currentUserAccountID)
) {
options.push(CONST.REPORT.SECONDARY_ACTIONS.SPLIT);
}
Expand Down Expand Up @@ -1106,7 +1110,7 @@ function getSecondaryReportActions({

options.push(CONST.REPORT.SECONDARY_ACTIONS.VIEW_DETAILS);

if (isDeleteAction(report, reportTransactions, reportActions ?? [])) {
if (isDeleteAction(report, reportTransactions, currentUserAccountID, reportActions ?? [])) {
options.push(CONST.REPORT.SECONDARY_ACTIONS.DELETE);
}

Expand Down Expand Up @@ -1185,7 +1189,7 @@ function getSecondaryTransactionThreadActions({

if (
isSplitAction(parentReport, [reportTransaction], originalTransaction, currentUserLogin, currentUserAccountID, policy, grandParentReport, isProduction) &&
!shouldShowEditSplitInDeleteAction(parentReport, [reportTransaction], reportAction ? [reportAction] : [], originalTransaction, isProduction)
!shouldShowEditSplitInDeleteAction(parentReport, [reportTransaction], reportAction ? [reportAction] : [], originalTransaction, isProduction, currentUserAccountID)
) {
options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.SPLIT);
}
Expand Down Expand Up @@ -1216,7 +1220,7 @@ function getSecondaryTransactionThreadActions({

options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.VIEW_DETAILS);

if (isDeleteAction(parentReport, [reportTransaction], reportAction ? [reportAction] : [])) {
if (isDeleteAction(parentReport, [reportTransaction], currentUserAccountID, reportAction ? [reportAction] : [])) {
options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.DELETE);
}

Expand Down
12 changes: 7 additions & 5 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3207,10 +3207,10 @@ function canDeleteCardTransactionByLiabilityType(transaction: OnyxEntry<Transact
return transaction?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.ALLOW;
}

function canDeleteMoneyRequestReport(report: OnyxEntry<Report>, reportTransactions: Transaction[], reportActions: ReportAction[]): boolean {
function canDeleteMoneyRequestReport(report: OnyxEntry<Report>, reportTransactions: Transaction[], reportActions: ReportAction[], currentUserAccountID: number): boolean {
const transaction = reportTransactions.at(0);
const transactionID = transaction?.transactionID;
const isOwner = transactionID ? getIOUActionForTransactionID(reportActions, transactionID)?.actorAccountID === deprecatedCurrentUserAccountID : false;
const isOwner = transactionID ? getIOUActionForTransactionID(reportActions, transactionID)?.actorAccountID === currentUserAccountID : false;
const isReportOpenOrProcessing = isOpenReport(report) || isProcessingReport(report);
const isSingleTransaction = reportTransactions.length === 1;

Expand All @@ -3225,7 +3225,7 @@ function canDeleteMoneyRequestReport(report: OnyxEntry<Report>, reportTransactio
}

if (isInvoiceReport(report)) {
return report?.ownerAccountID === deprecatedCurrentUserAccountID && isReportOpenOrProcessing;
return report?.ownerAccountID === currentUserAccountID && isReportOpenOrProcessing;
}

// Users cannot delete a report in the unreported or IOU cases, but they can delete individual transactions.
Expand All @@ -3239,7 +3239,7 @@ function canDeleteMoneyRequestReport(report: OnyxEntry<Report>, reportTransactio
return false;
}

const isReportSubmitter = isCurrentUserSubmitter(report);
const isReportSubmitter = isCurrentUserSubmitter(report, currentUserAccountID);
return isReportSubmitter && (isOpenReport(report) || (isProcessingReport(report) && isAwaitingFirstLevelApproval(report)));
}

Expand All @@ -3256,9 +3256,10 @@ function canDeleteReportAction(
transaction: OnyxEntry<Transaction> | undefined,
transactions: OnyxCollection<Transaction>,
childReportActions: OnyxCollection<ReportAction>,
currentUserAccountID: number,
): boolean {
const report = getReportOrDraftReport(reportID);
const isActionOwner = reportAction?.actorAccountID === deprecatedCurrentUserAccountID;
const isActionOwner = reportAction?.actorAccountID === currentUserAccountID;
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`] ?? null;

if (isDemoTransaction(transaction)) {
Expand Down Expand Up @@ -3290,6 +3291,7 @@ function canDeleteReportAction(
report,
Object.values(transactions ?? {}).filter((t): t is Transaction => !!t),
Object.values(childReportActions ?? {}).filter((action): action is ReportAction => !!action),
currentUserAccountID,
);
}

Expand Down
5 changes: 3 additions & 2 deletions src/libs/SearchUIUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6370,6 +6370,7 @@ function navigateToSearchRHP(route: {route: string; getRoute: (backTo?: string)
function shouldShowDeleteOption(
selectedTransactions: Record<string, SelectedTransactionInfo>,
currentSearchResults: SearchResults['data'] | undefined,
currentUserAccountID: number,
selectedReports: SelectedReports[] = [],
searchDataType?: SearchDataTypes,
) {
Expand All @@ -6394,7 +6395,7 @@ function shouldShowDeleteOption(
reportTransactions.push(item);
}
}
return canDeleteMoneyRequestReport(fullReport, reportTransactions, reportActionsArray);
return canDeleteMoneyRequestReport(fullReport, reportTransactions, reportActionsArray, currentUserAccountID);
})
: selectedTransactionsKeys.every((id) => {
const transaction = currentSearchResults?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`] ?? selectedTransactions[id]?.transaction;
Expand All @@ -6408,7 +6409,7 @@ function shouldShowDeleteOption(
Object.values(reportActions ?? {}).find((action) => (isMoneyRequestAction(action) ? getOriginalMessage(action)?.IOUTransactionID : undefined) === id) ??
selectedTransactions[id].reportAction;

return canDeleteMoneyRequestReport(parentReport, [transaction], parentReportAction ? [parentReportAction] : []);
return canDeleteMoneyRequestReport(parentReport, [transaction], parentReportAction ? [parentReportAction] : [], currentUserAccountID);
});
}

Expand Down
16 changes: 14 additions & 2 deletions src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,19 @@ const ContextMenuActions: ContextMenuAction[] = [
isAnonymousAction: false,
textTranslateKey: 'common.delete',
icon: 'Trashcan',
shouldShow: ({type, reportAction, isArchivedRoom, isChronosReport, reportID: reportIDParam, moneyRequestAction, iouTransaction, transactions, childReportActions, isProduction}) => {
shouldShow: ({
type,
reportAction,
isArchivedRoom,
isChronosReport,
reportID: reportIDParam,
moneyRequestAction,
iouTransaction,
transactions,
childReportActions,
isProduction,
currentUserAccountID,
}) => {
// Until deleting parent threads is supported in FE, we will prevent the user from deleting a thread parent
let reportID = reportIDParam;

Expand All @@ -1545,7 +1557,7 @@ const ContextMenuActions: ContextMenuAction[] = [
return (
!!reportIDParam &&
type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION &&
canDeleteReportAction(moneyRequestAction ?? reportAction, reportID, iouTransaction, transactions, childReportActions) &&
canDeleteReportAction(moneyRequestAction ?? reportAction, reportID, iouTransaction, transactions, childReportActions, currentUserAccountID) &&
!isArchivedRoom &&
!isChronosReport &&
!isMessageDeleted(reportAction)
Expand Down
2 changes: 1 addition & 1 deletion tests/perf-test/ReportUtils.perf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('ReportUtils', () => {
const reportAction = {...createRandomReportAction(1), actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT} as unknown as ReportAction;

await waitForBatchedUpdates();
await measureFunction(() => canDeleteReportAction(reportAction, reportID, transaction, undefined, undefined));
await measureFunction(() => canDeleteReportAction(reportAction, reportID, transaction, undefined, undefined, 1));
});

test('[ReportUtils] getReportRecipientAccountID on 1k participants', async () => {
Expand Down
61 changes: 30 additions & 31 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5418,7 +5418,7 @@ describe('ReportUtils', () => {
},
});
});
expect(canDeleteMoneyRequestReport(invoiceReport, [], [])).toBe(true);
expect(canDeleteMoneyRequestReport(invoiceReport, [], [], currentUserAccountID)).toBe(true);
});

it('should allow deletion if the expense report is submitted but not yet approved by anyone', async () => {
Expand Down Expand Up @@ -5457,7 +5457,7 @@ describe('ReportUtils', () => {
});
});

expect(canDeleteMoneyRequestReport(expenseReport, [], [])).toBe(true);
expect(canDeleteMoneyRequestReport(expenseReport, [], [], currentUserAccountID)).toBe(true);
});
});

Expand Down Expand Up @@ -8589,35 +8589,21 @@ describe('ReportUtils', () => {
});

describe('canDeleteReportAction', () => {
it('should return false for delete button visibility if transaction is not allowed to be deleted', () => {
const parentReport = LHNTestUtils.getFakeReport();
const report = LHNTestUtils.getFakeReport();
const parentReportAction: ReportAction = {
...LHNTestUtils.getFakeReportAction(),
message: [
{
type: 'COMMENT',
html: 'hey',
text: 'hey',
isEdited: false,
whisperedTo: [],
isDeletedParentAction: false,
moderationDecision: {
decision: CONST.MODERATION.MODERATOR_DECISION_PENDING_REMOVE,
},
},
],
childReportID: report.reportID,
it('should return false for delete button visibility if transaction is not allowed to be deleted', async () => {
// Given a restricted managed-card expense on an open expense report owned by the current user
const expenseReport = {
...LHNTestUtils.getFakeReport(),
type: CONST.REPORT.TYPE.EXPENSE,
stateNum: CONST.REPORT.STATE_NUM.OPEN,
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
ownerAccountID: currentUserAccountID,
};
report.parentReportID = parentReport.reportID;
report.parentReportActionID = parentReportAction.reportActionID;
const currentReportId = '';
const transactionID = 1;
const moneyRequestAction = {
...parentReportAction,
...LHNTestUtils.getFakeReportAction(),
actorAccountID: currentUserAccountID,
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
reportID: '1',
reportID: expenseReport.reportID,
originalMessage: {
IOUTransactionID: '1',
amount: 100,
Expand All @@ -8626,21 +8612,34 @@ describe('ReportUtils', () => {
type: CONST.IOU.REPORT_ACTION_TYPE.PAY,
paymentType: CONST.IOU.PAYMENT_TYPE.EXPENSIFY,
},
message: [
{
type: 'COMMENT',
html: 'hey',
text: 'hey',
isEdited: false,
whisperedTo: [],
isDeletedParentAction: false,
},
],
};

const transaction: Transaction = {
...createRandomTransaction(transactionID),
category: '',
tag: '',
created: testDate,
reportID: currentReportId,
reportID: expenseReport.reportID,
managedCard: true,
comment: {
liabilityType: CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT,
},
};

expect(canDeleteReportAction(moneyRequestAction, currentReportId, transaction, undefined, undefined)).toBe(false);
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport);

// Then the owner cannot delete it because the card transaction's liability type restricts deletion
expect(canDeleteReportAction(moneyRequestAction, expenseReport.reportID, transaction, undefined, undefined, currentUserAccountID)).toBe(false);
});

it('should return true for demo transaction', () => {
Expand Down Expand Up @@ -8684,7 +8683,7 @@ describe('ReportUtils', () => {
},
};

expect(canDeleteReportAction(moneyRequestAction, '1', transaction, undefined, undefined)).toBe(true);
expect(canDeleteReportAction(moneyRequestAction, '1', transaction, undefined, undefined, currentUserAccountID)).toBe(true);
});

it('should return false for unreported card expense imported with deleting disabled', async () => {
Expand Down Expand Up @@ -8730,7 +8729,7 @@ describe('ReportUtils', () => {
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${selfDMReport.reportID}`, selfDMReport);

// Then it should return false since the unreported card expense is imported with deleting disabled
expect(canDeleteReportAction(trackExpenseAction, selfDMReport.reportID, transaction, undefined, undefined)).toBe(false);
expect(canDeleteReportAction(trackExpenseAction, selfDMReport.reportID, transaction, undefined, undefined, currentUserAccountID)).toBe(false);
});

it("should return false for ADD_COMMENT report action the current user (admin of the personal policy) didn't comment", async () => {
Expand All @@ -8757,7 +8756,7 @@ describe('ReportUtils', () => {
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${adminPolicy.id}`, adminPolicy);

expect(canDeleteReportAction(reportAction, report.reportID, undefined, undefined, undefined)).toBe(false);
expect(canDeleteReportAction(reportAction, report.reportID, undefined, undefined, undefined, currentUserAccountID)).toBe(false);
});
});

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/Search/SearchUIUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10603,7 +10603,7 @@ describe('SearchUIUtils', () => {

await Onyx.merge(ONYXKEYS.SESSION, {accountID: TEST_ACCOUNT_ID});

expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults)).toBe(true);
expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults, TEST_ACCOUNT_ID)).toBe(true);
});

it('should show delete option for unreported expense which can be deleted', async () => {
Expand Down Expand Up @@ -10789,7 +10789,7 @@ describe('SearchUIUtils', () => {

await Onyx.merge(ONYXKEYS.SESSION, {accountID: TEST_ACCOUNT_ID});

expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults)).toBe(true);
expect(SearchUIUtils.shouldShowDeleteOption(selectedTransactions, currentSearchResults, TEST_ACCOUNT_ID)).toBe(true);
});
});
describe('getToFieldValueForTransaction', () => {
Expand Down
Loading