Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pay elsewhere button does not show up for Admin after report approval #37867

Closed
1 of 6 tasks
m-natarajan opened this issue Mar 7, 2024 · 17 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@m-natarajan
Copy link

m-natarajan commented Mar 7, 2024

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


issue was found when executing #36814
Version Number: 1.4.48-0
Reproducible in staging?: y
Reproducible in production?: n
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: applause internal team
Slack conversation:

Action Performed:

Precondition:

  • The Collect workspace has admin and employee. Admin is also approver.
  • The Collect workspace has indirect reimbursement.
  1. [Employee] Go to workspace chat.
  2. {Employee] Create a manual request.
  3. [Admin] Go to workspace chat with employee.
  4. [Admin] Click Approve.

Expected Result:

Pay elsewhere button will show up for Admin after report approval.

Actual Result:

Pay elsewhere button does not show up for Admin after report approval.

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

Bug6404841_1709778815950.20240307_102912.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~015f3e017244f4a846
  • Upwork Job ID: 1767788547695489024
  • Last Price Increase: 2024-03-13
@m-natarajan m-natarajan added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 7, 2024
Copy link

melvin-bot bot commented Mar 7, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Mar 7, 2024
Copy link
Contributor

github-actions bot commented Mar 7, 2024

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Mar 7, 2024

Triggered auto assignment to @MariaHCD (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@m-natarajan
Copy link
Author

@MariaHCD 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.

@m-natarajan
Copy link
Author

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

@c3024
Copy link
Contributor

c3024 commented Mar 7, 2024

Now we have the logic of checking if the user is a payer based on the reimbursement choice in #36814

App/src/libs/ReportUtils.ts

Lines 1243 to 1247 in ffa731a

if (policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES) {
const isReimburser = session?.email === policy?.reimburserEmail;
return isReimburser && (isApproved || isManager);
}
if (policy?.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_MANUAL) {

Backend sometimes sends the reimbursement choice blank.

Screenshot 2024-03-07 at 11 05 34 AM

Only after changing the reimbursement choice to None and back to Indirect as in the video does the reimbursement choice get updated in such cases. Then the Pay elsewhere button appears correctly.

reimbursementManual.mp4

So, I think this should be fixed by sending correct pusher response from backend.

cc: @marcochavezf

@MariaHCD
Copy link
Contributor

MariaHCD commented Mar 7, 2024

@marcochavezf I see that we started sending reimbursementChoice here: https://github.com/Expensify/Web-Expensify/pull/40396

Looks like getPolicyOnyxUpdates is deprecated now.

So perhaps we need to set reimbursementChoice in fetchPolicies?

I don't think this has to block the app deploy since it looks like a backend issue for now.

@MariaHCD MariaHCD added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Mar 7, 2024
@marcochavezf
Copy link
Contributor

We should be sending the reimbursement values from Auth here (which is the function where we should use in favor of the deprecated getPolicyOnyxUpdates). I'm not sure if we should include it in the fetchPolicies since it gets the data from the policy summary and I think the data should be lean.

I'm wondering if we're not sending the values from Auth for this policy because it's not a primary policy? 🤔

@sakluger
Copy link
Contributor

sakluger commented Mar 8, 2024

@marcochavezf @MariaHCD - just confirming that this a wave-collect bug, right? If so, can it be worked on externally, or does it need to be internal?

@melvin-bot melvin-bot bot added the Overdue label Mar 11, 2024
@MariaHCD
Copy link
Contributor

I think it is a wave-collect bug, although I'll defer to @marcochavezf to confirm. As far as I can tell right now, this needs an internal fix and cannot be worked on externally.

@melvin-bot melvin-bot bot removed the Overdue label Mar 11, 2024
@marcochavezf
Copy link
Contributor

Yup, it should be part of wave-collect and agree it should be internal

@MariaHCD MariaHCD assigned marcochavezf and unassigned MariaHCD Mar 13, 2024
@MariaHCD MariaHCD added the Internal Requires API changes or must be handled by Expensify staff label Mar 13, 2024
Copy link

melvin-bot bot commented Mar 13, 2024

Job added to Upwork: https://www.upwork.com/jobs/~015f3e017244f4a846

Copy link

melvin-bot bot commented Mar 13, 2024

Triggered auto assignment to Contributor Plus for review of internal employee PR - @s77rt (Internal)

@marcochavezf marcochavezf added the Reviewing Has a PR in review label Mar 13, 2024
@marcochavezf
Copy link
Contributor

PR up

@marcochavezf marcochavezf added Weekly KSv2 and removed Daily KSv2 labels Mar 13, 2024
@marcochavezf
Copy link
Contributor

Hmm, a lot of automated tests are broken in the PR, it seems we'd need to come up with a different strategy.

@marcochavezf marcochavezf added Daily KSv2 and removed Weekly KSv2 labels Mar 13, 2024
@marcochavezf
Copy link
Contributor

New PR up

@marcochavezf
Copy link
Contributor

PR was deployed to production, it works now on both staging and production:

Screenshot 2024-03-18 at 2 56 32 p m

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
Archived in project
Development

No branches or pull requests

6 participants