From 45c6a70d915df8bae9f309f0d9782dcb492eab6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 26 Sep 2019 18:56:34 +0200
Subject: [PATCH 1/2] Fix extra `nil` added to assigned investment IDs

When `valuator_group` was `nil`, `[valuator_group&.investment_ids]` is
evaluated to `nil`, and so we were adding an extra element to the array.

We could add a `compact` call to the resulting array, but I find it
easier to convert `nil` to an array using `to_a`.
---
 app/models/valuator.rb       | 2 +-
 spec/models/valuator_spec.rb | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/models/valuator.rb b/app/models/valuator.rb
index 625395f0c..994f7ba58 100644
--- a/app/models/valuator.rb
+++ b/app/models/valuator.rb
@@ -18,7 +18,7 @@ class Valuator < ApplicationRecord
   end
 
   def assigned_investment_ids
-    investment_ids + [valuator_group&.investment_ids].flatten
+    investment_ids + valuator_group&.investment_ids.to_a
   end
 
 end
diff --git a/spec/models/valuator_spec.rb b/spec/models/valuator_spec.rb
index 7f1efa3c7..021d47d33 100644
--- a/spec/models/valuator_spec.rb
+++ b/spec/models/valuator_spec.rb
@@ -28,8 +28,8 @@ describe Valuator do
       investment2.valuators << valuator
 
       assigned_investment_ids = valuator.assigned_investment_ids
-      expect(assigned_investment_ids).to include investment1.id
-      expect(assigned_investment_ids).to include investment2.id
+
+      expect(assigned_investment_ids).to match_array [investment1.id, investment2.id]
       expect(assigned_investment_ids).not_to include investment3.id
     end
 
@@ -45,8 +45,8 @@ describe Valuator do
       investment2.valuator_groups << group
 
       assigned_investment_ids = valuator.assigned_investment_ids
-      expect(assigned_investment_ids).to include investment1.id
-      expect(assigned_investment_ids).to include investment2.id
+
+      expect(assigned_investment_ids).to match_array [investment1.id, investment2.id]
       expect(assigned_investment_ids).not_to include investment3.id
     end
   end

From 3b11f8b567040be9af9e2a07632e7370c5e21bb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 26 Sep 2019 19:15:15 +0200
Subject: [PATCH 2/2] Avoid duplicate records in `current_or_recounting`

Joining two scopes with `+` does not remove duplicate records. Luckily
now that we've upgraded to Rails 5, we can join scopes using `.or`.

The test was testing for the presence of elements, bud didn't test for
duplicate records. Testing the exact contents of the array revealed this
behaviour.
---
 app/models/poll.rb            | 4 ++--
 spec/models/poll/poll_spec.rb | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/app/models/poll.rb b/app/models/poll.rb
index 9f48cc258..2a1a0759d 100644
--- a/app/models/poll.rb
+++ b/app/models/poll.rb
@@ -42,7 +42,7 @@ class Poll < ApplicationRecord
   scope :public_polls, -> { where(related: nil) }
   scope :current,  -> { where("starts_at <= ? and ? <= ends_at", Date.current.beginning_of_day, Date.current.beginning_of_day) }
   scope :expired,  -> { where("ends_at < ?", Date.current.beginning_of_day) }
-  scope :recounting, -> { Poll.where(ends_at: (Date.current.beginning_of_day - RECOUNT_DURATION)..Date.current.beginning_of_day) }
+  scope :recounting, -> { where(ends_at: (Date.current.beginning_of_day - RECOUNT_DURATION)..Date.current.beginning_of_day) }
   scope :published, -> { where("published = ?", true) }
   scope :by_geozone_id, ->(geozone_id) { where(geozones: { id: geozone_id }.joins(:geozones)) }
   scope :public_for_api, -> { all }
@@ -86,7 +86,7 @@ class Poll < ApplicationRecord
   end
 
   def self.current_or_recounting
-    current + recounting
+    current.or(recounting)
   end
 
   def answerable_by?(user)
diff --git a/spec/models/poll/poll_spec.rb b/spec/models/poll/poll_spec.rb
index 59af1682a..21c05f46c 100644
--- a/spec/models/poll/poll_spec.rb
+++ b/spec/models/poll/poll_spec.rb
@@ -119,9 +119,9 @@ describe Poll do
 
       recounting_polls = Poll.recounting
 
+      expect(recounting_polls).to eq [recounting]
       expect(recounting_polls).not_to include(current)
       expect(recounting_polls).not_to include(expired)
-      expect(recounting_polls).to include(recounting)
     end
   end
 
@@ -133,8 +133,7 @@ describe Poll do
 
       current_or_recounting = Poll.current_or_recounting
 
-      expect(current_or_recounting).to include(current)
-      expect(current_or_recounting).to include(recounting)
+      expect(current_or_recounting).to match_array [current, recounting]
       expect(current_or_recounting).not_to include(expired)
     end
   end