diff --git a/Gemfile.lock b/Gemfile.lock
index c0e95e3b9b..81eca25817 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -64,7 +64,7 @@ GEM
debug_inspector (>= 0.0.1)
bootsnap (1.10.2)
msgpack (~> 1.2)
- brakeman (5.2.0)
+ brakeman (5.2.1)
builder (3.2.4)
bullet (7.0.1)
activesupport (>= 3.0.0)
@@ -218,7 +218,7 @@ GEM
httparty (0.20.0)
mime-types (~> 3.0)
multi_xml (>= 0.5.2)
- i18n (1.8.11)
+ i18n (1.9.1)
concurrent-ruby (~> 1.0)
ipaddress (0.8.3)
jbuilder (2.11.5)
@@ -317,7 +317,7 @@ GEM
coderay (~> 1.1)
method_source (~> 1.0)
public_suffix (4.0.6)
- puma (5.6.0)
+ puma (5.6.1)
nio4r (~> 2.0)
pundit (2.1.1)
activesupport (>= 3.0.0)
@@ -380,12 +380,12 @@ GEM
rspec-mocks (~> 3.10.0)
rspec-collection_matchers (1.2.0)
rspec-expectations (>= 2.99.0.beta1)
- rspec-core (3.10.1)
+ rspec-core (3.10.2)
rspec-support (~> 3.10.0)
rspec-expectations (3.10.2)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.10.0)
- rspec-mocks (3.10.2)
+ rspec-mocks (3.10.3)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.10.0)
rspec-rails (5.1.0)
diff --git a/app/controllers/concerns/paginable.rb b/app/controllers/concerns/paginable.rb
index 34d73c95d6..0a81985b49 100644
--- a/app/controllers/concerns/paginable.rb
+++ b/app/controllers/concerns/paginable.rb
@@ -8,7 +8,7 @@ module Paginable
##
# Regex to validate sort_field param is safe
- SORT_COLUMN_FORMAT = /[\w_]+\.[\w_]/.freeze
+ SORT_COLUMN_FORMAT = /[\w_]+\.[\w_]+$/.freeze
PAGINATION_QUERY_PARAMS = %i[page sort_field sort_direction
search controller action].freeze
@@ -129,7 +129,7 @@ def paginable?
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
def refine_query(scope)
@args = @args.with_indifferent_access
- scope = scope.search(@args[:search]) if @args[:search].present?
+ scope = scope.search(@args[:search]).distinct if @args[:search].present?
# Can raise NoMethodError if the scope does not define a search method
if @args[:sort_field].present?
frmt = @args[:sort_field][SORT_COLUMN_FORMAT]
@@ -148,8 +148,9 @@ def refine_query(scope)
scope = scope.order(order_field.to_sym => sort_direction.to_s)
else
order_field = ActiveRecord::Base.sanitize_sql(@args[:sort_field])
+ sd = ActiveRecord::Base.sanitize_sql(sort_direction)
scope = scope.includes(table_part.singularize.to_sym)
- .order("#{order_field} #{sort_direction}")
+ .order("#{order_field} #{sd}")
end
end
if @args[:page] != 'ALL'
diff --git a/app/controllers/contributors_controller.rb b/app/controllers/contributors_controller.rb
index a92ea53356..3c210b63d4 100644
--- a/app/controllers/contributors_controller.rb
+++ b/app/controllers/contributors_controller.rb
@@ -30,7 +30,7 @@ def edit
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
# POST /plans/:plan_id/contributors
def create
- authorize @plan
+ authorize @plan, :edit?
args = translate_roles(hash: contributor_params)
args = process_org(hash: args)
diff --git a/app/controllers/org_admin/phase_versions_controller.rb b/app/controllers/org_admin/phase_versions_controller.rb
index 1d8ae32444..096acd4867 100644
--- a/app/controllers/org_admin/phase_versions_controller.rb
+++ b/app/controllers/org_admin/phase_versions_controller.rb
@@ -8,7 +8,7 @@ class PhaseVersionsController < ApplicationController
# POST /org_admin/templates/:template_id/phases/:phase_id/versions
def create
@phase = Phase.find(params[:phase_id])
- authorize @phase, :create?
+ authorize @phase
@new_phase = get_modifiable(@phase)
flash[:notice] = if @new_phase == @phase
'This template is already a draft'
diff --git a/app/controllers/org_admin/plans_controller.rb b/app/controllers/org_admin/plans_controller.rb
index da5aad0ac3..2c7ec2a396 100644
--- a/app/controllers/org_admin/plans_controller.rb
+++ b/app/controllers/org_admin/plans_controller.rb
@@ -43,7 +43,7 @@ def feedback_complete
# rubocop:enable Metrics/AbcSize
# GET /org_admin/download_plans
- # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
+ # rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def download_plans
# Test auth directly and throw Pundit error sincePundit
# is unaware of namespacing
@@ -68,9 +68,9 @@ def download_plans
csv << [
plan.title.to_s,
plan.template.title.to_s,
- (plan.owner.org.present? ? plan.owner.org.name : '').to_s,
- plan.owner.name(false).to_s,
- plan.owner.email.to_s,
+ (plan.owner&.org&.present? ? plan.owner.org.name : '').to_s,
+ plan.owner&.name(false)&.to_s,
+ plan.owner&.email&.to_s,
l(plan.latest_update.to_date, format: :csv).to_s,
Plan::VISIBILITY_MESSAGE[plan.visibility.to_sym].capitalize.to_s
]
@@ -81,6 +81,6 @@ def download_plans
format.csv { send_data plans, filename: "#{file_name}.csv" }
end
end
- # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
+ # rubocop:enable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
end
end
diff --git a/app/controllers/paginable/contributors_controller.rb b/app/controllers/paginable/contributors_controller.rb
index 091515df11..3e50ef5b65 100644
--- a/app/controllers/paginable/contributors_controller.rb
+++ b/app/controllers/paginable/contributors_controller.rb
@@ -12,7 +12,7 @@ class ContributorsController < ApplicationController
# GET /paginable/plans/:plan_id/contributors/index/:page
def index
@plan = Plan.find_by(id: params[:plan_id])
- authorize @plan
+ authorize @plan, :show?
paginable_renderise(
partial: 'index',
scope: Contributor.where(plan_id: @plan.id),
diff --git a/app/controllers/paginable/plans_controller.rb b/app/controllers/paginable/plans_controller.rb
index 4f17fb41aa..c0c5667f63 100644
--- a/app/controllers/paginable/plans_controller.rb
+++ b/app/controllers/paginable/plans_controller.rb
@@ -7,7 +7,7 @@ class PlansController < ApplicationController
# /paginable/plans/privately_visible/:page
def privately_visible
- raise Pundit::NotAuthorizedError unless Paginable::PlanPolicy.new(current_user).privately_visible?
+ authorize Plan
paginable_renderise(
partial: 'privately_visible',
@@ -19,9 +19,7 @@ def privately_visible
# GET /paginable/plans/organisationally_or_publicly_visible/:page
def organisationally_or_publicly_visible
- unless Paginable::PlanPolicy.new(current_user).organisationally_or_publicly_visible?
- raise Pundit::NotAuthorizedError
- end
+ authorize Plan
paginable_renderise(
partial: 'organisationally_or_publicly_visible',
diff --git a/app/policies/department_policy.rb b/app/policies/department_policy.rb
index 1632749478..95018eb5cf 100644
--- a/app/policies/department_policy.rb
+++ b/app/policies/department_policy.rb
@@ -5,6 +5,11 @@
class DepartmentPolicy < ApplicationPolicy
# NOTE: @user is the signed_in_user and @record is an instance of Department
+ def index?
+ (@user.can_org_admin? && @user.org.id == @department.org_id) ||
+ @user.can_super_admin?
+ end
+
def new?
@user.can_org_admin? || @user.can_super_admin?
end
diff --git a/app/policies/phase_policy.rb b/app/policies/phase_policy.rb
index 18ed607788..5d2b683162 100644
--- a/app/policies/phase_policy.rb
+++ b/app/policies/phase_policy.rb
@@ -12,30 +12,34 @@ class PhasePolicy < ApplicationPolicy
# - The template which they are modifying belongs to their org
def show?
- @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end
def preview?
- @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ end
+
+ def edit?
+ user.can_modify_templates? && (@record.template.org_id == user.org_id)
end
def update?
- @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end
def new?
- @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end
def create?
- @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end
def destroy?
- @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end
def sort?
- @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
+ @user.can_modify_templates? && (@record.template.org_id == @user.org_id)
end
end
diff --git a/app/policies/plan_policy.rb b/app/policies/plan_policy.rb
index 1ba95e6580..a2a95f92b6 100644
--- a/app/policies/plan_policy.rb
+++ b/app/policies/plan_policy.rb
@@ -5,6 +5,10 @@
class PlanPolicy < ApplicationPolicy
# NOTE: @user is the signed_in_user and @record is an instance of Plan
+ def index?
+ @user.present?
+ end
+
def show?
@record.readable_by?(@user.id)
end
@@ -70,4 +74,12 @@ def select_guidances_list?
def update_guidances_list?
@record.editable_by?(@user.id)
end
+
+ def privately_visible?
+ @user.present?
+ end
+
+ def organisationally_or_publicly_visible?
+ @user.present?
+ end
end
diff --git a/app/views/shared/export/_plan.erb b/app/views/shared/export/_plan.erb
index f5c2e45e83..423de9ae25 100644
--- a/app/views/shared/export/_plan.erb
+++ b/app/views/shared/export/_plan.erb
@@ -78,7 +78,7 @@
<%# case for displaying comments OR text %>
<% elsif !blank %>
- <%= sanitize answer.text %>
+ <%= sanitize answer&.text %>
<% end %>
<% end %>
diff --git a/app/views/shared/export/_plan_txt.erb b/app/views/shared/export/_plan_txt.erb
index fd8ce323e6..32f83e8303 100644
--- a/app/views/shared/export/_plan_txt.erb
+++ b/app/views/shared/export/_plan_txt.erb
@@ -1,7 +1,7 @@
<%= "#{@plan.title}" %>
<%= "----------------------------------------------------------\n" %>
<% if @show_coversheet %>
-<%= @hash[:attribution].many? ? _("Creators: ") : _('Creator:') %> <%= @hash[:attribution].join(', ') %>
+<%= @hash[:attribution].length > 1 ? _("Creators: ") : _('Creator:') %> <%= @hash[:attribution].join(', ') %>
<%= _("Affiliation: ") + @hash[:affiliation] %>
<% if @hash[:funder].present? %>
<%= _("Template: ") + @hash[:funder] %>
@@ -24,7 +24,7 @@
<% @hash[:phases].each do |phase| %>
<%# Only render selected phase %>
<% if phase[:title] == @selected_phase.title %>
-<%= (@hash[:phases].many? ? "#{phase[:title]}" : "") %>
+<%= (@hash[:phases].length > 1 ? "#{phase[:title]}" : "") %>
<% phase[:sections].each do |section| %>
<% if display_section?(@hash[:customization], section, @show_custom_sections) && num_section_questions(@plan, section, phase) > 0 %>
<% if @show_sections_questions %>