aaronskiba
commented
2 months ago # app/controllers/plan_exports_controller.rb
if privately_authorized? && export_params[:form].present?
skip_authorization
@show_coversheet = export_params[:project_details].present?
@show_sections_questions = export_params[:question_headings].present?
@show_unanswered = export_params[:unanswered_questions].present?
@show_custom_sections = export_params[:custom_sections].present?
@show_research_outputs = export_params[:research_outputs].present?
@public_plan = false
elsif publicly_authorized?
skip_authorization
@show_coversheet = true
@show_sections_questions = true
@show_unanswered = true
@show_custom_sections = true
@show_research_outputs = @plan.research_outputs&.any? || false
@public_plan = true
##################################################################
def publicly_authorized?
PublicPagePolicy.new(current_user, @plan).plan_organisationally_exportable? ||
PublicPagePolicy.new(current_user, @plan).plan_export?
end # app/policies/public_page_policy.rb
def plan_export?
@record.publicly_visible?
end
def plan_organisationally_exportable?
if @record.is_a?(Plan) && @user.is_a?(User)
return @record.publicly_visible? ||
(@record.organisationally_visible? && @record.owner.present? &&
@record.owner.org_id == @user.org_id)
endAs @nicolasfranck pointed out, in the "Organization Plans section" of the /plans page, plans where plan.owner.org_id != current_user.org_id are listed. But for the elsif publicly_authorized? check to be true, @record.owner.org_id == @user.org_id must also be true. So we have a mismatch here. Either certain plans should not be listed in the first place, or else def plan_organisationally_exportable? must be updated to enable download of these organizational plans.
I'm also wondering about the following question posed by @nicolasfranck: "Normally the creator of the plan has the same organization as the organization of the plan. But according to this line, the organization of the plan itself can also be set when the primary research organization is set. Why should one do this? The primary research organization is only important for template selection, right? That is the only reason why this may happen, not?"
Please complete the following fields as applicable:
What version of the DMPRoadmap code are you running? (e.g. v2.2.0)
4.1.0Expected behaviour:
When a plan download is listed somewhere as downloadable, it should be downloadable
Actual behaviour:
Some plans, on your "dashboard", that are only "organizational or publicly" visible, have a download link. But some of those links lead to this error:
This happened to a plan with the following characteristics (example):
org_idvalue equal tocurrent_user.org_idplan.roleshave users equal to the logged in user's organization. So that plan is only included in organizationally list because of thatorg_idin the plan record, not because it is affiliated with that organization.Steps to reproduce:
org_idequal to X, and attach users that have as organization YNotes and thoughts
trueforprivately_visible?but not forexport_params[:form].present?(See here). So that line is skipped, and then it checks if it publicly visible, which fails of course. Without thatexport_params[:form].present?it "works". As I have seen, thatexport[form]=trueis there to differentiate between requests coming from the plan download page (where you can provide settings), and those coming from outside (publicly) where you cannot/should not provide settings. May the logic for allowance and formatting should not be put on line?format(PDF). Why should that handle even try to respect the format?