From 1f225c59ed142462a0d64a6acc1fe3844c741a5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 20 Mar 2019 17:53:24 +0100
Subject: [PATCH] Use PHASE_KINDS to calculate "or_later?" methods

This implementation is a bit more robust because we don't have to change
any of the "or_later?" methods if we add or remove a new phase.

We could also use metaprogramming to reduce code duplication in these
methods. So far, I've decided to keep the code simple since the
duplication seems reasonable.
---
 app/models/budget.rb       |  4 ++--
 app/models/budget/phase.rb | 12 ++++++++++
 spec/models/budget_spec.rb | 49 +++++++++++++++++++++-----------------
 3 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/app/models/budget.rb b/app/models/budget.rb
index 8cc5c9b8f..19cc85a10 100644
--- a/app/models/budget.rb
+++ b/app/models/budget.rb
@@ -126,7 +126,7 @@ class Budget < ApplicationRecord
   end
 
   def publishing_prices_or_later?
-    publishing_prices? || balloting_or_later?
+    current_phase&.publishing_prices_or_later?
   end
 
   def balloting_process?
@@ -134,7 +134,7 @@ class Budget < ApplicationRecord
   end
 
   def balloting_or_later?
-    balloting_process? || finished?
+    current_phase&.balloting_or_later?
   end
 
   def heading_price(heading)
diff --git a/app/models/budget/phase.rb b/app/models/budget/phase.rb
index 5204d217a..cbacefa60 100644
--- a/app/models/budget/phase.rb
+++ b/app/models/budget/phase.rb
@@ -47,6 +47,14 @@ class Budget
       end
     end
 
+    def publishing_prices_or_later?
+      in_phase_or_later?("publishing_prices")
+    end
+
+    def balloting_or_later?
+      in_phase_or_later?("balloting")
+    end
+
     private
 
       def adjust_date_ranges
@@ -84,5 +92,9 @@ class Budget
         end
       end
 
+      def in_phase_or_later?(phase)
+        PHASE_KINDS.index(kind) >= PHASE_KINDS.index(phase)
+      end
+
   end
 end
diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb
index da069f558..d5fc6ffc4 100644
--- a/spec/models/budget_spec.rb
+++ b/spec/models/budget_spec.rb
@@ -97,33 +97,38 @@ describe Budget do
       expect(budget).to be_finished
     end
 
-    it "balloting_or_later?" do
-      budget.phase = "drafting"
-      expect(budget).not_to be_balloting_or_later
+    describe "#publishing_prices_or_later?" do
+      it "returns false before publishing prices" do
+        budget.phase = "valuating"
+        expect(budget).not_to be_publishing_prices_or_later
+      end
 
-      budget.phase = "accepting"
-      expect(budget).not_to be_balloting_or_later
+      it "returns true while publishing prices" do
+        budget.phase = "publishing_prices"
+        expect(budget).to be_publishing_prices_or_later
+      end
 
-      budget.phase = "reviewing"
-      expect(budget).not_to be_balloting_or_later
+      it "returns true after publishing prices" do
+        budget.phase = "balloting"
+        expect(budget).to be_publishing_prices_or_later
+      end
+    end
 
-      budget.phase = "selecting"
-      expect(budget).not_to be_balloting_or_later
+    describe "#balloting_or_later?" do
+      it "returns false before balloting" do
+        budget.phase = "publishing_prices"
+        expect(budget).not_to be_balloting_or_later
+      end
 
-      budget.phase = "valuating"
-      expect(budget).not_to be_balloting_or_later
+      it "returns true while balloting" do
+        budget.phase = "balloting"
+        expect(budget).to be_balloting_or_later
+      end
 
-      budget.phase = "publishing_prices"
-      expect(budget).not_to be_balloting_or_later
-
-      budget.phase = "balloting"
-      expect(budget).to be_balloting_or_later
-
-      budget.phase = "reviewing_ballots"
-      expect(budget).to be_balloting_or_later
-
-      budget.phase = "finished"
-      expect(budget).to be_balloting_or_later
+      it "returns true after balloting" do
+        budget.phase = "finished"
+        expect(budget).to be_balloting_or_later
+      end
     end
   end