From f8e6e98d3ab53f635822ddebaaf6b47847a4d0f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 8 Nov 2019 22:11:25 +0100 Subject: [PATCH] Define stats and result permissions with scopes When defining abilities, scopes cover more cases because they can be used to check permissions for a record and to filter a collection. Ruby blocks can only be used to check permissions for a record. Note the `Budget::Phase.kind_or_later` name sounds funny, probably because we use the word "phase" for both an an attribute in the budgets table and an object associated with the budget, and so naming methods for a budget phase is a bit tricky. --- app/models/abilities/everyone.rb | 12 ++++------ app/models/budget.rb | 1 + app/models/budget/phase.rb | 6 ++++- app/models/concerns/reportable.rb | 4 ++++ spec/models/budget_spec.rb | 36 ++++++++++++++++++++++-------- spec/models/concerns/reportable.rb | 30 +++++++++++++++++++++++++ 6 files changed, 71 insertions(+), 18 deletions(-) diff --git a/app/models/abilities/everyone.rb b/app/models/abilities/everyone.rb index 6cdc8b6c9..e2a59c140 100644 --- a/app/models/abilities/everyone.rb +++ b/app/models/abilities/everyone.rb @@ -7,20 +7,16 @@ module Abilities can [:read, :map, :summary, :share], Proposal can :read, Comment can :read, Poll - can :results, Poll do |poll| - poll.expired? && poll.results_enabled? - end - can :stats, Poll do |poll| - poll.expired? && poll.stats_enabled? - end + can :results, Poll, id: Poll.expired.results_enabled.ids + can :stats, Poll, id: Poll.expired.stats_enabled.ids can :read, Poll::Question can :read, User can [:read, :welcome], Budget can [:read], Budget can [:read], Budget::Group can [:read, :print, :json_data], Budget::Investment - can(:read_results, Budget) { |budget| budget.results_enabled? && budget.finished? } - can(:read_stats, Budget) { |budget| budget.stats_enabled? && budget.valuating_or_later? } + can :read_results, Budget, id: Budget.finished.results_enabled.ids + can :read_stats, Budget, id: Budget.valuating_or_later.stats_enabled.ids can :read_executions, Budget, phase: "finished" can :new, DirectMessage can [:read, :debate, :draft_publication, :allegations, :result_publication, diff --git a/app/models/budget.rb b/app/models/budget.rb index 57be318ca..26489419c 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -47,6 +47,7 @@ class Budget < ApplicationRecord scope :reviewing, -> { where(phase: "reviewing") } scope :selecting, -> { where(phase: "selecting") } scope :valuating, -> { where(phase: "valuating") } + scope :valuating_or_later, -> { where(phase: Budget::Phase.kind_or_later("valuating")) } scope :publishing_prices, -> { where(phase: "publishing_prices") } scope :balloting, -> { where(phase: "balloting") } scope :reviewing_ballots, -> { where(phase: "reviewing_ballots") } diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb index de90075a7..de3b8c0f1 100644 --- a/app/models/budget/phase.rb +++ b/app/models/budget/phase.rb @@ -32,6 +32,10 @@ class Budget define_singleton_method(phase) { find_by(kind: phase) } end + def self.kind_or_later(phase) + PHASE_KINDS[PHASE_KINDS.index(phase)..-1] + end + def next_enabled_phase next_phase&.enabled? ? next_phase : next_phase&.next_enabled_phase end @@ -92,7 +96,7 @@ class Budget end def in_phase_or_later?(phase) - PHASE_KINDS.index(kind) >= PHASE_KINDS.index(phase) + self.class.kind_or_later(phase).include?(kind) end end end diff --git a/app/models/concerns/reportable.rb b/app/models/concerns/reportable.rb index f584faa86..5a57551f8 100644 --- a/app/models/concerns/reportable.rb +++ b/app/models/concerns/reportable.rb @@ -4,6 +4,10 @@ module Reportable included do has_one :report, as: :process, inverse_of: :process, dependent: :destroy accepts_nested_attributes_for :report + + Report::KINDS.each do |kind| + scope "#{kind}_enabled", -> { joins(:report).where("reports.#{kind}": true) } + end end def report diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index 08fb0cea2..3b85e61d9 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -7,6 +7,33 @@ describe Budget do it_behaves_like "reportable" it_behaves_like "globalizable", :budget + describe "scopes" do + describe ".open" do + it "returns all budgets that are not in the finished phase" do + (Budget::Phase::PHASE_KINDS - ["finished"]).each do |phase| + budget = create(:budget, phase: phase) + expect(Budget.open).to include(budget) + end + end + end + + describe ".valuating_or_later" do + it "returns budgets valuating or later" do + valuating = create(:budget, :valuating) + finished = create(:budget, :finished) + + expect(Budget.valuating_or_later).to match_array([valuating, finished]) + end + + it "does not return budgets which haven't reached valuation" do + create(:budget, :drafting) + create(:budget, :selecting) + + expect(Budget.valuating_or_later).to be_empty + end + end + end + describe "name" do before do budget.update(name_en: "object name") @@ -172,15 +199,6 @@ describe Budget do end end - describe "#open" do - it "returns all budgets that are not in the finished phase" do - (Budget::Phase::PHASE_KINDS - ["finished"]).each do |phase| - budget = create(:budget, phase: phase) - expect(Budget.open).to include(budget) - end - end - end - describe "heading_price" do it "returns the heading price if the heading provided is part of the budget" do heading = create(:budget_heading, price: 100, budget: budget) diff --git a/spec/models/concerns/reportable.rb b/spec/models/concerns/reportable.rb index 107a739fb..e35e4e759 100644 --- a/spec/models/concerns/reportable.rb +++ b/spec/models/concerns/reportable.rb @@ -1,6 +1,36 @@ shared_examples "reportable" do let(:reportable) { create(model_name(described_class)) } + describe "scopes" do + describe ".results_enabled" do + it "includes records with results enabled" do + reportable.update!(results_enabled: true) + + expect(described_class.results_enabled).to eq [reportable] + end + + it "does not include records without results enabled" do + reportable.update!(results_enabled: false) + + expect(described_class.results_enabled).to be_empty + end + end + + describe ".stats_enabled" do + it "includes records with stats enabled" do + reportable.update!(stats_enabled: true) + + expect(described_class.stats_enabled).to eq [reportable] + end + + it "does not include records without stats enabled" do + reportable.update!(stats_enabled: false) + + expect(described_class.stats_enabled).to be_empty + end + end + end + describe "#results_enabled" do it "can write and read the attribute" do reportable.results_enabled = true