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

non-admin approvers are removed from a report when it is retracted #46913

Closed
1 of 6 tasks
m-natarajan opened this issue Aug 6, 2024 · 31 comments
Closed
1 of 6 tasks

non-admin approvers are removed from a report when it is retracted #46913

m-natarajan opened this issue Aug 6, 2024 · 31 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 Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff

Comments

@m-natarajan
Copy link

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.17-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @puneetlath
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1722958568843569

Action Performed:

  1. Create a policy in NewDot and configure it to have an approver that is not an admin and non-instant submit
  2. As a submitter create an expense report in NewDot in submit it
  3. Log into OldDot as the submitter and retract the report
  4. Log into NewDot as the approver and try to open that report from the workspace chat

Expected Result:

Should have access to the report

Actual Result:

hmm it is not here page appears

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

image (9)

Recording.421.mp4

View all open jobs on GitHub

@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 6, 2024
Copy link

melvin-bot bot commented Aug 6, 2024

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

@puneetlath puneetlath added the Internal Requires API changes or must be handled by Expensify staff label Aug 7, 2024
@adelekennedy adelekennedy added the Hot Pick Ready for an engineer to pick up and run with label Aug 8, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 8, 2024
@adelekennedy adelekennedy added Weekly KSv2 and removed Daily KSv2 labels Aug 9, 2024
@melvin-bot melvin-bot bot removed the Overdue label Aug 9, 2024
@adelekennedy
Copy link

still waiting for an internal pick up

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Aug 13, 2024
Copy link

melvin-bot bot commented Aug 13, 2024

Triggered auto assignment to @blimpich, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@aldo-expensify
Copy link
Contributor

@mollfpr C+ reviewed the PR linked to this issue.

@ishpaul777 did you link the right issue?

@ishpaul777
Copy link
Contributor

Yes that's the issue https://expensify.enterprise.slack.com/archives/C05LX9D6E07/p1723557020714749?thread_ts=1722608185.846799&channel=C05LX9D6E07&message_ts=1723557020.714749

@blimpich
Copy link
Contributor

Not sure why Melvin auto-assigned me to this. Can't pick this up right now.

@blimpich blimpich removed their assignment Aug 13, 2024
@aldo-expensify
Copy link
Contributor

@blimpich I think the Internal tag autoassigns an engineer. If you can't work on it, I think you are supposed to find someone else to work on it.

@blimpich
Copy link
Contributor

blimpich commented Aug 13, 2024

@aldo-expensify It didn't use to, is that a recent change we made? Isn't that just a re-hash of the engineering label auto-assigner?

@blimpich
Copy link
Contributor

blimpich commented Aug 13, 2024

I just removed and re-added the internal label on this issue and it didn't auto-assign.

My guess is this might be a bug related to the related PR trying to find an internal engineer to review this PR. That would make more sense since labels were all applied days ago but that melvin comment on the PR lines up exactly to the minute I was assigned to this.

I'm pretty sure we only auto-assign internal engineers for issues tagged AutoAssignerNewDotQuality at the moment.

@blimpich blimpich removed their assignment Aug 13, 2024
@aldo-expensify
Copy link
Contributor

Ahhh, maybe you are right and Internal doesn't auto assign an engineer, I thought it did 🤷

@aldo-expensify aldo-expensify removed Reviewing Has a PR in review Internal Requires API changes or must be handled by Expensify staff labels Aug 14, 2024
@aldo-expensify
Copy link
Contributor

Assigning them since I believe they need to be paid

@ishpaul777
Copy link
Contributor

This is ready for payment! 🎉

@adelekennedy adelekennedy added the Awaiting Payment Auto-added when associated PR is deployed to production label Aug 26, 2024
@puneetlath
Copy link
Contributor

I believe we also need to update the back-end to not remove the approvers from the report when it is retracted, is that right?

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 1, 2024
@ishpaul777
Copy link
Contributor

I believe we also need to update the back-end to not remove the approvers from the report when it is retracted, is that right?

i guess that should not hold payment, right ?

@melvin-bot melvin-bot bot added the Overdue label Sep 3, 2024
@adelekennedy
Copy link

agreed - I think we can move forward with payment here

@melvin-bot melvin-bot bot removed the Overdue label Sep 4, 2024
@adelekennedy
Copy link

adelekennedy commented Sep 4, 2024

Payouts due:

@ishpaul777
Copy link
Contributor

I accepted offer on upwork, I guess @mollfpr get paid through Newdot

@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
Copy link

melvin-bot bot commented Sep 9, 2024

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

@adelekennedy
Copy link

@puneetlath payment is made here but do we need to keep this open until an engineer will pick up the back-end update or is that being worked on elsewhere?

@melvin-bot melvin-bot bot removed the Overdue label Sep 10, 2024
@puneetlath
Copy link
Contributor

I'm not aware of it being elsewhere. You can create a separate issue if you think that's easier to manage.

@garrettmknight
Copy link
Contributor

$250 approved for @mollfpr

@melvin-bot melvin-bot bot added the Overdue label Sep 12, 2024
Copy link

melvin-bot bot commented Sep 13, 2024

@mollfpr, @adelekennedy, @ishpaul777 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@dylanexpensify
Copy link
Contributor

@adelekennedy created BE issue here to track in wave-control.

@ishpaul777
Copy link
Contributor

new issue is created this is good to close :

@melvin-bot melvin-bot bot removed the Overdue label Sep 16, 2024
@puneetlath
Copy link
Contributor

Thanks everyone.

@deetergp
Copy link
Contributor

Is there missing piece of the workspace setup here? I picked up https://github.com/Expensify/Expensify/issues/428467 which should ostensibly have the same reproduction steps as this one, but I don't seem to be have the option to retract the report in OldDot as the submitter.

@puneetlath
Copy link
Contributor

You should be able to retract it as long as it hasn't been approved yet. Can you show a screenshot of what you see?

@deetergp
Copy link
Contributor

deetergp commented Oct 7, 2024

Ah, the missing piece was that instant submit was on. I am now able to reproduce this and will figure out why it is happening and fix it.

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 Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff
Projects
Archived in project
Development

No branches or pull requests

10 participants