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

don't accept signoffs unless they are required #1230

Open
bhearsum opened this issue Mar 10, 2020 · 4 comments
Open

don't accept signoffs unless they are required #1230

bhearsum opened this issue Mar 10, 2020 · 4 comments
Labels
admin admin app & api (aus4-admin.mozilla.org)

Comments

@bhearsum
Copy link
Contributor

Currently, a user can sign off on changes with any role they hold -- even if that role is not required for the change. This can be confusing, because it sometimes leads to changes having 2 required signoffs, and 2 signoffs having been done -- but one or both of them not being from a required group. At a glance, it looks like signoffs have been met when they haven't been.

We should stop accepting signoffs unless they are required for the change.

@bhearsum bhearsum added the admin admin app & api (aus4-admin.mozilla.org) label Mar 10, 2020
@srfraser
Copy link
Contributor

With the releng-relman group not matching a sign-off that needs relman, is that also what users should expect?

If I read a requirement of '1 member from releng-relman' then I would expect that signing off as 'releng-relman', 'relman' or 'releng' would work, but I don't know if that's actually valid.

I don't know if it makes sense to sign off as 'releng-relman' unless the UI makes it clear that this is how we do or group memberships

@bhearsum
Copy link
Contributor Author

If I read a requirement of '1 member from releng-relman' then I would expect that signing off as 'releng-relman', 'relman' or 'releng' would work, but I don't know if that's actually valid.

This is not how it works, but I think it's a fair assumption to make for anyone not intimately familiar with Balrog's internals. Making that assumption work is probably not an easy fix...I think the best path forward here, at least for now, is to change the UX in a way that helps users make correct assumptions.

@sarah-clements sarah-clements self-assigned this Oct 25, 2021
@sarah-clements sarah-clements removed their assignment Jan 21, 2022
@sarah-clements
Copy link
Contributor

For the next person looking at this, the fix is simple but there's a bunch of duplicate code that someone might want to clean up (rather than just adding a one line of code in x number of places).

@jcristau
Copy link
Contributor

jcristau commented Dec 6, 2023

Related: #1145

That was fixed recently in the UI for rule changes. I believe permission and release changes still have that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin admin app & api (aus4-admin.mozilla.org)
Projects
None yet
Development

No branches or pull requests

4 participants