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

Prevent '%{org_title} Plans' Section From Displaying Plans Created by Users From Other Organisations #3413

Closed
wants to merge 5 commits into from

Conversation

aaronskiba
Copy link
Contributor

@aaronskiba aaronskiba commented Apr 30, 2024

Fixes #3345
Fixes #3414

Changes proposed in this PR:

  • app/controllers/application_controller.rb

    • Add pdf handling in render_respond_to_format_with_error_message
      • render_respond_to_format_with_error_message is called both when rescuing from Pundit::NotAuthorizedError and ActiveRecord::RecordNotFound. The method works properly with .html format, but prior to this change, ActionController::UnknownFormat was thrown for .pdf format.
  • Edit scope :organisationally_or_publicly_visible

    • Within this scope, replace Org.org_admin_plans with newly created Org.owned_plans.
      • Org.org_admin_plans would return any plan where plan.org_id = Org.id. In addition, it would return any plan where a user with user.org_id = Org.id had Administrator access on the plan.
      • Org.owned_plans only returns plans where the Creator access for the plan belongs to a user with user.org_id = Org.id

`render_respond_to_format_with_error_message` is called both when rescuing from Pundit::NotAuthorizedError and ActiveRecord::RecordNotFound. The method works properly with .html format, but prior to this change, ActionController::UnknownFormat was thrown for .pdf format.
- This commit changes the Plan.organisationally_or_publicly_visible(user) scope
- Prior to this commit, we used `Org.org_admin_plans`. This would return any plan where plan.org_id = Org.id. In addition, it would return any plan where a user with user.org_id = Org.id had Administrator access on the plan.
- This commit instead uses Org.owned_plans. This only returns plans where Creator access belongs to a user with user.org_id = Org.id.
Here, instead of first assigning to the `plan_ids` variable, we are directly referencing `user.org.owned_plans`. This change should not affect the query result of `scope :organisationally_or_publicly_visible`
Copy link

</tr>
1 Error
🚫

Please include a CHANGELOG entry.

You can find it at [CHANGELOG.md](https://github.com/DMPRoadmap/roadmap/blob/main/CHANGELOG.md).

Generated by 🚫 Danger

Removing `rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized` from line 25, because the exact statement also exists on line 20 of this same file.
@aaronskiba aaronskiba marked this pull request as draft May 6, 2024 16:41
@aaronskiba
Copy link
Contributor Author

Closing this for now. More discussion is needed on how to resolve issue #3345.

@aaronskiba aaronskiba closed this May 14, 2024
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.

1 participant