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

Edit Community/Collection - Assign Roles/Groups #105

Merged
merged 3 commits into from Mar 4, 2020

Conversation

benbosman
Copy link
Member

This includes support to edit the roles or groups of communities and collections.

It includes:

  • the Community Administrator group
  • the Collection Administrator group
  • the Collection Submitters group
  • the Collection Default item READ rights group
  • the Collection Default bitstream READ rights group
  • the Collection Workflow groups

Edit Collection - Assign Roles/Groups
collections.md Outdated Show resolved Hide resolved
Edit Collection - Assign Roles/Groups
Copy link
Member

@abollini abollini left a comment

Choose a reason for hiding this comment

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

I have just one question about the workflow groups inline, overall the contract looks good and can be merged for me

collections.md Show resolved Hide resolved
communities.md Outdated Show resolved Hide resolved
* submittersGroup: the Collection Submitters group
* itemReadGroup: the Collection Default item READ rights group
* bitstreamReadGroup: the Collection Default bitstream READ rights group
* workflowGroups/<:workflow-role>: the Collection Workflow groups
Copy link
Member

Choose a reason for hiding this comment

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

should we splt the workflowGroups in two subpaths one for "collection" groups and one for general groups? in the configurable workflow indeed it is possible to define roles that are repository wide and not specific of a collection. Split them over separate subpath will make easier the management of permission as collection admins are not expected to be always allowed to edit "repository wide" roles.
In addition or in alternative, the defined roles should be exposed in the workflow definition and maybe here we can flag the "repository", "collection" and "item" roles

Copy link
Member Author

Choose a reason for hiding this comment

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

For DSpace 6, the edit collection page only displays the workflow groups with the collection scope.
This is indeed because only general admins can manage the repository-wide workflow groups

It's indeed possible to extend this to expose collections/<:uuid>/collectionWorkflowGroups/<:workflow-role> and collections/<:uuid>/repositoryWorkflowGroups/<:workflow-role> where only general admins can access the latter.
But the collections/<:uuid>/repositoryWorkflowGroups/<:workflow-role> would work quite differently since it can't be created or deleted. It would only be present to display the group.
For that reason, I would personally prefer to use /api/config/workflowdefinitions/search/findByCollection?uuid=<:collection-uuid> and create an addition to expose the roles in the workflow definition or workflow step.

Copy link
Member

Choose a reason for hiding this comment

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

The current approach (in this REST Contract) seems fine to me, along with what @benbosman notes. If we find a reason to enhance/change this Contract in the future we can do so then.

Edit Collection - Assign Roles/Groups
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 This looks good to me now, thanks @benbosman

As @abollini already gave this a +1, I'll merge this as-is. If we find the need for future enhancements, those can come in a future PR.

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.

None yet

3 participants