From f3a6c18a84df960a0e4e3823e048f6c47656481b Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Mon, 14 Oct 2019 14:50:05 +0200 Subject: [PATCH] Improve permission checking for reports [SCI-3991] --- app/controllers/concerns/report_actions.rb | 42 ++++++++++------------ app/controllers/reports_controller.rb | 18 +++++----- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/app/controllers/concerns/report_actions.rb b/app/controllers/concerns/report_actions.rb index 9b279910f..7bc1e0ca6 100644 --- a/app/controllers/concerns/report_actions.rb +++ b/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' }) - .keys - .collect(&:to_i) + module_ids = (params[:modules].select { |_, p| p == '1' }).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| - next unless selected_modules.include?(my_module.id) - + selected_modules.order(:workflow_order).each do |my_module| res << generate_new_el(false) el = generate_el( 'reports/elements/my_module_element.html.erb', @@ -75,13 +74,12 @@ module ReportActions contents.values.each do |element| if contents.has_many elements = params.select { |k| k.starts_with?("module_#{element}") } - elements = elements.select { |_,v| v == '1' }.keys - elements.map! { |el| el.gsub('module_', '')}.map!{|el| el.split('_') } + elements = elements.select { |_, v| v == '1' }.keys + elements.map! { |el| el.gsub('module_', '') }.map! { |el| el.split('_') } elements.map! { |el| [el[0].to_sym, el[1].to_i] } break unless elements.empty? else - present = in_params?("module_#{element}".to_sym) || - in_params?(element.to_sym) + present = in_params?("module_#{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', - { checklist: checklist } + 'reports/elements/step_checklist_element.html.erb', 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', - { asset: asset } + 'reports/elements/step_asset_element.html.erb', 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', - { table: table } + 'reports/elements/step_table_element.html.erb', table: table ) end end if in_params? :step_comments res << generate_new_el(false) res << generate_el( - 'reports/elements/step_comments_element.html.erb', - { step: step, order: :asc } + 'reports/elements/step_comments_element.html.erb', 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', - { result: result, order: :asc } + 'reports/elements/result_comments_element.html.erb', 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 diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index da1046f68..b958d5f98 100644 --- a/app/controllers/reports_controller.rb +++ b/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]) - exp_module_ids = experiment.my_modules.pluck(:id) - modules = (params[:modules].select { |k, p| exp_module_ids.include?(k.to_i) && p == '1' }) - .keys - .collect(&:to_i) + experiment = @project.experiments.find_by(id: params[:id]) + module_ids = (params[:modules].select { |_, p| p == '1' }).keys.collect(&:to_i) + selected_modules = experiment.my_modules.where(id: module_ids) 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