An expense requires approval from the second approver again if the final approver approves and subsequently unapproves the report.

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.77-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @Julesssss
Slack conversation (hyperlinked to channel name): expense

Action Performed:

1. Create three new users, user A, B & C.
2. As user A, create a workspace.
3. Enable delayed submission, and set the frequency to Manual.
4. Add user B and C to the workspace.
5. Go to the workflows, and create a new workflow where when user B submits an expense, it requires approval from user C and user A.
6. As user B, submit an expense to the workspace.
7. As user C, approve the expense.
8. As user A, approve the expense.
9. As user A, Un-approve the expense.

Expected Result:

The expense requires approval from user A only

Actual Result:

The expense requires the approval of both user C and user A again.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Recording.855.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @Julesssss

[melvin-bot]

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[mallenexpensify]

Was able to reproduce, see user c1 in screenshot below.

@Julesssss does this need to be Internal?

[Tony-MK]

Proposal

Please re-state the problem that we are trying to solve in this issue.

An expense requires approval from the second approver again if the final approver approves and subsequently unapproves the report.

What is the root cause of that problem?

In NextStepUtils.buildNextStep function, nextApproverDisplayName is the previous approver got from NextStepUtils.getNextApproverDisplayName.

App/src/libs/NextStepUtils.ts

Line 93 in a6ea6a1

const nextApproverDisplayName = getNextApproverDisplayName(report);

We are returning the submitToAccountID in IOU.getNextApproverAccountID instead of the current user account ID.

App/src/libs/NextStepUtils.ts

Lines 69 to 73 in a6ea6a1

	function getNextApproverDisplayName(report: OnyxEntry<Report>) {
	const approverAccountID = getNextApproverAccountID(report);
	return ReportUtils.getDisplayNameForParticipant(approverAccountID) ?? ReportUtils.getPersonalDetailsForAccountID(approverAccountID).login;
	}

nextApproverEmail is undefined because the currentUserEmail is the last element in approvalChain and the current user is the last to approve the iou.

App/src/libs/actions/IOU.ts

Lines 7289 to 7292 in a6ea6a1

	const nextApproverEmail = approvalChain.length === 1 ? approvalChain.at(0) : approvalChain.at(approvalChain.indexOf(currentUserEmail) + 1);
	if (!nextApproverEmail) {
	return submitToAccountID;
	}

What changes do you think we should make in order to solve the problem?

Instead of returning submitToAccountID, we should return the account ID of the current email if nextApproverEmail is undefined

App/src/libs/actions/IOU.ts

Line 7291 in a6ea6a1

return submitToAccountID;

return currentUserPersonalDetails?.accountID ?? PersonalDetailsUtils.getAccountIDsByLogins([currentUserEmail]).at(0);;

Also, change to the backend so when the user unapproved the report, the report.managerID should not change to the other approver.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We should add a unit test IOU.getNextApproverAccountID with reports in policy with different approval procedure and assert whenever the accountID is not we expect.

[Tony-MK]

@Julesssss does this need to be Internal?

We do need a backend fix but also need a front-end fix to build the next steps optimistically.

[melvin-bot]

@mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@mallenexpensify Still overdue 6 days?! Let's take care of this!

[mallenexpensify]

@Julesssss assigned to you for 👀 once back. Please confirm if this needs to be internal and that

we also need a front-end fix to build the next steps optimistically.

[melvin-bot]

@Julesssss @mallenexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[Julesssss]

We'll be solving the original bug here before moving forward here.

[melvin-bot]

@Julesssss, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mallenexpensify]

Bumped to weekly since PR is being worked on #52297 (comment)

[Julesssss]

I bumped the PR, as it has been stale for a while.