From 355b78ad5e2414d7a7357de36451796beb85afd9 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 11 Apr 2022 18:40:29 +0100 Subject: [PATCH 1/5] Allow OR filters within scope categories and AND filters across them --- app/controllers/case_logs_controller.rb | 12 ++++------ app/models/case_log.rb | 10 +++++++- spec/models/case_log_spec.rb | 32 +++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index f9d760a3a..5b9bc5eb5 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -121,18 +121,16 @@ private end def filtered_case_logs - user_case_logs = current_user.case_logs + query = CaseLog.for_organisation(current_user.organisation) if session[:case_logs_filters].present? filters = JSON.parse(session[:case_logs_filters]) + filters.each do |category, values| + next unless values.reject(&:empty?).present? - %w[status year].each do |category| - if filters[category] - filtered_by_category = filters[category].select(&:present?).map { |filter| user_case_logs.public_send("filter_by_#{category}", filter) }.flatten - user_case_logs = CaseLog.where(id: filtered_by_category.map(&:id)) - end + query = query.public_send("filter_by_#{category}", values) end end - user_case_logs + query.all end def set_session_filters diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 376ae410a..7ff88e31d 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -35,7 +35,15 @@ class CaseLog < ApplicationRecord scope :for_organisation, ->(org) { where(owning_organisation: org).or(where(managing_organisation: org)) } scope :filter_by_status, ->(status) { where status: status } - scope :filter_by_year, ->(year) { where(startdate: Time.zone.local(year, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) } + scope :filter_by_year, ->(year) { + years = Array(year) + first_year = years.shift + query = where('startdate >= ?', Time.utc(first_year.to_i, 4, 1)).where('startdate <= ?',Time.utc(first_year.to_i + 1, 3, 31).end_of_day) + years.each do |year| + query = query.or(where('startdate >= ?', Time.utc(year.to_i, 4, 1)).where('startdate <= ?',Time.utc(year.to_i + 1, 3, 31).end_of_day)) + end + query.all + } AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze OPTIONAL_FIELDS = %w[postcode_known la_known first_time_property_let_as_social_housing tenant_code propcode].freeze diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index b648f27f2..ef2016c8e 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -1825,4 +1825,36 @@ RSpec.describe CaseLog do end end end + + describe "scopes" do + before do + FactoryBot.create(:case_log, :in_progress, startdate: Time.utc(2021, 5, 3)) + FactoryBot.create(:case_log, :completed, startdate: Time.utc(2021, 5, 3)) + FactoryBot.create(:case_log, startdate: Time.utc(2022, 6, 3)) + end + + context "when filtering by year" do + it "allows filtering on a single year" do + expect(CaseLog.filter_by_year("2021").count).to eq(2) + end + + it "allows filtering by multiple years using OR" do + expect(CaseLog.filter_by_year(["2021", "2022"]).count).to eq(3) + end + + it "can filter by year(s) AND status" do + expect(CaseLog.filter_by_year(["2021", "2022"]).filter_by_status("completed").count).to eq(1) + end + end + + context "when filtering on status" do + it "allows filtering on a single status" do + expect(CaseLog.filter_by_status("in_progress").count).to eq(2) + end + + it "allows filtering on multiple statuses" do + expect(CaseLog.filter_by_status(["in_progress", "completed"]).count).to eq(3) + end + end + end end From 2ffc7230f80728e13143a558e7c4861dc52b74c2 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 11 Apr 2022 18:47:09 +0100 Subject: [PATCH 2/5] Rubocop --- app/controllers/case_logs_controller.rb | 2 +- app/models/case_log.rb | 8 ++++---- spec/models/case_log_spec.rb | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 5b9bc5eb5..2402ebe15 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -125,7 +125,7 @@ private if session[:case_logs_filters].present? filters = JSON.parse(session[:case_logs_filters]) filters.each do |category, values| - next unless values.reject(&:empty?).present? + next if values.reject(&:empty?).blank? query = query.public_send("filter_by_#{category}", values) end diff --git a/app/models/case_log.rb b/app/models/case_log.rb index 7ff88e31d..d19957594 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -35,12 +35,12 @@ class CaseLog < ApplicationRecord scope :for_organisation, ->(org) { where(owning_organisation: org).or(where(managing_organisation: org)) } scope :filter_by_status, ->(status) { where status: status } - scope :filter_by_year, ->(year) { - years = Array(year) + scope :filter_by_year, lambda { |year_s| + years = Array(year_s) first_year = years.shift - query = where('startdate >= ?', Time.utc(first_year.to_i, 4, 1)).where('startdate <= ?',Time.utc(first_year.to_i + 1, 3, 31).end_of_day) + query = where("startdate >= ?", Time.utc(first_year.to_i, 4, 1)).where("startdate <= ?", Time.utc(first_year.to_i + 1, 3, 31).end_of_day) years.each do |year| - query = query.or(where('startdate >= ?', Time.utc(year.to_i, 4, 1)).where('startdate <= ?',Time.utc(year.to_i + 1, 3, 31).end_of_day)) + query = query.or(where("startdate >= ?", Time.utc(year.to_i, 4, 1)).where("startdate <= ?", Time.utc(year.to_i + 1, 3, 31).end_of_day)) end query.all } diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb index ef2016c8e..8c10a4c8d 100644 --- a/spec/models/case_log_spec.rb +++ b/spec/models/case_log_spec.rb @@ -1835,25 +1835,25 @@ RSpec.describe CaseLog do context "when filtering by year" do it "allows filtering on a single year" do - expect(CaseLog.filter_by_year("2021").count).to eq(2) + expect(described_class.filter_by_year("2021").count).to eq(2) end it "allows filtering by multiple years using OR" do - expect(CaseLog.filter_by_year(["2021", "2022"]).count).to eq(3) + expect(described_class.filter_by_year(%w[2021 2022]).count).to eq(3) end it "can filter by year(s) AND status" do - expect(CaseLog.filter_by_year(["2021", "2022"]).filter_by_status("completed").count).to eq(1) + expect(described_class.filter_by_year(%w[2021 2022]).filter_by_status("completed").count).to eq(1) end end context "when filtering on status" do it "allows filtering on a single status" do - expect(CaseLog.filter_by_status("in_progress").count).to eq(2) + expect(described_class.filter_by_status("in_progress").count).to eq(2) end it "allows filtering on multiple statuses" do - expect(CaseLog.filter_by_status(["in_progress", "completed"]).count).to eq(3) + expect(described_class.filter_by_status(%w[in_progress completed]).count).to eq(3) end end end From cec447e790a1fe345325235dbd8cee767af9db41 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Mon, 11 Apr 2022 18:56:30 +0100 Subject: [PATCH 3/5] Remove some of the N+1 query madness by preloading required organisations --- app/controllers/case_logs_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 2402ebe15..3f0e2ea53 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -130,7 +130,7 @@ private query = query.public_send("filter_by_#{category}", values) end end - query.all + query.all.includes(:owning_organisation, :managing_organisation) end def set_session_filters From 7cfbfce7c1401fdfe3e661d49f51f385cb7ded42 Mon Sep 17 00:00:00 2001 From: baarkerlounger Date: Tue, 12 Apr 2022 10:46:06 +0100 Subject: [PATCH 4/5] Refactor scopes --- app/controllers/case_logs_controller.rb | 2 +- app/models/case_log.rb | 11 +++++------ app/views/case_logs/_log_filters.erb | 10 +++++----- app/views/filters/_checkbox_filter.html.erb | 4 ++-- spec/models/case_log_spec.rb | 8 ++++---- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/app/controllers/case_logs_controller.rb b/app/controllers/case_logs_controller.rb index 3f0e2ea53..b4b4f4851 100644 --- a/app/controllers/case_logs_controller.rb +++ b/app/controllers/case_logs_controller.rb @@ -135,7 +135,7 @@ private def set_session_filters new_filters = session[:case_logs_filters].present? ? JSON.parse(session[:case_logs_filters]) : {} - %i[status year].each { |filter| new_filters[filter] = params[filter] if params[filter].present? } + %i[status years].each { |filter| new_filters[filter] = params[filter] if params[filter].present? } session[:case_logs_filters] = new_filters.to_json end end diff --git a/app/models/case_log.rb b/app/models/case_log.rb index d19957594..b398c4eb6 100644 --- a/app/models/case_log.rb +++ b/app/models/case_log.rb @@ -35,15 +35,14 @@ class CaseLog < ApplicationRecord scope :for_organisation, ->(org) { where(owning_organisation: org).or(where(managing_organisation: org)) } scope :filter_by_status, ->(status) { where status: status } - scope :filter_by_year, lambda { |year_s| - years = Array(year_s) + scope :filter_by_years, lambda { |years| first_year = years.shift - query = where("startdate >= ?", Time.utc(first_year.to_i, 4, 1)).where("startdate <= ?", Time.utc(first_year.to_i + 1, 3, 31).end_of_day) - years.each do |year| - query = query.or(where("startdate >= ?", Time.utc(year.to_i, 4, 1)).where("startdate <= ?", Time.utc(year.to_i + 1, 3, 31).end_of_day)) - end + query = filter_by_year(first_year) + years.each { |year| query = query.or(filter_by_year(year)) } query.all } + scope :filter_by_year, ->(year) { where(startdate: Time.utc(year.to_i, 4, 1)...Time.utc(year.to_i + 1, 4, 1)) } + AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze OPTIONAL_FIELDS = %w[postcode_known la_known first_time_property_let_as_social_housing tenant_code propcode].freeze diff --git a/app/views/case_logs/_log_filters.erb b/app/views/case_logs/_log_filters.erb index fb1cb32fa..09432faa7 100644 --- a/app/views/case_logs/_log_filters.erb +++ b/app/views/case_logs/_log_filters.erb @@ -2,9 +2,9 @@

Filters

-