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

Rename 'delegatable' permissions to 'delegated' #3205

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

yndajas
Copy link
Member

@yndajas yndajas commented Oct 1, 2024

Trello

This came out of work to fix 'delegatable' permissions. We've gradually settled on some new terminology, the main change being that we now refer to them as 'delegated' permissions (all permissions are delegatable). This updates the codebase, including the UI and documentation, to reflect that shift

Screenshots

Before

image

After

image image

This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs.

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

title: "What is a 'delegated' permission?"
} do %>
<p class="govuk-body">
When a permission is delegated, publishing managers are able to grant the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a helpful explainer, but I'm not sure I understand the distinction between "delegated" and "delegatable" here. Isn't "delegatable" the right word here for someone being able to delegate a thing? Using the past participle feels like a thing that's already happened, rather than a thing that can happen

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see how this could be confusing, and I think the choice of distinguishing verbs isn't the most natural, but 'delegating' and 'granting' permissions are two separate concepts. We grant permissions to individuals, while we delegate the authority to grant permissions to publishing managers. I wonder if we need to clarify that here (and maybe also in the access and permissions documentation)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tl;dr 'delegated' == 'grantable by publishing managers'

Copy link
Member

@brucebolt brucebolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all makes sense to me. As long as the name is consistent throughout, I'm not really too concerned about the exact term.

@callumknights
Copy link
Contributor

Makes sense to me as well. It take me reading your other comment explaining this to George for me to figure out exactly what's going on, so it's not super natural, but I do understand and agree with the terminology.

Copy link
Contributor

@Gweaton Gweaton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more consistent and seems to be clearer for others, particularly people who haven't worked much on Signon, so let's do it.

This renames the `SupportedPermission` field `delegatable` to
`delegated`, in line with our revised and hopefully clearer terminology:
- all permissions are 'delegatable'
- all permissions are delegatable because we have a feature that
  supports 'delegating' permissions (on the `SupportedPermission` model)
- some permissions are 'delegated', meaning organisation/super
  organisation admins can grant them to users they can manage
@yndajas yndajas force-pushed the improve-delegatable-language branch from ac06c21 to 8dcc438 Compare October 3, 2024 16:07
@yndajas yndajas merged commit 7cd74f2 into main Oct 3, 2024
15 checks passed
@yndajas yndajas deleted the improve-delegatable-language branch October 3, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants