Notifications for reviewer role going beyond the admin set

Descriptive summary

Anyone with reviewer role on an admin set is getting the notifications for works submitted on ALL admin sets. This issue exists in both the 1-0 stable and the master branch.

Rationale

Users with the approving role should only get useful notifications.

Expected behavior

Anyone with the workflow role of reviewer on an admin set should be notified of works submitted to that admin set which they should review.

Actual behavior

Anyone with the workflow role of reviewer on any admin set are getting notifications of works submitted to all admin sets.

Steps to reproduce the behavior

I created two separate admin sets and 6 users with varying participant roles and workflow roles, and submitted works from each user who was able to submit to each admin set. Reviewers for only admin set 1 received notifications for works submitted to both admin sets.

Spreadsheet of users, roles, and what they were able to do in the app.

I like to see the SQL for https://github.com/samvera/hyrax/blob/master/app/services/hyrax/workflow/permission_query.rb#L266-L271 so this snippet was taken from one of the specs using sub_query_for_user.to_sql:

SELECT CAST("sipity_agents"."proxy_for_id" AS integer)
FROM "sipity_agents"
WHERE (
    "sipity_agents"."id" IN (
        SELECT "sipity_workflow_responsibilities"."agent_id"
        FROM "sipity_workflow_responsibilities"
        WHERE "sipity_workflow_responsibilities"."workflow_role_id" IN (
            SELECT "sipity_workflow_roles"."id"
            FROM "sipity_workflow_roles"
            WHERE "sipity_workflow_roles"."role_id" IN (1)
        )
      ) OR "sipity_agents"."id" IN (
        SELECT "sipity_entity_specific_responsibilities"."agent_id"
        FROM "sipity_entity_specific_responsibilities"
        WHERE "sipity_entity_specific_responsibilities"."workflow_role_id" IN (
            SELECT "sipity_workflow_roles"."id"
            FROM "sipity_workflow_roles"
            WHERE "sipity_workflow_roles"."role_id" IN (1)
        ) AND "sipity_entity_specific_responsibilities"."entity_id" = '1'
      )
) AND "sipity_agents"."proxy_for_type" = 'User'

It difficult to identify an AdminSet entity in this query. It seems the use of the sipity_workflow_responsibilities and sipity_workflow_roles might be global things, whereas the sipity_entity_specific_responsibilities is specific to an "entity", and these two things are combined with an OR relation. Perhaps what we need is a WHERE with an AND instead of an OR to combine the sipity_workflow_responsibilities and the sipity_entity_specific_responsibilities, but the entity must be the AdminSet that a new Work belongs to. But I'm totally guessing at this point, because I don't understand how sipity_workflow_responsibilities and and sipity_workflow_roles correspond to user management roles for an AdminSet.

@darrenleeweber You're right that this query doesn't involve AdminSets at all. The Sipity internals work independent of AdminSets, and we have yet to reconcile the two models for assigning roles (the ones defined in Sipity workflows, and the ones used for AdminSets). The Notre Dame team has been working on some documentation that will unify/clarify the roles used within Hyrax. @laritakr has been a key player there.

It's complicated further by the fact that the roles defined in the default workflows bundled with Hyrax map cleanly to the AdminSet roles, but downstream users are free to define their own roles that do not map cleanly to AdminSets.

Tagging in @jeremyf and @jcoyne, the two folks who best understand this particular module.

I’ll try to note how I understand the problem, at the moment, for the workflow permission problem:

notifications can be sent to an Agent (aka User) with a Responsibility (using a preferred notification method)
Sipity has two types or “levels” of Responsibility: EntitySpecificResponsibility and WorkflowResponsibility
- a WorkflowResponsibility is independent of any Entity in the workflow (and the state of the entity in the workflow)
- an EntitySpecificResponsibility is independent of any type of Workflow
notifications are currently sent to all the Agents who have one or both of these Responsibility types; that is, the notifications are not restricted to Agents who must have both responsibilities.

In this issue, the “expected behavior” is: Anyone with the workflow role of reviewer on an admin set should be notified of works submitted to that admin set which they should review.

So, it seems like the “expected behavior” is requesting something that is not in the Sipity design; that is, it wants a combination of a WorkflowResponsibility and an EntitySpecificResponsibility. In other words, the issue requests something like an EntitySpecificWorkflowResponsibility. Perhaps this can be defined as all the Agents who must have a Responsibility in both the WorkflowResponsibility and the EntitySpecificResponsibility when the Entity is an AdminSet and when the Workflow is "adding a work to the admin set".

Moreover, it might require a complex definition of Entity that involves a parent-child relationship between an AdminSet and all the Works that are associated with it. Maybe this involves definition of a specific workflow that is just "add work to admin set"?

Also, I’m starting to wonder whether this might require dynamic creation of a new Workflow that is specific to an AdminSet. That is, the workflow will be a record in the workflows table and it will be prefixed with some identifier that connects it to a specific Entity, i.e. the AdminSet. This results in a proliferation of workflows.

This is likely some kind of UI features for dynamic selection/creation of workflows on an AdminSet entity and somehow define how each User/Role on the AdminSet is attached to the workflow. The manager of the admin set can choose pre-existing generic workflows or create a new one based on the generic options or create a new one from scratch.

Notes on reproducing the bug noted in this issue:

start the development app
- run solr_wrapper and fcrepo_wrapper in separate windows
- bundle exec rake engine_cart:server
sign-up with an admin user (already added to .internal_test_app/config/role_map.yml)
- e.g. admin@example.com
create a new Administrative Set (called Notifications-A)
- click on Administrative Sets in the left side panel
- click the button to create a new admin-set, name it (i.e. "Notifications-A") and save it
- this redirects to the "edit" page for the new admin-set
- note that the Workflow tab has two options, a default and a one-step-mediated-deposit workflow; leave the selection on the default workflow
- note that the admin@example.com user who created the new admin-set has all of the Workflow Roles that are possible for this admin-set (this can be seen by clicking on the Workflow Roles navigation item in the left side panel, i.e. /admin/workflow_roles?locale=en)
```
Notifications-A - approving (default)
Notifications-A - depositing (default)
Notifications-A - managing (default)
Notifications-A - approving (one_step_mediated_deposit)
Notifications-A - depositing (one_step_mediated_deposit)
Notifications-A - managing (one_step_mediated_deposit)
```
create a new Administrative Set (called Notifications-B)
- follows steps above again and note the updated Workflow Roles
- the admin@example.com user now has all workflow roles for both admin-set objects
- note that the UI has an X option to remove any of these workflow roles for that user; this UI feature may provide the functionality to resolve this issue; that is, just remove the rules that are not required.
add users who can submit works to one or more admin-set objects
- sign-out for the admin@example.com user
- modify the .internal_test_app/config/role_map.yml file to add users, e.g.
```
development:
admin:
- admin@example.com
depositor:
- depositorA@example.com
- depositorB@example.com
```
- restart the hyrax app: [Cnt-C] and then bundle exec rake engine_cart:server
- login/sign-up for both of the new depositors
- note that the new users cannot submit any new Work object until they are added to an admin-set by the manager of an admin-set
add users to admin-set
- login as admin@example.com and go to the Administrative Sets page
- click on the Notifications-A admin-set and click on Edit
- use the Participants panel
- Add user: depositorA@example.com, with workflow role: Depositor, and click Add
- do the same to add depositorB@example.com to Notifications-B.
- note that the Workflow Roles page does not update these new role allocations
- this might be a bug?
- updating an admin-set to add user roles does not trigger a related update to the workflow roles
- it lists the depositor users without any roles, i.e.
```
depositora@example.com | No roles
depositorb@example.com | No roles
```
- following the admin user pattern, this page should list their roles as follows:
```
depositora@example.com | Notifications-A - depositing (default)
depositorb@example.com | Notifications-B - depositing (default)
```
submit a work to an admin-set
- logout for admin@example.com and login for depositorA@example.com
- click on Works and note there are now buttons to Create batch of works and Add new work
- click on Add new work, select Generic Work and click on Create work
- fill in fields (e.g. "Title-A" etc.), add a file, choose visibility Public, check the deposit agreement and click the Save button
- this user can only add works to the Noticiations-A admin-set, so the new work should have the Relationships content as such: In Administrative Set: Notifications-A
check for notifications
- logout for depositorA@example.com and login for admin@example.com
- there is nothing in Your Activity > Notifications (i.e. /notifications?locale=en)
- there is nothing in Review Submissions: Under Review
- the new work appears in Review Submissions: Published
- it appears there was no review or approval required to publish this work and no notifications about the deposit for the admin user (bug?)
try to create an admin-set and a manager who receives notifications when a work is added to the admin-set
- modify the .internal_test_app/config/role_map.yml file to add a manager, e.g.
```
development:
[etc. as above, plus:]
manager:
- managerB@example.com
```
- sign out for admin user and sign-up for managerB@example.com
- sign out for manager and sign-in for admin user
- create a new admin-set (Notifications-C) and select the one_step_mediated_deposit workflow this time, then add depositorB as a depositor and add managerB as a manager and save the admin-set; then the Workflow Roles looks like this:
- add a new work to Notifications-C
- login as depositorB user and go to Works to add a new generic work
- Use the Relationships tab on the new work to ensure it is assigned to Notifications-C (otherwise follow the instructions above to create a new work)
- When this work is submitted, the app responds with Workflow Roles - Pending review and the notification icon for the depositorB user has a red box highlighting the number 1 in it. The notification page contains a message: "{date-time} | Deposit needs review | Workflow Roles ({object-id}) was deposited by {user} and is awaiting approval"
- review the submitted work
- login as managerB and notice the notification icon highlights 1 new notification (which reads as above)
- go to Tasks > Review Submissions in the left side panel and notice there is a new review item
- login as admin user and also note the same notification and the presence of the same entry in the Review Submissions page
- login as managerB, go to review submissions, click on the Work and (at present, to be modified by better UI design/layout) there is a review panel at the very bottom of the page (should be moved above the work panel); open the Review and Approval panel and select approve and click submit, then the status should change from pending review to deposited and there is a new notification message.
- login as admin and notice the same notification message
try to create a new admin-set without a new manager and check that managerB does not recieve any notifications when a work is added to the new admin-set, but the admin user should get notifications
- follow steps above for details, call the new admin-set Notifications-D say and ensure it is using the one_step_mediated_deposit workflow
- login as depositorB and deposit a new work into Notifications-D and note the new notification that it is pending review
- login as depositorA and notice there are no notifications for this user
- login as managerB and notice there is a new notification about the submission on Notifications-D although managerB has no workflow roles on this admin-set and cannot see this admin-set in the Administrative Sets UI listing; also, managerB has a new item in the Review Submissions page, but when this user views the new submission, there are no details and there is no Review and Approve panel available; the item appears with an alert saying "The work is not currently available because it has not yet completed the approval process". THIS CONFIRMS THE BEHAVIOR NOTED IN THIS ISSUE
- login as admin and check the Workflow Roles page and note that managerB has no roles on the new Notifications-D admin-set; also note that the admin user does get notification about review of this new submission and is able to review and approve it; so approve it and note the status changes to Deposited and a new notification is issued about this approval (and, yes, managerB should not, but does get this notification too)

samvera / hyrax