[$250] Workspace - No Approve button for policy admin after expense submit

lanitochka17 commented 3 months ago

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: 9.0.12-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: N/A

Issue found when executing PR https://github.com/Expensify/App/pull/45867

Action Performed:

Preconditions: user has Control policy. Owner is an approver. Workspace is connected to QBO.

Go to workspace chat
Click '+' > Submit expense.
Click on expense preview
Click Submit

Expected Result:

There is an Approve button appears in expense report after Submit

Actual Result:

No Approve button appears in expense report

Workaround:

Unknown

Platforms:

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/b647c6ee-b4e9-47ee-a1dc-782cbfb57a0d

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~016dcbec225a5081d4
Upwork Job ID: 1816601615703698448
Last Price Increase: 2024-08-08
Automatic offers:

eh2077 | Reviewer | 103440897

Issue Owner

Current Issue Owner: @lschurr

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @lschurr (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.

lanitochka17 commented 3 months ago

@lschurr FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

melvin-bot[bot] commented 3 months ago

Job added to Upwork: https://www.upwork.com/jobs/~016dcbec225a5081d4

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eh2077 (External)

NJ-2020 commented 3 months ago

Proposal

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

No Approve button for policy admin after expense submit

What is the root cause of that problem?

Right here: https://github.com/Expensify/App/blob/4727f72aae4b30aa04153692d0b142e8bacdf703/src/components/MoneyReportHeader.tsx#L124 We'll only show the approve button if we do not have an export integration button (which also means we're not connected to any integration)

So the shouldShowSettlementButton returns false and does not show the approve button https://github.com/Expensify/App/blob/4727f72aae4b30aa04153692d0b142e8bacdf703/src/components/MoneyReportHeader.tsx#L264

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

We can delete this code !shouldShowExportIntegrationButton which will show the approve button if we also have an export integration button.

What alternative solutions did you explore? (Optional)

daledah commented 3 months ago

Proposal

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

No Approve button appears in expense report

What is the root cause of that problem?

This is the logic for the correct states of the expense report which can be exported https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/libs/ReportUtils.ts#L7269

As it shows, only reports that are approved/reimbursed or closed can be exported.

However, the code for shouldShowExportIntegrationButton in https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/components/ReportActionItem/ReportPreview.tsx#L344 and https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/components/MoneyReportHeader.tsx#L122

Are not checking that condition.

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

Update https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/components/MoneyReportHeader.tsx#L122 to

const shouldShowExportIntegrationButton = !shouldShowPayButton && !shouldShowSubmitButton && connectedIntegration && isAdmin && ReportUtils.canBeExported(moneyRequestReport);

Likewise for https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/components/ReportActionItem/ReportPreview.tsx#L344

!shouldShowPayButton && !shouldShowSubmitButton && connectedIntegration && isAdmin && ReportUtils.canBeExported(iouReport)

The !shouldShowPayButton && !shouldShowSubmitButton can potentially be removed because they should already be covered in ReportUtils.canBeExported(iouReport)

What alternative solutions did you explore? (Optional)

In both locations https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/components/ReportActionItem/ReportPreview.tsx#L344 and https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/components/MoneyReportHeader.tsx#L122 Add !shouldShowApproveButton so if approve button will show, the export button will not show. However there could be cases that export button is shown but disabled as the report still doesn't meet the condition in https://github.com/Expensify/App/blob/5e6527613d0f234fcb22507b878697a2bec356c5/src/libs/ReportUtils.ts#L7265. This could be confusing to the user so it's best to add a tooltip like The IOU report can only be exported after it's approved, reimbursed or closed to the disabled export button to clarify to the user why it's disabled.

eh2077 commented 3 months ago

Reviewing proposals. This issue is subtle and I'll need more time to understand it.

melvin-bot[bot] commented 3 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 3 months ago

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

lschurr commented 3 months ago

Just a bump here @eh2077

eh2077 commented 3 months ago

Just found that free trials of my accounts have been ended, so I can't test the workflow anymore.

@lschurr Can you help to grant free trials for my accounts? I requested here https://expensify.slack.com/archives/C02NK2DQWUX/p1722781825171489

eh2077 commented 3 months ago

Found that new account is granted with 7-day free trial, so I'll be able to review this today.

eh2077 commented 2 months ago

I think we should go with @daledah 's proposal - Because only reports that are approved/reimbursed or closed can be exported, so we should hide export button when an expense needs to be approved.

✅	❌