From 3aee2923a254de9896e961ec3e3a3ed8c4ac4555 Mon Sep 17 00:00:00 2001
From: natdeanlewissoftwire
 <94526761+natdeanlewissoftwire@users.noreply.github.com>
Date: Wed, 27 Sep 2023 14:03:47 +0100
Subject: [PATCH] CLDC-2816 scheme filter bug 2 (#1946)

* feat: move ordering after filtering to avoid ORDER BY using expressions not in SELECT DISTINCT list

* feat: add multiple schemes so tests will fail if this bug appears again

* feat: include schemes and locations with nil confirmed values in incomplete scopes

* refactor: lint
---
 app/models/location.rb       | 1 +
 app/models/scheme.rb         | 1 +
 spec/models/location_spec.rb | 6 ++++--
 spec/models/scheme_spec.rb   | 7 +++++--
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/app/models/location.rb b/app/models/location.rb
index f25fd7d5d..954c034f9 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -49,6 +49,7 @@ class Location < ApplicationRecord
 
   scope :incomplete, lambda {
     where.not(confirmed: true)
+      .or(where(confirmed: nil))
   }
 
   scope :deactivated, lambda {
diff --git a/app/models/scheme.rb b/app/models/scheme.rb
index bab388b5d..55d15dfb2 100644
--- a/app/models/scheme.rb
+++ b/app/models/scheme.rb
@@ -42,6 +42,7 @@ class Scheme < ApplicationRecord
 
   scope :incomplete, lambda {
     where.not(confirmed: true)
+         .or(where(confirmed: nil))
     .or(where.not(id: Location.select(:scheme_id).where(confirmed: true).distinct))
     .where.not(id: joins(:scheme_deactivation_periods).reactivating_soon.pluck(:id))
     .where.not(id: joins(:scheme_deactivation_periods).deactivated.pluck(:id))
diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb
index f24a5f4fa..4425e1a6c 100644
--- a/spec/models/location_spec.rb
+++ b/spec/models/location_spec.rb
@@ -932,6 +932,7 @@ RSpec.describe Location, type: :model do
 
   describe "filter by status" do
     let!(:incomplete_location) { FactoryBot.create(:location, :incomplete, startdate: Time.zone.local(2022, 4, 1)) }
+    let!(:incomplete_location_with_nil_confirmed) { FactoryBot.create(:location, :incomplete, startdate: Time.zone.local(2022, 4, 1), confirmed: nil) }
     let!(:active_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 1)) }
     let(:deactivating_soon_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 1)) }
     let(:deactivated_location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 1)) }
@@ -954,8 +955,9 @@ RSpec.describe Location, type: :model do
 
     context "when filtering by incomplete status" do
       it "returns only incomplete locations" do
-        expect(described_class.filter_by_status(%w[incomplete]).count).to eq(1)
-        expect(described_class.filter_by_status(%w[incomplete]).first).to eq(incomplete_location)
+        expect(described_class.filter_by_status(%w[incomplete]).count).to eq(2)
+        expect(described_class.filter_by_status(%w[incomplete])).to include(incomplete_location)
+        expect(described_class.filter_by_status(%w[incomplete])).to include(incomplete_location_with_nil_confirmed)
       end
     end
 
diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb
index 1e578fed5..b63866675 100644
--- a/spec/models/scheme_spec.rb
+++ b/spec/models/scheme_spec.rb
@@ -112,6 +112,7 @@ RSpec.describe Scheme, type: :model do
       context "when filtering by status" do
         let!(:incomplete_scheme) { FactoryBot.create(:scheme, :incomplete, service_name: "name") }
         let!(:incomplete_scheme_2) { FactoryBot.create(:scheme, :incomplete, service_name: "name") }
+        let!(:incomplete_scheme_with_nil_confirmed) { FactoryBot.create(:scheme, :incomplete, service_name: "name", confirmed: nil) }
         let(:active_scheme) { FactoryBot.create(:scheme) }
         let(:active_scheme_2) { FactoryBot.create(:scheme) }
         let(:deactivating_soon_scheme) { FactoryBot.create(:scheme) }
@@ -143,17 +144,19 @@ RSpec.describe Scheme, type: :model do
 
         context "when filtering by incomplete status" do
           it "returns only incomplete schemes" do
-            expect(described_class.filter_by_status(%w[incomplete]).count).to eq(2)
+            expect(described_class.filter_by_status(%w[incomplete]).count).to eq(3)
             expect(described_class.filter_by_status(%w[incomplete])).to include(incomplete_scheme)
             expect(described_class.filter_by_status(%w[incomplete])).to include(incomplete_scheme_2)
+            expect(described_class.filter_by_status(%w[incomplete])).to include(incomplete_scheme_with_nil_confirmed)
           end
         end
 
         context "when filtering by incomplete status and searching" do
           it "returns only incomplete schemes" do
-            expect(described_class.search_by("name").filter_by_status(%w[incomplete]).count).to eq(2)
+            expect(described_class.search_by("name").filter_by_status(%w[incomplete]).count).to eq(3)
             expect(described_class.search_by("name").filter_by_status(%w[incomplete])).to include(incomplete_scheme)
             expect(described_class.search_by("name").filter_by_status(%w[incomplete])).to include(incomplete_scheme_2)
+            expect(described_class.search_by("name").filter_by_status(%w[incomplete])).to include(incomplete_scheme_with_nil_confirmed)
           end
         end