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

[HOLD for payment 2024-07-24] Expense - "Unapprove" option exists on Manually reimbursed reports, on Collect policy with Instant Submit, Submit & Close #45009

Closed
6 tasks done
izarutskaya opened this issue Jul 8, 2024 · 23 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@izarutskaya
Copy link

izarutskaya commented Jul 8, 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!


Version Number: 9.0.5-4
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Precondition:

  • Add approval settings in Workflow is disabled (submit and close mode).
  1. Go to staging.new.expensify.com
  2. Go to workspace chat as admin.
  3. Submit an expense.
  4. Go to transaction thread.
  5. Go offline.
  6. Pay elsewhere the report.
  7. Click on the report header.
  8. Note that there is no "Unapprove" option.
  9. Go online.

Expected Result:

There should be no "Unapprove" option after returning online because there is no approver.
On Old Dot, there is no "Unapprove" option for paid expense when the mode is Submit and close.

Actual Result:

"Unapprove" option reappears after returning online when there is no approver.

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

Bug6536260_1720468348607.bandicam_2024-07-09_03-47-01-422.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @JmillsExpensify
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Jul 8, 2024
Copy link

melvin-bot bot commented Jul 8, 2024

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

Copy link

melvin-bot bot commented Jul 8, 2024

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

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

github-actions bot commented Jul 8, 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.

@izarutskaya
Copy link
Author

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

@izarutskaya
Copy link
Author

We think this issue might be related to the #collect project.

@thienlnam
Copy link
Contributor

Looks like this came from #44229 where we added the Unapprove feature

cc @rushatgabhane @Beamanator

@thienlnam
Copy link
Contributor

Don't think this needs to be a deploy blocker since this is an edge case scenario

@thienlnam thienlnam added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 DeployBlocker Indicates it should block deploying the API labels Jul 8, 2024
@Beamanator
Copy link
Contributor

Interesting, ya i think you're right - i'm happy to work on this one if you want @thienlnam - seems internal if this is a "while coming back online" since that probably means a backend change

@thienlnam thienlnam assigned Beamanator and unassigned thienlnam Jul 9, 2024
@JmillsExpensify
Copy link

Added to wave-control in the meantime.

@Beamanator Beamanator added the Internal Requires API changes or must be handled by Expensify staff label Jul 10, 2024
@Beamanator
Copy link
Contributor

Beamanator commented Jul 11, 2024

A few notes when reproducing:

  1. I don't believe we need the "Go offline then online" steps, I can reproduce in dev while doing everything online
  2. I'm pretty sure this isn't a bug, in the detailed section of our doc I wrote that we'll show the button when "The report is Approved (where state is 2 and status is 3) or was Manually Reimbursed (where state is 2 and status is 4)"
    • In this case, the report was Manually Reimbursed (state 2, status 4)
  3. I figured out that we do NOT show in NewDot b/c the policy is "on instant submit & submit & close"!

It isn't great that Unapprove does NOT show up when paying offline though, so let's try to get that fixed / added in either this or a different issue

@Beamanator
Copy link
Contributor

@Beamanator Beamanator changed the title Expense - "Unapprove" option reappears after returning online when there is no approver Expense - "Unapprove" option exists on Manually reimbursed reports, on Collect policy with Instant Submit, Submit & Close Jul 11, 2024
@garrettmknight
Copy link
Contributor

Following up from the slack conversation, we want to effectively hide Unapprove from all paid reports. Separately, paid reports should have the 'Cancel payment' option but that wasn't showing. We're fixing that elsewhere.

@Beamanator
Copy link
Contributor

PR up ^ - @rushatgabhane can you please review since you worked on the original App PR that this helps fix? :D

@Beamanator Beamanator added Daily KSv2 and removed Weekly KSv2 labels Jul 12, 2024
@rushatgabhane
Copy link
Member

yes ofcourse

@Expensify Expensify deleted a comment from Beamanator Jul 15, 2024
@Beamanator
Copy link
Contributor

PR is in staging 👍

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Jul 17, 2024
@melvin-bot melvin-bot bot changed the title Expense - "Unapprove" option exists on Manually reimbursed reports, on Collect policy with Instant Submit, Submit & Close [HOLD for payment 2024-07-24] Expense - "Unapprove" option exists on Manually reimbursed reports, on Collect policy with Instant Submit, Submit & Close Jul 17, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 17, 2024
Copy link

melvin-bot bot commented Jul 17, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jul 17, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.7-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-07-24. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Jul 17, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@rushatgabhane] The PR that introduced the bug has been identified. Link to the PR:
  • [@rushatgabhane] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@rushatgabhane] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@rushatgabhane] Determine if we should create a regression test for this bug.
  • [@rushatgabhane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 23, 2024
Copy link

melvin-bot bot commented Jul 24, 2024

Payment Summary

Upwork Job

BugZero Checklist (@JmillsExpensify)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@melvin-bot melvin-bot bot added the Overdue label Jul 25, 2024
@JmillsExpensify
Copy link

Payment summary above is correct. @rushatgabhane please fill out checklist and submit an expense in NewDot.

@rushatgabhane
Copy link
Member

  1. The PR that introduced the bug has been identified. Link to the PR: This was my misunderstanding.

  2. The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N.A.

  3. A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N.A.

  4. Determine if we should create a regression test for this bug. Yes

  5. If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again

       1. Go to a collect workspace that has approvals ON
       2. Create a few manual requests
       3. Submit the report (this may happen automatically if the workspace is on instant submit)
       4. Have the admin approve the report
       5. As admin, Click on the report header - verify you see the "Unapprove" button
       6. Mark the report as "paid elsewhere"
       7. Click back on the report header - verify you DO NOT see the "Unapprove" button
    

@mallenexpensify
Copy link
Contributor

Contributor+: @rushatgabhane due $250 via NewDot

TestRail GH

@JmillsExpensify
Copy link

$250 approved for @rushatgabhane

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
Status: Done
Development

No branches or pull requests

7 participants