Improve permission checking for reports [SCI-3991]

app/controllers/concerns/report_actions.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module ReportActions
   extend ActiveSupport::Concern
 
@@ -30,19 +32,18 @@ module ReportActions
   def generate_project_contents_json
     res = []
     if params.include? :modules
-      modules = (params[:modules].select { |_, p| p == '1' })
+      module_ids = (params[:modules].select { |_, p| p == '1' }).keys.collect(&:to_i)
-                .keys
-                .collect(&:to_i)
 
       # Get unique experiments from given modules
-      experiments = MyModule.where(id: modules).map(&:experiment).uniq
+      experiments = @project.experiments.distinct.joins(:my_modules).where('my_modules.id': module_ids)
       experiments.each do |experiment|
         res << generate_new_el(false)
         el = generate_el(
           'reports/elements/experiment_element.html.erb',
           experiment: experiment
         )
-        el[:children] = generate_experiment_contents_json(experiment, modules)
+        selected_modules = experiment.my_modules.where(id: module_ids)
+        el[:children] = generate_experiment_contents_json(selected_modules)
         res << el
       end
     end
@@ -50,11 +51,9 @@ module ReportActions
     res
   end
 
-  def generate_experiment_contents_json(experiment, selected_modules)
+  def generate_experiment_contents_json(selected_modules)
     res = []
-    experiment.my_modules.order(:workflow_order).each do |my_module|
+    selected_modules.order(:workflow_order).each do |my_module|
-      next unless selected_modules.include?(my_module.id)
-
       res << generate_new_el(false)
       el = generate_el(
         'reports/elements/my_module_element.html.erb',
@@ -80,8 +79,7 @@ module ReportActions
           elements.map! { |el| [el[0].to_sym, el[1].to_i] }
           break unless elements.empty?
         else
-          present = in_params?("module_#{element}".to_sym) ||
+          present = in_params?("module_#{element}".to_sym) || in_params?(element.to_sym)
-                    in_params?(element.to_sym)
           if present
             elements << [element.to_sym, nil]
             break
@@ -131,8 +129,7 @@ module ReportActions
       step.checklists.asc.each do |checklist|
         res << generate_new_el(false)
         res << generate_el(
-          'reports/elements/step_checklist_element.html.erb',
+          'reports/elements/step_checklist_element.html.erb', checklist: checklist
-          { checklist: checklist }
         )
       end
     end
@@ -140,8 +137,7 @@ module ReportActions
       step.assets.each do |asset|
         res << generate_new_el(false)
         res << generate_el(
-          'reports/elements/step_asset_element.html.erb',
+          'reports/elements/step_asset_element.html.erb', asset: asset
-          { asset: asset }
         )
       end
     end
@@ -149,16 +145,14 @@ module ReportActions
       step.tables.each do |table|
         res << generate_new_el(false)
         res << generate_el(
-          'reports/elements/step_table_element.html.erb',
+          'reports/elements/step_table_element.html.erb', table: table
-          { table: table }
         )
       end
     end
     if in_params? :step_comments
       res << generate_new_el(false)
       res << generate_el(
-        'reports/elements/step_comments_element.html.erb',
+        'reports/elements/step_comments_element.html.erb', step: step, order: :asc
-        { step: step, order: :asc }
       )
     end
     res << generate_new_el(false)
@@ -170,8 +164,7 @@ module ReportActions
     if in_params? :result_comments
       res << generate_new_el(true)
       res << generate_el(
-        'reports/elements/result_comments_element.html.erb',
+        'reports/elements/result_comments_element.html.erb', result: result, order: :asc
-        { result: result, order: :asc }
       )
     else
       res << generate_new_el(false)
@@ -180,11 +173,12 @@ module ReportActions
   end
 
   def elements_empty?(elements)
-    return true if elements.blank? || elements.count == 0
+    return true if elements.blank? || elements.count.zero?
 
     if elements.count == 1
       el = elements[0]
       return true if el.include?(:new_element) && el[:new_element]
+
       return false
     end
     false

app/controllers/reports_controller.rb

@@ -325,22 +325,20 @@ class ReportsController < ApplicationController
       if elements_empty? elements
         format.json { render json: {}, status: :no_content }
       else
-        format.json {
+        format.json do
           render json: {
             status: :ok,
             elements: elements
           }
-        }
+        end
       end
     end
   end
 
   def experiment_contents
-    experiment = @project.experiments.find_by_id(params[:id])
+    experiment = @project.experiments.find_by(id: params[:id])
-    exp_module_ids = experiment.my_modules.pluck(:id)
+    module_ids = (params[:modules].select { |_, p| p == '1' }).keys.collect(&:to_i)
-    modules = (params[:modules].select { |k, p| exp_module_ids.include?(k.to_i) && p == '1' })
+    selected_modules = experiment.my_modules.where(id: module_ids)
-              .keys
-              .collect(&:to_i)
 
     respond_to do |format|
       if experiment.blank?
@@ -348,7 +346,7 @@ class ReportsController < ApplicationController
       elsif modules.blank?
         format.json { render json: {}, status: :no_content }
       else
-        elements = generate_experiment_contents_json(experiment, modules)
+        elements = generate_experiment_contents_json(selected_modules)
       end
 
       if elements_empty? elements
@@ -451,12 +449,12 @@ class ReportsController < ApplicationController
   AvailableRepository = Struct.new(:id, :name)
 
   def load_vars
-    @report = Report.find_by_id(params[:id])
+    @report = current_team.reports.find_by(id: params[:id])
     render_404 unless @report
   end
 
   def load_vars_nested
-    @project = Project.find_by_id(params[:project_id])
+    @project = current_team.projects.find_by(id: params[:project_id])
     render_404 unless @project
     render_403 unless can_read_project?(@project)
   end