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

Refactor process for resetting collection ACLs to PermissionTemplate settings #5037

Merged
merged 5 commits into from
Jul 17, 2021

Conversation

no-reply
Copy link
Contributor

extract and provide one implementation of AdminSet/Collection#reset_access_controls!

these methods had a lot in common. extracting them to a single implementation sets us up to add a valkyrie ready version of this behavior.

@samvera/hyrax-code-reviewers

tamsin johnson added 3 commits July 16, 2021 14:32
both collections and admin sets have (ActiveFedora) implementations of
`#reset_access_controls!`. the behavior differs very slightly, but a good
portion of the underlying private method footprint is a case
character-for-character code duplication.

extract the dupliated behavior to the common collaborator `PermissionTemplate`.
extract and provide one implementation of
`AdminSet/Collection#reset_access_controls!`

these methods had a lot in common. extracting them to a single implementation
makes space for deprecating the ActiveFedora::Base versions of them, and
provides a home for a valkyrie compatible implementation.
this behavior is now extracted to the PermissionTemplate. use that
implementation internally, and recommend others do the same.
@no-reply no-reply changed the title Reset acls Refactor process for resetting collection ACLs to PermissionTemplate settings Jul 16, 2021
@jeremyf
Copy link
Contributor

jeremyf commented Jul 17, 2021

I like inverting from (in summary) collection.reset to permission_template.reset(collection: collection). This highlights the active role that the permission template takes in "governing" the collection.

@jeremyf jeremyf merged commit 832972a into main Jul 17, 2021
@jeremyf jeremyf deleted the reset-acls branch July 17, 2021 14:15
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.

2 participants