[HOLD for payment 2024-03-26] [$500] Web - Expense - App crashes when changing the role of user from auditor to employee

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.28-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4208303
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

Pre-requisite: user have must created a collect workspace, submit an expense as an employee and invited another user as an auditor.

1. As the auditor on ND, go to an expense report submitted by another employee.
2. As the admin on OD on another device, change the role of the auditor to an employee.
3. Verify the expense report page on the "auditor" side.

Expected Result:

No crashes should occur.

Actual Result:

The app crashes for the auditor side after the admin change their role from auditor to employee.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6348878_1705780997594.bandicam_2024-01-19_20-26-31-610.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0176be1dfa904141ed
• Upwork Job ID: 1749419039920336896
• Last Price Increase: 2024-01-22
• Automatic offers:
• aimane-chnaif | Contributor | 28116898
• tienifr | Contributor | 28144968

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0176be1dfa904141ed

[melvin-bot]

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[kbecciv]

We think that this bug might be related to #wave7-collect-approvers
CC @RachCHopkins

[paultsimura]

Proposal

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

When the expense is opened as Auditor, the app crashes after the user is demoted to Employee

What is the root cause of that problem?

The root cause lies here:

App/src/pages/home/ReportScreen.js

Lines 616 to 622 in 1d21e28

	selector: (parentReportActions, props) => {
	const parentReportActionID = lodashGet(props, 'report.parentReportActionID');
	if (!parentReportActionID) {
	return {};
	}
	return parentReportActions[parentReportActionID];
	},

When the user is demoted, they do not have access to the expense report anymore, so the parentReportActions here is undefined. And we are trying to access undefined[key], which causes the crash.

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

We need to early return if the parentReportActions is undefined:

selector: (parentReportActions, props) => {
    const parentReportActionID = lodashGet(props, 'report.parentReportActionID');
    if (!parentReportActions || !parentReportActionID) {
        return {};
    }
    return parentReportActions[parentReportActionID];
},

Result:

Screen.Recording.2024-01-22.at.14.42.34.mov

What alternative solutions did you explore? (Optional)

We can simply use the null-safe lodashGet with default {} value:

return lodashGet(parentReportActions, parentReportActionID, {});

[tienifr]

Proposal

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

There're 2 issues in the auditor side after the admin change their role from auditor to employee

1. If auditor currently is on policy expense chat, the app loads indefinitely
2. If auditor currently is on IOU report, the app crashes

What is the root cause of that problem?

1. In here, we just handle the logic of navigating to Concierge page if the user lost access to the money request/IOU report, we're not handling for the case user lost access to the policy expense chat

Here's explanation on what we should do in this case

2. When the user lost access to the IOU report, the parentReportActions here will be nully, so when we try to access a property of it here, it causes the crash

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

1. In here, we need to handle the case where the user lost access to the policy expense chat as well, in this case the report will also become empty. We can update that line to

((ReportUtils.isMoneyRequest(prevReport) || ReportUtils.isMoneyRequestReport(prevReport) || ReportUtils.isPolicyExpenseChat(prevReport)) && _.isEmpty(report))

Just added || ReportUtils.isPolicyExpenseChat(prevReport) to cover expense chat case.

2. In here, we should also return {} if the parentReportActions is falsy, there's no parentReportActions so it's fine that parentReportAction will also be empty

if (!parentReportActionID || !parentReportActions) {

What alternative solutions did you explore? (Optional)

There's also another potential UX issue, if the auditor was on IOU report or money request, and then auditor access is removed, the user will then navigate to Not found page, meanwhile if the auditor was on Policy expense chat, they will get redirected to Concierge. If we think this inconsistency is worth fixing (so in both cases it navigates to the Concierge page), we'll need to modify the navigation logic here a bit so it won't navigate back to parent report if parent is also unavailable/to be removed, or similar logic. But I'm not sure if this is in scope since we're already fixing 2 main issues above.

[aimane-chnaif]

This is not reproducible on main

[paultsimura]

This is not reproducible on main

Yup, was fixed by #34648

[jliexpensify]

Great, so to confirm: this can be closed?

[tienifr]

Great, so to confirm: this can be closed?

@jliexpensify @aimane-chnaif we still have the infinite loading issue on the policy expense chat below

There're 2 issues in the auditor side after the admin change their role from auditor to employee
If auditor currently is on policy expense chat, the app loads indefinitely

Screen.Recording.2024-01-23.at.9.18.32.AM.mp4

The PR only fixes the issue on the IOU report.

[aimane-chnaif]

we still have the infinite loading issue on the policy expense chat below

Agree this is bug. I think we can use this GH since it's similar repro step, or create separate one.
If continues here, I'd like to take this as C+. (seems no C+ assigned here)