DMP not listed in 'Admin > Plans: Notifications' when feedback requested by non-creator user

[don-stuckey]

A bug related to the feedback-request feature was reported to the DMPonline team by an org-admin (OA) user.

Description of the bug

The following is a high-level description of the bug as experienced by the OA user:

1. The OA received an email notification informing that one of the users had requested feedback on a DMP.
2. The OA went to the DMP and provided feedback on the DMP via the comments tab.
3. The OA then went to the Admin > Plans page to see the DMP listed in the Notifications section with a view to clicking "Complete" for the DMP (given that the OA had at this point provided feedback on the DMP).
4. But the DMP was not listed in the Notifications section* of the Admin > Plans page, and so the OA could not click "Complete" for the DMP to finish the process.

*This is the section where the DMPs that users have requested feedback on are listed.

Initial findings

We carried out an initial investigation of this bug, and our findings are as follows:

(1) There were two standard users associated with the target DMP (I'm using dummy data here):

 plan_id | user_id | role_access | role_active |     user_email     |
---------------------------------------------------------------------
  123456 |   1     |          14 | t           | [email protected] |        
  123456 |   2     |          15 | f           | [email protected] |        

We can see that user 1 has a role that is active and has role_access = 14, which means that user 1 has the following roles with respect to the DMP: administrator + editor + commentator.

And we can see that user 2 has a role that is in-active and has role_access 15, which means user 2 has the following roles with respect to the DMP: creator + administrator + editor + commenter

(2) We then looked at the query that is run to retrieve the DMPs to be displayed in the Notifications section of the Admin > Plans page, which is the following:

SELECT
 "roles"."plan_id"
 FROM "roles"
 INNER JOIN "users" ON "users"."id" = "roles"."user_id"
 INNER JOIN "plans" ON "plans"."id" = "roles"."plan_id"
 WHERE
 (
  ("roles"."access" in (1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31)))
 AND (users.org_id = 105195746
  AND plans.feedback_requested is TRUE
  AND roles.active is TRUE);

We can see that the where-clause filters for specific roles and for active roles. And we can see that role access 14 is not in the list of roles used in the filter, and so user 1 doesn't meet that criteria. In addition, user 2 has an in-active role and thus doesn't meet the criteria for the role to be active.

As a result, given that the roles (and thus the users) associated with the target DMP do not meet the criteria in the where-clause, the target DMP is not returned, and hence is not displayed in the Notifications section of the Admin > Plans page for the OA to see.

(3) The controller that initiates the above query is in the index method of the file ./app/controllers/org_admin/plans_controller.rb:

    def index
      # Test auth directly and throw Pundit error sincePundit
      # is unaware of namespacing
      raise Pundit::NotAuthorizedError unless current_user.present? && current_user.can_org_admin?

      sql = 'users.org_id = ? AND plans.feedback_requested is TRUE AND roles.active is TRUE'
      feedback_ids = Role.creator.joins(:user, :plan)
                         .where(sql, current_user.org_id).pluck(:plan_id)
      @feedback_plans = Plan.where(id: feedback_ids).compact

      @super_admin = current_user.can_super_admin?
      @clicked_through = params[:click_through].present?
      @plans = if @super_admin
                 Plan.all.page(1).includes(:template, roles: { user: :org })
               else
                 current_user.org.org_admin_plans.page(1)
               end
    end

In the assignment of the feedback_ids variable, the query uses Role.creator, which filters for roles that include the creator bit flag (value 1). This means only roles with an odd-numbered access value (e.g. 1, 3, 5, ..., 31) are matched, since those are the combinations that include the creator role. As a result, if a user has any combination of roles that does not include the creator bit flag (such as access level 14), their DMP will not be included in the list shown in the Notifications section.

Hence, it appears to be by design that it is only DMPs that have an active creator role associated with them that can be displayed in the Notifications section of the Admin > Plans page.

Design implications and open questions

The above bug raises the following design questions, which should ideally be addressed within the context of the roadmap application as a whole before any fixes for the bug are developed:

1. If a user has requested feedback on a DMP but the user is not the creator (i.e. owner) of the DMP, should the org-admin still see that DMP listed in the Notifications section of the Admin > Plans page?

2. Should the "Request feedback" tab in a DMP be displayed for a user if the user does not have a creator (i.e. owner) role with respect to the DMP?

3. Should it be possible for an org-admin to make a user an owner of a DMP (i.e. set the user as the creator of the DMP) if the original owner of the DMP is no longer active in the system?