Skip to content

Commit 905ffe5

Browse files
iwizniaiwiznia
authored
Merge pull request Expensify#65690 from FitseTLT/fix-disallow-canAddTransaction-for-expense-report-non-submitter
2 parents 5a2c9e3 + fbbe11b commit 905ffe5

8 files changed

Lines changed: 50 additions & 40 deletions

File tree

‎src/components/BrokenConnectionDescription.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function BrokenConnectionDescription({transactionID, policy, report}: BrokenConn
3939
return translate('violations.brokenConnection530Error');
4040
}
4141

42-
if (isPolicyAdmin && !isCurrentUserSubmitter(report?.reportID)) {
42+
if (isPolicyAdmin && !isCurrentUserSubmitter(report)) {
4343
return (
4444
<>
4545
{`${translate('violations.adminBrokenConnectionError')}`}

‎src/libs/ReportPreviewActionUtils.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ function canSubmit(
5353
}
5454

5555
const isExpense = isExpenseReport(report);
56-
const isSubmitter = isCurrentUserSubmitter(report.reportID);
56+
const isSubmitter = isCurrentUserSubmitter(report);
5757
const isOpen = isOpenReport(report);
5858
const isManager = report.managerID === getCurrentUserAccountID();
5959
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
@@ -111,7 +111,7 @@ function canApprove(report: Report, violations: OnyxCollection<TransactionViolat
111111
}
112112

113113
const isPreventSelfApprovalEnabled = policy?.preventSelfApproval;
114-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
114+
const isReportSubmitter = isCurrentUserSubmitter(report);
115115

116116
if (isPreventSelfApprovalEnabled && isReportSubmitter) {
117117
return false;
@@ -213,7 +213,7 @@ function canReview(report: Report, violations: OnyxCollection<TransactionViolati
213213
hasViolations(report.reportID, violations) ||
214214
hasNoticeTypeViolations(report.reportID, violations, true) ||
215215
hasWarningTypeViolations(report.reportID, violations, true);
216-
const isSubmitter = isCurrentUserSubmitter(report.reportID);
216+
const isSubmitter = isCurrentUserSubmitter(report);
217217
const isOpen = isOpenExpenseReport(report);
218218
const isReimbursed = isSettled(report);
219219

‎src/libs/ReportPrimaryActionUtils.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,9 @@ function isAddExpenseAction(report: Report, reportTransactions: Transaction[], i
6565
}
6666

6767
const isExpenseReport = isExpenseReportUtils(report);
68-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
6968
const canAddTransaction = canAddTransactionUtil(report);
7069

71-
return isExpenseReport && canAddTransaction && isReportSubmitter && reportTransactions.length === 0;
70+
return isExpenseReport && canAddTransaction && reportTransactions.length === 0;
7271
}
7372

7473
function isSubmitAction(report: Report, reportTransactions: Transaction[], policy?: Policy, reportNameValuePairs?: ReportNameValuePairs, reportActions?: ReportAction[]) {
@@ -77,7 +76,7 @@ function isSubmitAction(report: Report, reportTransactions: Transaction[], polic
7776
}
7877

7978
const isExpenseReport = isExpenseReportUtils(report);
80-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
79+
const isReportSubmitter = isCurrentUserSubmitter(report);
8180
const isOpenReport = isOpenReportUtils(report);
8281
const isManualSubmitEnabled = getCorrectedAutoReportingFrequency(policy) === CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MANUAL;
8382
const transactionAreComplete = reportTransactions.every((transaction) => transaction.amount !== 0 || transaction.modifiedAmount !== 0);
@@ -132,7 +131,7 @@ function isApproveAction(report: Report, reportTransactions: Transaction[], poli
132131
}
133132

134133
const isPreventSelfApprovalEnabled = policy?.preventSelfApproval;
135-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
134+
const isReportSubmitter = isCurrentUserSubmitter(report);
136135

137136
if (isPreventSelfApprovalEnabled && isReportSubmitter) {
138137
return false;
@@ -266,7 +265,7 @@ function isReviewDuplicatesAction(report: Report, reportTransactions: Transactio
266265
}
267266

268267
const isReportApprover = isApproverUtils(policy, getCurrentUserAccountID());
269-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
268+
const isReportSubmitter = isCurrentUserSubmitter(report);
270269
const isProcessingReport = isProcessingReportUtils(report);
271270
const isReportOpen = isOpenReportUtils(report);
272271

@@ -294,7 +293,7 @@ function isMarkAsCashAction(report: Report, reportTransactions: Transaction[], v
294293
return true;
295294
}
296295

297-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
296+
const isReportSubmitter = isCurrentUserSubmitter(report);
298297
const isReportApprover = isApproverUtils(policy, getCurrentUserAccountID());
299298
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
300299

@@ -368,7 +367,7 @@ function isMarkAsCashActionForTransaction(parentReport: Report, violations: Tran
368367
return false;
369368
}
370369

371-
const isReportSubmitter = isCurrentUserSubmitter(parentReport.reportID);
370+
const isReportSubmitter = isCurrentUserSubmitter(parentReport);
372371
const isReportApprover = isApproverUtils(policy, getCurrentUserAccountID());
373372
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
374373

‎src/libs/ReportSecondaryActionUtils.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import {
5656
} from './TransactionUtils';
5757

5858
function isAddExpenseAction(report: Report, reportTransactions: Transaction[], isReportArchived = false) {
59-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
59+
const isReportSubmitter = isCurrentUserSubmitter(report);
6060

6161
if (!isReportSubmitter || reportTransactions.length === 0) {
6262
return false;
@@ -95,7 +95,7 @@ function isSplitAction(report: Report, reportTransactions: Transaction[], policy
9595
return false;
9696
}
9797

98-
const isSubmitter = isCurrentUserSubmitter(report.reportID);
98+
const isSubmitter = isCurrentUserSubmitter(report);
9999
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
100100
const isManager = (report.managerID ?? CONST.DEFAULT_NUMBER_ID) === getCurrentUserAccountID();
101101

@@ -131,7 +131,7 @@ function isSubmitAction(
131131
return false;
132132
}
133133

134-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
134+
const isReportSubmitter = isCurrentUserSubmitter(report);
135135
const isReportApprover = isApproverUtils(policy, getCurrentUserAccountID());
136136
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
137137
const isManager = report.managerID === getCurrentUserAccountID();
@@ -184,7 +184,7 @@ function isApproveAction(report: Report, reportTransactions: Transaction[], viol
184184
}
185185

186186
const isPreventSelfApprovalEnabled = policy?.preventSelfApproval;
187-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
187+
const isReportSubmitter = isCurrentUserSubmitter(report);
188188

189189
if (isPreventSelfApprovalEnabled && isReportSubmitter) {
190190
return false;
@@ -331,7 +331,7 @@ function isMarkAsExportedAction(report: Report, policy?: Policy): boolean {
331331
}
332332

333333
const isInvoiceReport = isInvoiceReportUtils(report);
334-
const isReportSender = isCurrentUserSubmitter(report.reportID);
334+
const isReportSender = isCurrentUserSubmitter(report);
335335

336336
if (isInvoiceReport && isReportSender) {
337337
return true;
@@ -399,7 +399,7 @@ function isHoldActionForTransaction(report: Report, reportTransaction: Transacti
399399
}
400400

401401
const isOpenReport = isOpenReportUtils(report);
402-
const isSubmitter = isCurrentUserSubmitter(report.reportID);
402+
const isSubmitter = isCurrentUserSubmitter(report);
403403
const isReportManager = isReportManagerUtils(report);
404404

405405
if (isOpenReport && (isSubmitter || isReportManager)) {
@@ -458,7 +458,7 @@ function isDeleteAction(report: Report, reportTransactions: Transaction[], repor
458458
return false;
459459
}
460460

461-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
461+
const isReportSubmitter = isCurrentUserSubmitter(report);
462462
const isApprovalEnabled = policy ? policy.approvalMode && policy.approvalMode !== CONST.POLICY.APPROVAL_MODE.OPTIONAL : false;
463463
const isForwarded = isProcessingReportUtils(report) && isApprovalEnabled && !isAwaitingFirstLevelApproval(report);
464464

@@ -478,7 +478,7 @@ function isRetractAction(report: Report, policy?: Policy): boolean {
478478
return false;
479479
}
480480

481-
const isReportSubmitter = isCurrentUserSubmitter(report.reportID);
481+
const isReportSubmitter = isCurrentUserSubmitter(report);
482482
if (!isReportSubmitter) {
483483
return false;
484484
}

‎src/libs/ReportUtils.ts‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,12 +1489,8 @@ function isSettled(reportOrID: OnyxInputOrEntry<Report> | SearchReport | string
14891489
/**
14901490
* Whether the current user is the submitter of the report
14911491
*/
1492-
function isCurrentUserSubmitter(reportID: string | undefined): boolean {
1493-
if (!allReports || !reportID) {
1494-
return false;
1495-
}
1496-
const report = allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
1497-
return !!(report && report.ownerAccountID === currentUserAccountID);
1492+
function isCurrentUserSubmitter(report: OnyxEntry<Report>): boolean {
1493+
return !!report && report.ownerAccountID === currentUserAccountID;
14981494
}
14991495

15001496
/**
@@ -2467,9 +2463,11 @@ function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry<Report>, isRep
24672463
* Return true if:
24682464
* - report is a non-settled IOU
24692465
* - report is a draft
2466+
* Returns false if:
2467+
* - if current user is not the submitter of an expense report
24702468
*/
24712469
function canAddTransaction(moneyRequestReport: OnyxEntry<Report>, isReportArchived = false): boolean {
2472-
if (!isMoneyRequestReport(moneyRequestReport)) {
2470+
if (!isMoneyRequestReport(moneyRequestReport) || (isExpenseReport(moneyRequestReport) && !isCurrentUserSubmitter(moneyRequestReport))) {
24732471
return false;
24742472
}
24752473
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
@@ -4088,7 +4086,7 @@ function canEditMoneyRequest(reportAction: OnyxInputOrEntry<ReportAction<typeof
40884086
return true;
40894087
}
40904088

4091-
if (policy?.type === CONST.POLICY.TYPE.CORPORATE && moneyRequestReport && isSubmitted && isCurrentUserSubmitter(moneyRequestReport.reportID)) {
4089+
if (policy?.type === CONST.POLICY.TYPE.CORPORATE && moneyRequestReport && isSubmitted && isCurrentUserSubmitter(moneyRequestReport)) {
40924090
const isForwarded = getSubmitToAccountID(policy, moneyRequestReport) !== moneyRequestReport.managerID;
40934091
return !isForwarded;
40944092
}
@@ -7908,7 +7906,7 @@ function shouldDisplayViolationsRBRInLHN(report: OnyxEntry<Report>, transactionV
79087906
}
79097907

79107908
// We only show the RBR to the submitter
7911-
if (!isCurrentUserSubmitter(report.reportID)) {
7909+
if (!isCurrentUserSubmitter(report)) {
79127910
return false;
79137911
}
79147912
if (!report.policyID || !reportsByPolicyID) {
@@ -8550,11 +8548,8 @@ function canRequestMoney(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>, o
85508548
return false;
85518549
}
85528550

8553-
// User can submit expenses in any IOU report, unless paid, but the user can only submit expenses in an expense report
8554-
// which is tied to their expense chat.
85558551
if (isMoneyRequestReport(report)) {
8556-
const canAddTransactions = canAddTransaction(report);
8557-
return isReportInGroupPolicy(report) ? isOwnPolicyExpenseChat && canAddTransactions : canAddTransactions;
8552+
return canAddTransaction(report);
85588553
}
85598554

85608555
// In the case of policy expense chat, users can only submit expenses from their own policy expense chat
@@ -9645,7 +9640,7 @@ function isAllowedToSubmitDraftExpenseReport(report: OnyxEntry<Report>): boolean
96459640
* What missing payment method does this report action indicate, if any?
96469641
*/
96479642
function getIndicatedMissingPaymentMethod(userWallet: OnyxEntry<UserWallet>, reportId: string | undefined, reportAction: ReportAction): MissingPaymentMethod | undefined {
9648-
const isSubmitterOfUnsettledReport = isCurrentUserSubmitter(reportId) && !isSettled(reportId);
9643+
const isSubmitterOfUnsettledReport = reportId && isCurrentUserSubmitter(getReport(reportId, allReports)) && !isSettled(reportId);
96499644
if (!reportId || !isSubmitterOfUnsettledReport || !isReimbursementQueuedAction(reportAction)) {
96509645
return undefined;
96519646
}

‎src/libs/TransactionUtils/index.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ function shouldShowBrokenConnectionViolationInternal(brokenConnectionViolations:
958958
return false;
959959
}
960960

961-
if (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report?.reportID)) {
961+
if (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report)) {
962962
return true;
963963
}
964964

@@ -1040,9 +1040,7 @@ function checkIfShouldShowMarkAsCashButton(hasRTERPendingViolation: boolean, sho
10401040
if (hasRTERPendingViolation) {
10411041
return true;
10421042
}
1043-
return (
1044-
shouldDisplayBrokenConnectionViolation && (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report?.reportID)) && !isReportApproved({report}) && !isReportManuallyReimbursed(report)
1045-
);
1043+
return shouldDisplayBrokenConnectionViolation && (!isPolicyAdmin(policy) || isCurrentUserSubmitter(report)) && !isReportApproved({report}) && !isReportManuallyReimbursed(report);
10461044
}
10471045

10481046
/**

‎src/pages/home/HeaderView.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ function HeaderView({report, parentReportAction, onNavigationMenuButtonClicked,
153153
const reportDescription = Parser.htmlToText(getReportDescription(report));
154154
const policyName = getPolicyName({report, returnEmptyIfNotFound: true});
155155
const policyDescription = getPolicyDescriptionText(policy);
156-
const isPersonalExpenseChat = isPolicyExpenseChat && isCurrentUserSubmitter(report?.reportID);
156+
const isPersonalExpenseChat = isPolicyExpenseChat && isCurrentUserSubmitter(report);
157157
const hasTeam2025Pricing = useHasTeam2025Pricing();
158158
const subscriptionPlan = useSubscriptionPlan();
159159

‎tests/unit/ReportUtilsTest.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1140,7 +1140,7 @@ describe('ReportUtils', () => {
11401140
...LHNTestUtils.getFakeReport(),
11411141
parentReportID: '102',
11421142
type: CONST.REPORT.TYPE.EXPENSE,
1143-
managerID: currentUserAccountID,
1143+
ownerAccountID: currentUserAccountID,
11441144
};
11451145
const moneyRequestOptions = temporary_getMoneyRequestOptions(report, undefined, [currentUserAccountID]);
11461146
expect(moneyRequestOptions.length).toBe(2);
@@ -1162,7 +1162,7 @@ describe('ReportUtils', () => {
11621162
stateNum: CONST.REPORT.STATE_NUM.OPEN,
11631163
statusNum: CONST.REPORT.STATUS_NUM.OPEN,
11641164
parentReportID: '103',
1165-
managerID: currentUserAccountID,
1165+
ownerAccountID: currentUserAccountID,
11661166
};
11671167
const paidPolicy = {
11681168
type: CONST.POLICY.TYPE.TEAM,
@@ -1293,6 +1293,7 @@ describe('ReportUtils', () => {
12931293
statusNum: CONST.REPORT.STATUS_NUM.SUBMITTED,
12941294
parentReportID: '101',
12951295
policyID: paidPolicy.id,
1296+
ownerAccountID: currentUserAccountID,
12961297
};
12971298
const moneyRequestOptions = temporary_getMoneyRequestOptions(report, paidPolicy, [currentUserAccountID, participantsAccountIDs.at(0) ?? CONST.DEFAULT_NUMBER_ID]);
12981299
expect(moneyRequestOptions.length).toBe(2);
@@ -3753,6 +3754,7 @@ describe('ReportUtils', () => {
37533754
const report: Report = {
37543755
...createRandomReport(10000),
37553756
type: CONST.REPORT.TYPE.EXPENSE,
3757+
ownerAccountID: currentUserAccountID,
37563758
};
37573759
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
37583760

@@ -3765,11 +3767,27 @@ describe('ReportUtils', () => {
37653767
expect(result).toBe(true);
37663768
});
37673769

3770+
it('should return false for an expense report the current user is not the submitter', async () => {
3771+
// Given an expense report the current user is not the submitter
3772+
const report: Report = {
3773+
...createRandomReport(10000),
3774+
type: CONST.REPORT.TYPE.EXPENSE,
3775+
ownerAccountID: currentUserAccountID + 1,
3776+
};
3777+
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
3778+
3779+
const result = canAddTransaction(report, false);
3780+
3781+
// Then the result is false
3782+
expect(result).toBe(false);
3783+
});
3784+
37683785
it('should return false for an archived report', async () => {
37693786
// Given an archived expense report
37703787
const report: Report = {
37713788
...createRandomReport(10001),
37723789
type: CONST.REPORT.TYPE.EXPENSE,
3790+
ownerAccountID: currentUserAccountID,
37733791
};
37743792
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
37753793
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.reportID}`, {private_isArchived: DateUtils.getDBTime()});

0 commit comments

Comments
 (0)