search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.52k stars 2.87k forks source link

[Instant Submit for Control] Submitters cannot edit expenses when a vacation delegate approver is assigned for first level approval #50483

Open garrettmknight opened 3 weeks ago

garrettmknight commented 3 weeks 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: N/A Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @HaleyJacobson Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727963381780969

Action Performed:

  1. Create a new workspace
  2. Invite Submitter, Approver A and Approver B
  3. Enable approvals and set Approver A as the approver
  4. As Approver A, go to OldDot and set Approver B as your vacation delegate
  5. As Submitter, create an expense in your workspace chat

Expected Result:

Since the report is awaiting the first level of approver, the submitter should be able to edit expenses on it.

Actual Result:

The submitter is unable to edit the expense even though the report is still awaiting the first level of approval. Note that this is because to allow editing at the first level of approval, we check whether the manager of the approver matches the first-level approver.

One way to fix it would be to add a check to see if the first-level approver has a vacation delegate set. If they do, instead of checking for the first-level approver when confirming whether it matches the report managerID, we should check for the first-level approver's vacation delegate.

Workaround:

No

Platforms:

All

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

melvin-bot[bot] commented 3 weeks ago

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

garrettmknight commented 3 weeks ago

cc @youssef-lr since we were discussing how to fix this.

greg-schroeder commented 2 weeks ago

Auth PR is merged, waiting for that to deploy before moving on to Web

greg-schroeder commented 2 weeks ago

Seems @youssef-lr has a couple of failing tests on the web PR, but nearly there

youssef-lr commented 2 weeks ago

PR is deployed, we can close this.

nkuoch commented 6 days ago

PR was reverted. Please see https://expensify.slack.com/archives/C03SSAQ7P/p1730164864383419

youssef-lr commented 5 days ago

New PR is up.

greg-schroeder commented 2 days ago

PR is still being worked on!