-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Category rules - "Approver" is enabled when "Add approvals" is still disabled in Workflows #48977
Comments
Triggered auto assignment to @mallenexpensify ( |
Triggered auto assignment to @mjasikowski ( |
👋 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:
|
We think that this bug might be related to #wave-control |
Edited by proposal-police: This proposal was edited at 2023-11-04T12:30:45Z. ProposalPlease re-state the problem that we are trying to solve in this issue."Approver" is enabled when "Add approvals" is still disabled in Workflows What is the root cause of that problem?We are only checking if workflows are enabled
What changes do you think we should make in order to solve the problem?We should use
Optional: we can also change the condition here to What alternative solutions did you explore? (Optional) |
tagging @WojtekBoman as the original author of #48171 |
is this related? |
@BrtqKr seems so, albeit looking at the design doc and testing steps involved it seems that the initial requirement was to disable the Approver button and show the hint if approvals are not enabled. This is consistent with proposed manual testing steps: Seeing that @marcaaron won't be available for the next few hours to clarify, I would suggest to treat the design document as the source of truth and for @WojtekBoman to create a PR based on the above. |
I'll prepare a fix for that :) |
This functionality is behind beta, removing the blocker label. |
@mjasikowski, @mallenexpensify, @marcaaron, @WojtekBoman Eep! 4 days overdue now. Issues have feelings too... |
Agree we should stick to the Design Doc on this one. It's hard to tell whether we are blocked on anything here, but sounds like @WojtekBoman is on the case. @mjasikowski I can take this off your hands since I am leading this project - but fine if you want to stick around 😄 |
@marcaaron feel free to take over, I'm OOO frequently now due to personal issues |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
I see this has been fixed on staging so I think we can close it out. |
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.32-0
Reproducible in staging?: Y
Reproducible in production?: N/A
Issue was found when executing this PR: #48171
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team
Action Performed:
Precondition:
Expected Result:
In Step 12, Approver feature should remain locked until Add approvals feature is enabled on Workflows page.
Actual Result:
In Step 12, Approver feature is unlocked when Add approvals feature is disabled on Workflows page, which is inconsistent with the behavior with "Prevent self-approvals" feature in Rules in Step 10.
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6599647_1726041745900.20240911_155552.mp4
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: