Allow only enterprise managers to connect apps #12507
Conversation
Otherwise it doesn't work because non-managers, like super admins, are not authorised to access enterprise data via the DFC API.
mkllnk
added
the
feature toggled
| @@ -1357,6 +1357,7 @@ en: | |||
| enable: "Allow data sharing" | |||
| disable: "Stop sharing" | |||
| loading: "Loading" | |||
| need_to_be_manager: "Only managers can connect apps." | |||
There was a problem hiding this comment.
Would this be a more helpful explanation? Helps work out how to solve the problem.
| need_to_be_manager: "Only managers can connect apps." | |
| need_to_be_manager: "You cannot connect an app because you are not a manager of this enterprise." |
There was a problem hiding this comment.
That would probably be more helpful. I kept it short because it fits better in that space. It's only seen by super-admins and therefore I wouldn't put too much effort into the message.
| @@ -14,6 +14,10 @@ def select_only_item(producers) | |||
| producers.size == 1 ? producers.first.id : nil | |||
| end | |||
|
|
|||
| def can_connect_apps?(enterprise) | |||
There was a problem hiding this comment.
What do you think about naming this for the condition? It's what the message talks about. Forgive me if I'm overthinking it now..
def managed_by_user?(enterprise)
There was a problem hiding this comment.
Yes, that sounds good. It is clearer for now because you know what the method is doing. If the condition changes in the future then we may need a different name again. Anyway, I think either is good but I applied your recommendation.
filipefurtad0
added
pr-staged-au
|
Hey @mkllnk , I've verified that after staging and logging in as superadmin the connect button is disabled 🟢
It remains enabled for both enterprise owners 🟢
And managers 🟢
(the orange line is meant to indicate, that for this enterprise, button remains with the endless spinner, but this is not introduced by this PR. The button is correctly displayed for other enterprises, and works as expected). I've noticed though, that the user is sent to the login screen, when clicking the Disconnect app button. There seems to be some inconsistent behavior, on this regard. I'll need to investigate master, to be sure this is not introduced by this PR.
|
What? Why?
Connecting apps only works for enterprise managers because non-managers, like super admins, are not authorised to access enterprise data via the DFC API.
What should we test?
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates