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

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

Open
aaronskiba opened this issue Apr 19, 2024 · 6 comments · May be fixed by #735
Open

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

aaronskiba opened this issue Apr 19, 2024 · 6 comments · May be fixed by #735
Assignees

Comments

@aaronskiba
Copy link
Collaborator

aaronskiba commented Apr 19, 2024

Please complete the following fields as applicable:

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

  • 4.0.2+portage-4.0.3

Expected behaviour:

  • A user should be able to download any of the plans listed on the /plans page.

Actual behaviour:

  • In the %{org_title} Plans section of the /plans page, an error is encountered when attempting to download some plans.

  • (Screenshot of behaviour in development environment)

  • Screenshot from 2024-04-19 14-44-45

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
Copy link
Collaborator Author

aaronskiba commented Apr 19, 2024

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
Copy link
Collaborator Author

The following upstream issues relate to this problem:
DMPRoadmap#3345
DMPRoadmap#3346

This PR also relates to the issue:
DMPRoadmap#2965

@aaronskiba
Copy link
Collaborator Author

aaronskiba commented Apr 26, 2024

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 added a commit that referenced this issue Apr 26, 2024
This commit is intended to patch issue #732

This DMP Assistant fix was implemented in a way to minimise conflicts with the upstream, DMPRoadmap repo. This issue also exists in the upstream repo, so this commit will be readdressed after a fix is implemented there.
aaronskiba added a commit that referenced this issue Apr 26, 2024
This commit is intended to patch issue #732

This DMP Assistant fix was implemented in a way to minimise conflicts with the upstream, DMPRoadmap repo. This issue also exists in the upstream repo, so this commit will be readdressed after a fix is implemented there.
aaronskiba added a commit that referenced this issue Apr 29, 2024
This commit is intended to patch issue #732

This DMP Assistant fix was implemented in a way to minimise conflicts with the upstream, DMPRoadmap repo. This issue also exists in the upstream repo, so this commit will be readdressed after a fix is implemented there.
@aaronskiba
Copy link
Collaborator Author

aaronskiba commented Apr 29, 2024

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
Copy link
Collaborator Author

-- 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
Copy link
Collaborator Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant