From 9ef4ea19cff4b6610738f76c0376b709043f3b5e Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Wed, 25 Oct 2023 09:57:41 +0100 Subject: [PATCH] CLDC-2833 Add needstype filter (#1998) * feat: add needstype filter * feat: update filters view * feat: use helper methods for view * feat: update helper methods, and show managing and owning filters as well in support secondary nav * refactor: lint * feat: update helper logic * feat: update tests * feat: add needstype test and slight refactor * refactor: reorganise filter lists * refactor: erblint * Always show filter for support, only check visible logs * Remove needstype for support users on orgs page --------- Co-authored-by: Kat --- app/helpers/filters_helper.rb | 33 ++++++++++++++++++- app/models/lettings_log.rb | 7 ++++ app/models/user.rb | 4 +-- app/views/logs/_log_filters.html.erb | 48 +++++++++++++++++----------- spec/features/organisation_spec.rb | 26 ++++++++++++--- spec/features/user_spec.rb | 4 +-- spec/models/user_spec.rb | 6 ++-- 7 files changed, 96 insertions(+), 32 deletions(-) diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index e2da5b0ed..a5d825d55 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -26,6 +26,7 @@ module FiltersHelper filters["organisation"].present? || filters["managing_organisation"].present? || filters["status"]&.compact_blank&.any? || + filters["needstypes"]&.compact_blank&.any? || filters["years"]&.compact_blank&.any? || filters["bulk_upload_id"].present? end @@ -56,6 +57,13 @@ module FiltersHelper }.freeze end + def needstype_filters + { + "1" => "General needs", + "2" => "Supported housing", + }.freeze + end + def location_status_filters { "incomplete" => "Incomplete", @@ -108,6 +116,29 @@ module FiltersHelper user.support? || org.stock_owners.count > 1 || (org.holds_own_stock? && org.stock_owners.count.positive?) end + def logs_for_both_needstypes_present?(organisation) + return true if current_user.support? && organisation.blank? + return [1, 2].all? { |needstype| organisation.lettings_logs.visible.where(needstype:).count.positive? } if current_user.support? + + [1, 2].all? { |needstype| current_user.lettings_logs.visible.where(needstype:).count.positive? } + end + + def non_support_with_multiple_owning_orgs? + current_user.organisation.stock_owners.count > 1 && user_lettings_path? + end + + def non_support_with_multiple_managing_orgs? + current_user.organisation.managing_agents.count > 1 && user_lettings_path? + end + + def user_lettings_path? + request.path == "/lettings-logs" + end + + def user_or_org_lettings_path? + request.path.include?("/lettings-logs") + end + private def applied_filters_count(filter_type) @@ -126,7 +157,7 @@ private def filters_count(filters) filters.each.sum do |category, category_filters| - if %w[status years bulk_upload_id].include?(category) + if %w[years status needstypes bulk_upload_id].include?(category) category_filters.count(&:present?) elsif %w[user owning_organisation managing_organisation].include?(category) 1 diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index d9712ea52..1bcca5496 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -43,6 +43,13 @@ class LettingsLog < Log scope :filter_by_tenant_code, ->(tenant_code) { where("tenancycode ILIKE ?", "%#{tenant_code}%") } scope :filter_by_propcode, ->(propcode) { where("propcode ILIKE ?", "%#{propcode}%") } scope :filter_by_location_postcode, ->(postcode_full) { left_joins(:location).where("REPLACE(locations.postcode, ' ', '') ILIKE ?", "%#{postcode_full.delete(' ')}%") } + scope :filter_by_needstype, ->(needstype) { where(needstype:) } + scope :filter_by_needstypes, lambda { |needstypes, _user = nil| + first_needstype = needstypes.shift + query = filter_by_needstype(first_needstype) + needstypes.each { |needstype| query = query.or(filter_by_needstype(needstype)) } + query.all + } scope :search_by, lambda { |param| filter_by_location_postcode(param) .or(filter_by_tenant_code(param)) diff --git a/app/models/user.rb b/app/models/user.rb index c6deed8a1..fc26dc686 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -173,9 +173,9 @@ class User < ApplicationRecord def logs_filters(specific_org: false) if (support? && !specific_org) || organisation.has_managing_agents? || organisation.has_stock_owners? - %w[status years assigned_to user managing_organisation owning_organisation bulk_upload_id] + %w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id] else - %w[status years assigned_to user bulk_upload_id] + %w[years status needstypes assigned_to user bulk_upload_id] end end diff --git a/app/views/logs/_log_filters.html.erb b/app/views/logs/_log_filters.html.erb index 1034379db..589fb40ba 100644 --- a/app/views/logs/_log_filters.html.erb +++ b/app/views/logs/_log_filters.html.erb @@ -41,29 +41,39 @@ label: "Status", category: "status", } %> + + <% if logs_for_both_needstypes_present?(@organisation) && user_or_org_lettings_path? %> + <%= render partial: "filters/checkbox_filter", + locals: { + f:, + options: needstype_filters, + label: "Needs type", + category: "needstypes", + } %> + <% end %> <% end %> - <%= render partial: "filters/radio_filter", - locals: { - f:, - options: { - "all": { label: "Any user" }, - "you": { label: "You" }, - "specific_user": { - label: "Specific user", - conditional_filter: { - type: "select", - label: "User", - category: "user", - options: assigned_to_filter_options(current_user), - }, + <%= render partial: "filters/radio_filter", + locals: { + f:, + options: { + "all": { label: "Any user" }, + "you": { label: "You" }, + "specific_user": { + label: "Specific user", + conditional_filter: { + type: "select", + label: "User", + category: "user", + options: assigned_to_filter_options(current_user), }, }, - label: "Assigned to", - category: "assigned_to", - } %> + }, + label: "Assigned to", + category: "assigned_to", + } %> - <% if current_user.support? || current_user.organisation.stock_owners.count > 1 && request.path == "/lettings-logs" %> + <% if current_user.support? || non_support_with_multiple_owning_orgs? %> <%= render partial: "filters/radio_filter", locals: { f:, options: { @@ -83,7 +93,7 @@ } %> <% end %> - <% if (current_user.support? || current_user.organisation.managing_agents.count > 1) && request.path == "/lettings-logs" %> + <% if (current_user.support? || non_support_with_multiple_managing_orgs?) && user_or_org_lettings_path? %> <%= render partial: "filters/radio_filter", locals: { f:, options: { diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 6b4efb80c..2ff74ac9d 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -126,7 +126,8 @@ RSpec.describe "User Features" do context "when viewing lettings logs for specific organisation" do let(:first_log) { organisation.lettings_logs.first } let!(:log_to_search) { FactoryBot.create(:lettings_log, created_by: user) } - let!(:other_logs) { FactoryBot.create_list(:lettings_log, 4, created_by: user) } + let!(:other_general_needs_logs) { FactoryBot.create_list(:lettings_log, 4, created_by: user, needstype: 1) } + let!(:other_supported_housing_logs) { FactoryBot.create_list(:lettings_log, 4, created_by: user, needstype: 2) } let(:number_of_lettings_logs) { LettingsLog.count } before do @@ -148,7 +149,7 @@ RSpec.describe "User Features" do context "when searching for specific logs" do it "displays the logs belonging to the same organisation" do expect(page).to have_content(log_to_search.id) - other_logs.each do |log| + (other_general_needs_logs + other_supported_housing_logs).each do |log| expect(page).to have_content(log.id) end end @@ -168,7 +169,7 @@ RSpec.describe "User Features" do it "displays log matching the log ID" do expect(page).to have_link(log_to_search.id.to_s) - other_logs.each do |log| + (other_general_needs_logs + other_supported_housing_logs).each do |log| expect(page).not_to have_link(log.id.to_s) end end @@ -187,16 +188,31 @@ RSpec.describe "User Features" do end end - it "can filter lettings logs" do + it "has correct page details" do expect(page).to have_content("#{number_of_lettings_logs} total logs") organisation.lettings_logs.map(&:id).each do |lettings_log_id| expect(page).to have_link lettings_log_id.to_s, href: "/lettings-logs/#{lettings_log_id}" end + end + + it "can filter lettings logs by year" do check("years-2021-field") click_button("Apply filters") - expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2021&status[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=") + expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2021&status[]=&needstypes[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=&managing_organisation_select=all&managing_organisation=") expect(page).not_to have_link first_log.id.to_s, href: "/lettings-logs/#{first_log.id}" end + + it "can filter lettings logs by needstype" do + check("needstypes-1-field") + click_button("Apply filters") + expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&status[]=&needstypes[]=&needstypes[]=1&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=&managing_organisation_select=all&managing_organisation=") + other_general_needs_logs.each do |general_needs_log| + expect(page).to have_link general_needs_log.id.to_s, href: "/lettings-logs/#{general_needs_log.id}" + end + other_supported_housing_logs.each do |supported_housing_log| + expect(page).not_to have_link supported_housing_log.id.to_s, href: "/lettings-logs/#{supported_housing_log.id}" + end + end end context "when viewing sales logs for specific organisation" do diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index b3ee773d1..d657ea3dc 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -756,10 +756,10 @@ RSpec.describe "User Features" do expect(page).to have_field("owning-organisation-field", with: "") find("#owning-organisation-field").click.native.send_keys("F", "i", "l", "t", :down, :enter) click_button("Apply filters") - expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&assigned_to=all&owning_organisation_select=specific_org&owning_organisation=#{parent_organisation.id}&managing_organisation_select=all") + expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&%5Bneedstypes%5D%5B%5D=&assigned_to=all&owning_organisation_select=specific_org&owning_organisation=#{parent_organisation.id}&managing_organisation_select=all") choose("owning-organisation-select-all-field", allow_label_click: true) click_button("Apply filters") - expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&assigned_to=all&owning_organisation_select=all&managing_organisation_select=all") + expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&%5Bneedstypes%5D%5B%5D=&assigned_to=all&owning_organisation_select=all&managing_organisation_select=all") end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 63ea72dca..c272320b2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -163,7 +163,7 @@ RSpec.describe User, type: :model do end it "can filter lettings logs by user, year and status" do - expect(user.logs_filters).to eq(%w[status years assigned_to user bulk_upload_id]) + expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user bulk_upload_id]) end end @@ -173,7 +173,7 @@ RSpec.describe User, type: :model do end it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do - expect(user.logs_filters).to match_array(%w[status years assigned_to user managing_organisation owning_organisation bulk_upload_id]) + expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id]) end end end @@ -214,7 +214,7 @@ RSpec.describe User, type: :model do end it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do - expect(user.logs_filters).to match_array(%w[status years assigned_to user owning_organisation managing_organisation bulk_upload_id]) + expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user owning_organisation managing_organisation bulk_upload_id]) end end