portagenetwork / roadmap

Developed by the the Alliance in collaboration with University of Alberta, DMP Assistant a data management planning tool, forking the DMP Roadmap codebase
MIT License
6 stars 1 forks source link

Unable to Download Some Organisation Plans on `/plans` Path #732

Open aaronskiba opened 2 months ago

aaronskiba commented 2 months ago

Please complete the following fields as applicable:

What version of the DMPRoadmap code are you running? (e.g. v2.2.0)

Expected behaviour:

Actual behaviour:

Steps to reproduce:

  1. Login to the app (I am using the superadmin account)
  2. Scroll down to Portage Network Plans
  3. On each of the listed plans, click the pdf icon to download/open the plan on a new page.
aaronskiba commented 2 months ago

app/policies/public_page_policy.rb

  def plan_organisationally_exportable?
    if @record.is_a?(Plan) && @user.is_a?(User)
      byebug
      return @record.publicly_visible? ||
             (@record.organisationally_visible? && @record.owner.present? &&
              @record.owner.org_id == @user.org_id)
    end

    false
  end
[27, 36] in /home/aaron/Documents/GitHub/roadmap/app/policies/public_page_policy.rb
   27:   end
   28: 
   29:   def plan_organisationally_exportable?
   30:     if @record.is_a?(Plan) && @user.is_a?(User)
   31:       byebug
=> 32:       return @record.publicly_visible? ||
   33:              (@record.organisationally_visible? && @record.owner.present? &&
   34:               @record.owner.org_id == @user.org_id)
   35:     end
   36: 
(byebug) @record.publicly_visible?
false
(byebug) @record.organisationally_visible?
true
(byebug) @record.owner.present?
  User Load (1.8ms)  SELECT "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 17943], ["LIMIT", 1]]
  ↳ app/models/plan.rb:449:in `owner'
true
(byebug) @record.owner.org_id == @user.org_id
false
(byebug) @record.owner.org_id
69
(byebug) @user.org_id
8
(byebug) @record.org_id
8
(byebug) 

Because @record.owner.org_id != @user.org_id, plan_organisationally_exportable? returns false. Is this correct, or should the check be whether @record.org_id != @user.org_id? And if not, then why is the non-exportable plan listed at all?

aaronskiba commented 2 months ago

The following upstream issues relate to this problem: https://github.com/DMPRoadmap/roadmap/issues/3345 https://github.com/DMPRoadmap/roadmap/issues/3346

This PR also relates to the issue: https://github.com/DMPRoadmap/roadmap/pull/2965

aaronskiba commented 2 months ago

Because @record.owner.org_id != @user.org_id, plan_organisationally_exportable? returns false. Is this correct, or should the check be whether @record.org_id == @user.org_id? And if not, then why is the non-exportable plan listed at all?

The problem seems to be that some of these plans shouldn't be listed in the %{org_title} Plans section in the first place. The following text is provided in that section:

'"The table below lists the plans that users at your organization have created and shared within your organization. This allows you to download a PDF and view their plans as samples or to discover new research data."

Plan.organisationally_or_publicly_visible(user) is used to populate plans in the %{org_title} Plans section. This scope needs to fixed so that it will filter out plans created/owned by users outside of the logged-in user's organisation.

aaronskiba commented 2 months ago
WITH all_results AS (

-- 1) Plans where no users with users.org_id = plans.org_id have owner access of the plan
SELECT orgs.id AS affected_org_id, orgs.name AS affected_org_name, plans.id AS plan_id, plans.title AS plan_title
FROM orgs
JOIN plans ON orgs.id = plans.org_id

WHERE plans.visibility = 0
AND plans.complete = true
AND orgs.is_other = false
AND orgs.managed = true

AND NOT EXISTS (
    SELECT 1
    FROM roles
    JOIN users ON roles.user_id = users.id
    WHERE users.org_id = plans.org_id
    AND roles.plan_id = plans.id
    AND users.active = true
    AND roles.active = true
    AND roles.access = 15
)

UNION ALL

-- 2) Plans where users within an org have administrative access, but no users within that same org have owner access
SELECT orgs.id AS affected_org_id, orgs.name AS affected_org_name, plans.id AS plan_id, plans.title AS plan_title
FROM orgs
JOIN users ON orgs.id = users.org_id
JOIN roles ON users.id = roles.user_id
JOIN plans ON roles.plan_id = plans.id

WHERE plans.visibility = 0
AND plans.complete = true
AND orgs.is_other = false
AND orgs.managed = true
AND roles.active = true
AND users.active = true
AND roles.access IN (2,3,6,7,10,11,14,15,18,19,22,23,26,27,30,31)

AND NOT EXISTS (
    SELECT 1
    FROM roles AS r
    JOIN users AS u ON r.user_id = u.id

    WHERE r.plan_id = plans.id
    AND u.org_id = users.org_id
    AND r.active = true
    AND u.active = true
    AND r.access = 15
)
)
SELECT DISTINCT affected_org_id, affected_org_name, plan_id, plan_title
FROM all_results
ORDER BY affected_org_id, plan_id;

plans_with_affected_orgs.csv

aaronskiba commented 2 months ago
-- Public plans where no users with users.org_id = plans.org_id are owners of the plan
SELECT orgs.id AS affected_org_id, orgs.name AS affected_org_name, plans.id AS plan_id, plans.title AS plan_title
FROM orgs
JOIN plans ON orgs.id = plans.org_id
WHERE plans.visibility = 1
AND orgs.is_other = false
AND orgs.managed = true

AND complete = true
AND plans.org_id NOT IN (
    SELECT users.org_id
    FROM users
    JOIN roles ON users.id = roles.user_id
    AND roles.plan_id = plans.id
    AND roles.active = true
    AND users.active = true
    AND roles.access = 15
);

plans_with_affected_orgs.csv

aaronskiba commented 2 months ago

Removed high priority label because it does not appear that many existing plans are affected.