Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
95d4111
Implement bypass approvers logic in getApprovalChain function and add…
fahimj Sep 10, 2025
4069d85
Merge remote-tracking branch 'origin/main' into fix/69249
fahimj Sep 10, 2025
da07184
update getApprovalChain function to handle bypass scenarios for managers
fahimj Sep 11, 2025
ea3d8a3
Revert "update getApprovalChain function to handle bypass scenarios f…
fahimj Sep 12, 2025
3999783
Revert "Implement bypass approvers logic in getApprovalChain function…
fahimj Sep 12, 2025
84221b3
Add unit tests for take control functionality in getApprovalChain
fahimj Sep 12, 2025
c2a6eac
Add getBypassApproverIfTakeControl function to validate take control …
fahimj Sep 12, 2025
59741f3
Merge branch 'main' into fix/69249
fahimj Sep 27, 2025
c4cb8d5
Merge branch 'main' into fix/69249
fahimj Sep 27, 2025
51f90e4
Added logic to handle bypass approver in getNextApproverAccountID and…
fahimj Sep 27, 2025
d9285e0
Refactor getBypassApproverIfTakeControl to getBypassApproverIfTakenCo…
fahimj Sep 30, 2025
2a09607
Updated variable naming to isCurrentUserNextApprover
fahimj Sep 30, 2025
9d2a8a5
prettier
fahimj Sep 30, 2025
a4a34cc
Merge branch 'main' into fix/69249
fahimj Sep 30, 2025
9d1f5b6
Merge branch 'main' into fix/69249
fahimj Oct 1, 2025
478fb9d
Refactor approval logic in ReportUtils to return null for final appro…
fahimj Oct 1, 2025
af3d1b8
Refactor getBypassApproverIfTakenControl to return account ID instead…
fahimj Oct 1, 2025
aea04a4
prettier
fahimj Oct 1, 2025
be361f4
Refactor getNextApproverAccountID to return undefined instead of null…
fahimj Oct 1, 2025
7db3f4a
Remove unused variable lastTakeControlAction from getBypassApproverIf…
fahimj Oct 1, 2025
df79e2c
prettier
fahimj Oct 1, 2025
6da3f0a
Refactor getBypassApproverIfTakenControl to directly return the actor…
fahimj Oct 6, 2025
4fe8ede
Merge branch 'main' into fix/69249
fahimj Oct 6, 2025
41a6a08
Refactor approveMoneyRequest to use an object for parameters in build…
fahimj Oct 6, 2025
3e84086
Merge branch 'main' into fix/69249
fahimj Oct 14, 2025
18d01e5
use !nextApproverAccountID directly
fahimj Oct 15, 2025
b150943
update comments
fahimj Oct 15, 2025
60e21a7
Refactor role assignments in IOUTest to use constants for better main…
fahimj Oct 15, 2025
a13a819
remove redundant take control SUBMITTED_AND_CLOSED test case
fahimj Oct 15, 2025
4b6dc18
Remove unused personal details entry from ReportUtilsTest
fahimj Oct 15, 2025
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
47 changes: 46 additions & 1 deletion src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@
getReportActionMessageText,
getReportActionText,
getRetractedMessage,
getSortedReportActions,
getTravelUpdateMessage,
getWorkspaceCurrencyUpdateMessage,
getWorkspaceFrequencyUpdateMessage,
Expand Down Expand Up @@ -931,7 +932,7 @@
let conciergeReportID: OnyxEntry<string>;
Onyx.connect({
key: ONYXKEYS.CONCIERGE_REPORT_ID,
callback: (value) => {

Check warning on line 935 in src/libs/ReportUtils.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
conciergeReportID = value;
},
});
Expand All @@ -939,7 +940,7 @@
const defaultAvatarBuildingIconTestID = 'SvgDefaultAvatarBuilding Icon';
Onyx.connect({
key: ONYXKEYS.SESSION,
callback: (value) => {

Check warning on line 943 in src/libs/ReportUtils.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
// When signed out, val is undefined
if (!value) {
return;
Expand All @@ -957,7 +958,7 @@
let currentUserPersonalDetails: OnyxEntry<PersonalDetails>;
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {

Check warning on line 961 in src/libs/ReportUtils.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
if (currentUserAccountID) {
currentUserPersonalDetails = value?.[currentUserAccountID] ?? undefined;
}
Expand All @@ -969,14 +970,14 @@
let allReportsDraft: OnyxCollection<Report>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_DRAFT,
waitForCollectionCallback: true,

Check warning on line 973 in src/libs/ReportUtils.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
callback: (value) => (allReportsDraft = value),
});

let allPolicies: OnyxCollection<Policy>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY,
waitForCollectionCallback: true,

Check warning on line 980 in src/libs/ReportUtils.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
callback: (value) => (allPolicies = value),
});

Expand All @@ -984,7 +985,7 @@
let reportsByPolicyID: ReportByPolicyMap;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,

Check warning on line 988 in src/libs/ReportUtils.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
callback: (value) => {
allReports = value;
UnreadIndicatorUpdaterHelper().then((module) => {
Expand Down Expand Up @@ -1027,14 +1028,14 @@
let allBetas: OnyxEntry<Beta[]>;
Onyx.connect({
key: ONYXKEYS.BETAS,
callback: (value) => (allBetas = value),

Check warning on line 1031 in src/libs/ReportUtils.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
});

let allTransactions: OnyxCollection<Transaction> = {};
let reportsTransactions: Record<string, Transaction[]> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION,
waitForCollectionCallback: true,

Check warning on line 1038 in src/libs/ReportUtils.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
callback: (value) => {
if (!value) {
return;
Expand All @@ -1060,7 +1061,7 @@
let allReportActions: OnyxCollection<ReportActions>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,

Check warning on line 1064 in src/libs/ReportUtils.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
callback: (actions) => {
if (!actions) {
return;
Expand All @@ -1073,7 +1074,7 @@
const allReportMetadataKeyValue: Record<string, ReportMetadata> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_METADATA,
waitForCollectionCallback: true,

Check warning on line 1077 in src/libs/ReportUtils.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
callback: (value) => {
if (!value) {
return;
Expand Down Expand Up @@ -4295,6 +4296,13 @@
// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line deprecation/deprecation
const policy = getPolicy(report?.policyID);

// If the current user took control, then they are the final approver and we don't have a next approver
const bypassApprover = getBypassApproverIfTakenControl(report);
if (bypassApprover === currentUserAccountID) {
return undefined;
}

const approvalChain = getApprovalChain(policy, report);
const submitToAccountID = getSubmitToAccountID(policy, report);

Expand All @@ -4312,7 +4320,8 @@

const nextApproverEmail = approvalChain.length === 1 ? approvalChain.at(0) : approvalChain.at(approvalChain.indexOf(currentUserEmail ?? '') + 1);
if (!nextApproverEmail) {
return submitToAccountID;
Comment thread
amyevans marked this conversation as resolved.
// If there's no next approver in the chain, return undefined to indicate the current user is the final approver
return undefined;
}

return getAccountIDsByLogins([nextApproverEmail]).at(0);
Expand Down Expand Up @@ -11430,6 +11439,42 @@
return isPaidGroupPolicyPolicyUtils(newPolicy) && !!newPolicy.role;
}

/**
* Checks if someone took control of the report and if that take control is still valid
* A take control is invalidated if there's a SUBMITTED action after it
*/
function getBypassApproverIfTakenControl(expenseReport: OnyxEntry<Report>): number | null {
if (!expenseReport?.reportID) {
return null;
}

if (!isProcessingReport(expenseReport)) {
return null;
}

const reportActions = getAllReportActions(expenseReport.reportID);
if (!reportActions) {
return null;
}

// Sort actions by created timestamp to get chronological order
const sortedActions = getSortedReportActions(Object.values(reportActions ?? {}), true);

// Look through actions in reverse chronological order (newest first)
// If we find a SUBMITTED action, there's no valid take control since any take control would be older
for (const action of sortedActions) {
if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.SUBMITTED)) {
Comment thread
fahimj marked this conversation as resolved.
return null;
}

if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.TAKE_CONTROL)) {
return action.actorAccountID ?? null;
}
}
Comment on lines +11465 to +11473

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't have to check the next submitted action after TAKE_CONTROL as you are checking actions in reverse order. So If you find an SUBMITTED action then there is no valid take control.

Also, we don't have to check for SUBMITTED_AND_CLOSED as normally this report should be in processing state where this method is called. if we want to safe gaurd, we can instead check for processing state of the report instead of this and return early.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please review my update in line with your feedback


return null;
}

function getApprovalChain(policy: OnyxEntry<Policy>, expenseReport: OnyxEntry<Report>): string[] {
const approvalChain: string[] = [];
const fullApprovalChain: string[] = [];
Expand Down
32 changes: 18 additions & 14 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import Log from '@libs/Log';
import isReportOpenInRHP from '@libs/Navigation/helpers/isReportOpenInRHP';
import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
import Navigation, {navigationRef} from '@libs/Navigation/Navigation';
import {buildNextStep} from '@libs/NextStepUtils';
import {buildNextStep, buildNextStepNew} from '@libs/NextStepUtils';
import * as NumberUtils from '@libs/NumberUtils';
import {getManagerMcTestParticipant, getPersonalDetailsForAccountIDs} from '@libs/OptionsListUtils';
import Parser from '@libs/Parser';
Expand Down Expand Up @@ -170,6 +170,7 @@ import {
hasOutstandingChildRequest,
hasReportBeenReopened,
hasReportBeenRetracted,
hasViolations as hasViolationsReportUtils,
isArchivedReport,
isClosedReport as isClosedReportUtil,
isDraftReport,
Expand Down Expand Up @@ -10118,13 +10119,6 @@ function getIOUReportActionToApproveOrPay(chatReport: OnyxEntry<OnyxTypes.Report
});
}

function isLastApprover(approvalChain: string[]): boolean {
if (approvalChain.length === 0) {
return true;
}
return approvalChain.at(-1) === currentUserEmail;
}

function approveMoneyRequest(expenseReport: OnyxEntry<OnyxTypes.Report>, full?: boolean) {
if (!expenseReport) {
return;
Expand All @@ -10146,13 +10140,23 @@ function approveMoneyRequest(expenseReport: OnyxEntry<OnyxTypes.Report>, full?:

// This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850
// eslint-disable-next-line deprecation/deprecation
const approvalChain = getApprovalChain(getPolicy(expenseReport.policyID), expenseReport);

const predictedNextStatus = isLastApprover(approvalChain) ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED;
const predictedNextState = isLastApprover(approvalChain) ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED;
const managerID = isLastApprover(approvalChain) ? expenseReport.managerID : getNextApproverAccountID(expenseReport);
const policy = getPolicy(expenseReport.policyID);
Comment thread
fahimj marked this conversation as resolved.
const nextApproverAccountID = getNextApproverAccountID(expenseReport);
const predictedNextStatus = !nextApproverAccountID ? CONST.REPORT.STATUS_NUM.APPROVED : CONST.REPORT.STATUS_NUM.SUBMITTED;
const predictedNextState = !nextApproverAccountID ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED;
const managerID = !nextApproverAccountID ? expenseReport.managerID : nextApproverAccountID;

const optimisticNextStep = buildNextStep(expenseReport, predictedNextStatus);
const hasViolations = hasViolationsReportUtils(expenseReport.reportID, allTransactionViolations);
const isASAPSubmitBetaEnabled = Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas);
const optimisticNextStep = buildNextStepNew({
report: expenseReport,
policy,
currentUserAccountIDParam: userAccountID,
currentUserEmailParam: currentUserEmail,
hasViolations,
isASAPSubmitBetaEnabled,
predictedNextStatus,
});
const chatReport = getReportOrDraftReport(expenseReport.chatReportID);

const optimisticReportActionsData: OnyxUpdate = {
Expand Down
Loading
Loading