Skip to content

Commit b2bc284

Browse files
authored
Merge pull request Expensify#70283 from fahimj/fix/69249
Fix approval chain when the admin bypasses approvers
2 parents fc41e57 + 4b6dc18 commit b2bc284

3 files changed

Lines changed: 356 additions & 18 deletions

File tree

src/libs/ReportUtils.ts

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ import {
186186
getReportActionMessageText,
187187
getReportActionText,
188188
getRetractedMessage,
189+
getSortedReportActions,
189190
getTravelUpdateMessage,
190191
getWorkspaceCurrencyUpdateMessage,
191192
getWorkspaceFrequencyUpdateMessage,
@@ -4322,6 +4323,13 @@ function getNextApproverAccountID(report: OnyxEntry<Report>, isUnapproved = fals
43224323
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
43234324
// eslint-disable-next-line deprecation/deprecation
43244325
const policy = getPolicy(report?.policyID);
4326+
4327+
// If the current user took control, then they are the final approver and we don't have a next approver
4328+
const bypassApprover = getBypassApproverIfTakenControl(report);
4329+
if (bypassApprover === currentUserAccountID) {
4330+
return undefined;
4331+
}
4332+
43254333
const approvalChain = getApprovalChain(policy, report);
43264334
const submitToAccountID = getSubmitToAccountID(policy, report);
43274335

@@ -4339,7 +4347,8 @@ function getNextApproverAccountID(report: OnyxEntry<Report>, isUnapproved = fals
43394347

43404348
const nextApproverEmail = approvalChain.length === 1 ? approvalChain.at(0) : approvalChain.at(approvalChain.indexOf(currentUserEmail ?? '') + 1);
43414349
if (!nextApproverEmail) {
4342-
return submitToAccountID;
4350+
// If there's no next approver in the chain, return undefined to indicate the current user is the final approver
4351+
return undefined;
43434352
}
43444353

43454354
return getAccountIDsByLogins([nextApproverEmail]).at(0);
@@ -11468,6 +11477,42 @@ function isWorkspaceEligibleForReportChange(submitterEmail: string | undefined,
1146811477
return isPaidGroupPolicyPolicyUtils(newPolicy) && !!newPolicy.role;
1146911478
}
1147011479

11480+
/**
11481+
* Checks if someone took control of the report and if that take control is still valid
11482+
* A take control is invalidated if there's a SUBMITTED action after it
11483+
*/
11484+
function getBypassApproverIfTakenControl(expenseReport: OnyxEntry<Report>): number | null {
11485+
if (!expenseReport?.reportID) {
11486+
return null;
11487+
}
11488+
11489+
if (!isProcessingReport(expenseReport)) {
11490+
return null;
11491+
}
11492+
11493+
const reportActions = getAllReportActions(expenseReport.reportID);
11494+
if (!reportActions) {
11495+
return null;
11496+
}
11497+
11498+
// Sort actions by created timestamp to get chronological order
11499+
const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true);
11500+
11501+
// Look through actions in reverse chronological order (newest first)
11502+
// If we find a SUBMITTED action, there's no valid take control since any take control would be older
11503+
for (const action of sortedActions) {
11504+
if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED)) {
11505+
return null;
11506+
}
11507+
11508+
if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) {
11509+
return action.actorAccountID ?? null;
11510+
}
11511+
}
11512+
11513+
return null;
11514+
}
11515+
1147111516
function getApprovalChain(policy: OnyxEntry<Policy>, expenseReport: OnyxEntry<Report>): string[] {
1147211517
const approvalChain: string[] = [];
1147311518
const fullApprovalChain: string[] = [];

src/libs/actions/IOU.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ import Log from '@libs/Log';
6868
import isReportOpenInRHP from '@libs/Navigation/helpers/isReportOpenInRHP';
6969
import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
7070
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
71-
import {buildNextStep} from '@libs/NextStepUtils';
71+
import {buildNextStep, buildNextStepNew} from '@libs/NextStepUtils';
7272
import * as NumberUtils from '@libs/NumberUtils';
7373
import {getManagerMcTestParticipant, getPersonalDetailsForAccountIDs} from '@libs/OptionsListUtils';
7474
import Parser from '@libs/Parser';
@@ -170,6 +170,7 @@ import {
170170
hasOutstandingChildRequest,
171171
hasReportBeenReopened,
172172
hasReportBeenRetracted,
173+
hasViolations as hasViolationsReportUtils,
173174
isArchivedReport,
174175
isClosedReport as isClosedReportUtil,
175176
isDraftReport,
@@ -10100,13 +10101,6 @@ function getIOUReportActionToApproveOrPay(chatReport: OnyxEntry<OnyxTypes.Report
1010010101
});
1010110102
}
1010210103

10103-
function isLastApprover(approvalChain: string[]): boolean {
10104-
if (approvalChain.length === 0) {
10105-
return true;
10106-
}
10107-
return approvalChain.at(-1) === currentUserEmail;
10108-
}
10109-
1011010104
function approveMoneyRequest(expenseReport: OnyxEntry<OnyxTypes.Report>, full?: boolean) {
1011110105
if (!expenseReport) {
1011210106
return;
@@ -10128,13 +10122,23 @@ function approveMoneyRequest(expenseReport: OnyxEntry<OnyxTypes.Report>, full?:
1012810122

1012910123
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
1013010124
// eslint-disable-next-line deprecation/deprecation
10131-
const approvalChain = getApprovalChain(getPolicy(expenseReport.policyID), expenseReport);
10132-
10133-
const predictedNextStatus = isLastApprover(approvalChain) ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED;
10134-
const predictedNextState = isLastApprover(approvalChain) ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED;
10135-
const managerID = isLastApprover(approvalChain) ? expenseReport.managerID : getNextApproverAccountID(expenseReport);
10125+
const policy = getPolicy(expenseReport.policyID);
10126+
const nextApproverAccountID = getNextApproverAccountID(expenseReport);
10127+
const predictedNextStatus = !nextApproverAccountID ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED;
10128+
const predictedNextState = !nextApproverAccountID ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED;
10129+
const managerID = !nextApproverAccountID ? expenseReport.managerID : nextApproverAccountID;
1013610130

10137-
const optimisticNextStep = buildNextStep(expenseReport, predictedNextStatus);
10131+
const hasViolations = hasViolationsReportUtils(expenseReport.reportID, allTransactionViolations);
10132+
const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas);
10133+
const optimisticNextStep = buildNextStepNew({
10134+
report: expenseReport,
10135+
policy,
10136+
currentUserAccountIDParam: userAccountID,
10137+
currentUserEmailParam: currentUserEmail,
10138+
hasViolations,
10139+
isASAPSubmitBetaEnabled,
10140+
predictedNextStatus,
10141+
});
1013810142
const chatReport = getReportOrDraftReport(expenseReport.chatReportID);
1013910143

1014010144
const optimisticReportActionsData: OnyxUpdate = {

0 commit comments

Comments
 (0)