From 03b21e4a2a734d67f33c62050a73ab31ca77b0ce Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Tue, 7 Mar 2023 16:26:28 +0000 Subject: [PATCH] CLDC-2035 Fix search bug (#1327) * feat: fix search bug * feat: add correct sales log search filter behaviour * refactor: removal of logic from erbs * refactor: linting * test: add tests to check log filtering works --- app/components/search_component.rb | 2 + app/helpers/logs_helper.rb | 16 ++++++ app/models/lettings_log.rb | 1 - app/models/log.rb | 1 + app/models/sales_log.rb | 7 ++- app/views/logs/index.html.erb | 4 +- spec/services/filter_service_spec.rb | 80 +++++++++++++++++++++++----- 7 files changed, 94 insertions(+), 17 deletions(-) diff --git a/app/components/search_component.rb b/app/components/search_component.rb index d13882645..36d621240 100644 --- a/app/components/search_component.rb +++ b/app/components/search_component.rb @@ -23,6 +23,8 @@ class SearchComponent < ViewComponent::Base user_path(current_user) elsif request.path.include?("organisations") organisations_path + elsif request.path.include?("sales-logs") + sales_logs_path elsif request.path.include?("logs") lettings_logs_path end diff --git a/app/helpers/logs_helper.rb b/app/helpers/logs_helper.rb index 88ab2b314..7653b7e5c 100644 --- a/app/helpers/logs_helper.rb +++ b/app/helpers/logs_helper.rb @@ -23,4 +23,20 @@ module LogsHelper array = bulk_upload ? [bulk_upload.id] : [] array.index_with { |_bulk_upload_id| "With logs from bulk upload" } end + + def search_label_for_controller(controller) + case log_type_for_controller(controller) + when "lettings" + "Search by log ID, tenant code, property reference or postcode" + when "sales" + "Search by log ID, purchaser code or postcode" + end + end + + def csv_download_url_for_controller(controller) + case log_type_for_controller(controller) + when "lettings" + csv_download_lettings_logs_path(search: params["search"]) + end + end end diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 6864738cf..6491c100f 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -40,7 +40,6 @@ class LettingsLog < Log scope :filter_by_year, ->(year) { where(startdate: Time.zone.local(year.to_i, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) } scope :filter_by_tenant_code, ->(tenant_code) { where("tenancycode ILIKE ?", "%#{tenant_code}%") } scope :filter_by_propcode, ->(propcode) { where("propcode ILIKE ?", "%#{propcode}%") } - scope :filter_by_postcode, ->(postcode_full) { where("REPLACE(postcode_full, ' ', '') ILIKE ?", "%#{postcode_full.delete(' ')}%") } scope :filter_by_location_postcode, ->(postcode_full) { left_joins(:location).where("REPLACE(locations.postcode, ' ', '') ILIKE ?", "%#{postcode_full.delete(' ')}%") } scope :search_by, lambda { |param| filter_by_location_postcode(param) diff --git a/app/models/log.rb b/app/models/log.rb index 505839c69..f6f5f4173 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -18,6 +18,7 @@ class Log < ApplicationRecord years.each { |year| query = query.or(filter_by_year(year)) } query.all } + scope :filter_by_postcode, ->(postcode_full) { where("REPLACE(postcode_full, ' ', '') ILIKE ?", "%#{postcode_full.delete(' ')}%") } scope :filter_by_id, ->(id) { where(id:) } scope :filter_by_user, lambda { |selected_user, user| if !selected_user.include?("all") && user.present? diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index b2c6b08da..629215386 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -33,7 +33,12 @@ class SalesLog < Log before_validation :set_derived_fields! scope :filter_by_year, ->(year) { where(saledate: Time.zone.local(year.to_i, 4, 1)...Time.zone.local(year.to_i + 1, 4, 1)) } - scope :search_by, ->(param) { filter_by_id(param) } + scope :filter_by_purchaser_code, ->(purchid) { where("purchid ILIKE ?", "%#{purchid}%") } + scope :search_by, lambda { |param| + filter_by_purchaser_code(param) + .or(filter_by_postcode(param)) + .or(filter_by_id(param)) + } scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org) } OPTIONAL_FIELDS = %w[saledate_check purchid monthly_charges_value_check old_persons_shared_ownership_value_check].freeze diff --git a/app/views/logs/index.html.erb b/app/views/logs/index.html.erb index 31768c731..0e4cd76df 100644 --- a/app/views/logs/index.html.erb +++ b/app/views/logs/index.html.erb @@ -64,9 +64,9 @@ <%= render partial: "log_filters" %>
- <%= render SearchComponent.new(current_user:, search_label: "Search by log ID, tenant code, property reference or postcode", value: @searched) %> + <%= render SearchComponent.new(current_user:, search_label: search_label_for_controller(controller), value: @searched) %> <%= govuk_section_break(visible: true, size: "m") %> - <%= render partial: "log_list", locals: { logs: @logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: csv_download_lettings_logs_path(search: @search_term) } %> + <%= render partial: "log_list", locals: { logs: @logs, title: "Logs", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count, csv_download_url: csv_download_url_for_controller(controller) } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
diff --git a/spec/services/filter_service_spec.rb b/spec/services/filter_service_spec.rb index 1e892fd56..2c89b1302 100644 --- a/spec/services/filter_service_spec.rb +++ b/spec/services/filter_service_spec.rb @@ -2,26 +2,80 @@ require "rails_helper" describe FilterService do describe "filter_by_search" do - before do - FactoryBot.create_list(:organisation, 5) - FactoryBot.create(:organisation, name: "Acme LTD") - end + context "when filtering organisations" do + before do + FactoryBot.create_list(:organisation, 5) + FactoryBot.create(:organisation, name: "Acme LTD") + end + + let(:organisation_list) { Organisation.all } + + context "when given a search term" do + let(:search_term) { "Acme" } - let(:organisation_list) { Organisation.all } + it "filters the collection on search term" do + expect(described_class.filter_by_search(organisation_list, search_term).count).to eq(1) + end + end - context "when given a search term" do - let(:search_term) { "Acme" } + context "when not given a search term" do + let(:search_term) { nil } - it "filters the collection on search term" do - expect(described_class.filter_by_search(organisation_list, search_term).count).to eq(1) + it "does not filter the given collection" do + expect(described_class.filter_by_search(organisation_list, search_term).count).to eq(6) + end end end - context "when not given a search term" do - let(:search_term) { nil } + context "when filtering logs" do + context "when filtering lettings logs" do + before do + FactoryBot.create_list(:lettings_log, 5) + FactoryBot.create(:lettings_log, postcode_full: "SW1 1AA") + end + + let(:lettings_log_list) { LettingsLog.all } + + context "when given a postcode" do + let(:search_term) { "SW1 1AA" } + + it "filters the collection on search term" do + expect(described_class.filter_by_search(lettings_log_list, search_term).count).to eq(1) + end + end + + context "when not given a search term" do + let(:search_term) { nil } + + it "does not filter the given collection" do + expect(described_class.filter_by_search(lettings_log_list, search_term).count).to eq(6) + end + end + end + + context "when filtering sales logs" do + before do + FactoryBot.create_list(:sales_log, 5) + FactoryBot.create(:sales_log, purchid: "2") + end + + let(:sales_log_list) { SalesLog.all } + + context "when given a purchid" do + let(:search_term) { "2" } + + it "filters the collection on search term" do + expect(described_class.filter_by_search(sales_log_list, search_term).count).to eq(1) + end + end + + context "when not given a search term" do + let(:search_term) { nil } - it "does not filter the given collection" do - expect(described_class.filter_by_search(organisation_list, search_term).count).to eq(6) + it "does not filter the given collection" do + expect(described_class.filter_by_search(sales_log_list, search_term).count).to eq(6) + end + end end end end