From 710a1d8357db6d14699dfb7ac52a105b6fc567d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Tue, 1 Oct 2019 02:19:41 +0200
Subject: [PATCH] Split Poll::Booth.search in two methods

We use this method in two different scenarios. In an AJAX request, we
don't want to return every booth if the search is blank. However, in a
normal HTTP GET request, we want to return every record when the search
is empty, as we do everywhere else.

It's possible the behaviour of the AJAX call is unusual, since it
searches all booths, and not just the ones assigned to a poll. If we
changed this behaviour, we could simplify the code and remove the
`quick_search` method.
---
 .../poll/booth_assignments_controller.rb      |  2 +-
 .../admin/poll/booths_controller.rb           |  2 +-
 app/models/poll/booth.rb                      |  9 +++++++-
 spec/models/poll/booth_spec.rb                | 22 ++++++++++++++++++-
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/app/controllers/admin/poll/booth_assignments_controller.rb b/app/controllers/admin/poll/booth_assignments_controller.rb
index 0fc6e386a..627e18ef0 100644
--- a/app/controllers/admin/poll/booth_assignments_controller.rb
+++ b/app/controllers/admin/poll/booth_assignments_controller.rb
@@ -9,7 +9,7 @@ class Admin::Poll::BoothAssignmentsController < Admin::Poll::BaseController
 
   def search_booths
     load_search
-    @booths = ::Poll::Booth.search(@search)
+    @booths = ::Poll::Booth.quick_search(@search)
     respond_to do |format|
       format.js
     end
diff --git a/app/controllers/admin/poll/booths_controller.rb b/app/controllers/admin/poll/booths_controller.rb
index c27a65694..9f80a0de4 100644
--- a/app/controllers/admin/poll/booths_controller.rb
+++ b/app/controllers/admin/poll/booths_controller.rb
@@ -2,7 +2,7 @@ class Admin::Poll::BoothsController < Admin::Poll::BaseController
   load_and_authorize_resource class: "Poll::Booth"
 
   def index
-    @booths = @booths.search params[:search] if params[:search]
+    @booths = @booths.search(params[:search])
     @booths = @booths.order(name: :asc).page(params[:page])
   end
 
diff --git a/app/models/poll/booth.rb b/app/models/poll/booth.rb
index 4c45b9d1a..64174a7a6 100644
--- a/app/models/poll/booth.rb
+++ b/app/models/poll/booth.rb
@@ -7,10 +7,17 @@ class Poll
     validates :name, presence: true, uniqueness: true
 
     def self.search(terms)
-      return Booth.none if terms.blank?
       Booth.where("name ILIKE ? OR location ILIKE ?", "%#{terms}%", "%#{terms}%")
     end
 
+    def self.quick_search(terms)
+      if terms.blank?
+        Booth.none
+      else
+        search(terms)
+      end
+    end
+
     def self.available
       where(polls: { id: Poll.current_or_recounting }).joins(:polls)
     end
diff --git a/spec/models/poll/booth_spec.rb b/spec/models/poll/booth_spec.rb
index 95a9b7ad3..e19d82bd6 100644
--- a/spec/models/poll/booth_spec.rb
+++ b/spec/models/poll/booth_spec.rb
@@ -13,7 +13,7 @@ describe Poll::Booth do
     expect(booth).not_to be_valid
   end
 
-  describe "#search" do
+  describe ".search" do
     it "finds booths searching by name or location" do
       booth1 = create(:poll_booth, name: "Booth number 1", location: "City center")
       booth2 = create(:poll_booth, name: "Central", location: "Town hall")
@@ -22,6 +22,26 @@ describe Poll::Booth do
       expect(Poll::Booth.search("hall")).to eq([booth2])
       expect(Poll::Booth.search("cen")).to match_array [booth1, booth2]
     end
+
+    it "returns all booths if search term is blank" do
+      2.times { create(:poll_booth) }
+
+      expect(Poll::Booth.search("").count).to eq 2
+    end
+
+    it "returns all booths if search term is nil" do
+      2.times { create(:poll_booth) }
+
+      expect(Poll::Booth.search(nil).count).to eq 2
+    end
+  end
+
+  describe ".quick_search" do
+    it "returns no booths if search term is blank" do
+      create(:poll_booth)
+
+      expect(Poll::Booth.quick_search("")).to be_empty
+    end
   end
 
   describe ".available" do