From d251f66820cdc1591af771e0b8f99855e91e1d15 Mon Sep 17 00:00:00 2001 From: Kat Date: Mon, 17 Jul 2023 15:08:18 +0100 Subject: [PATCH] Remove organisation filter --- app/helpers/filters_helper.rb | 7 ---- app/models/user.rb | 4 +-- app/services/filter_manager.rb | 2 -- app/views/logs/_log_filters.html.erb | 20 ----------- spec/features/user_spec.rb | 18 +++++----- spec/helpers/filters_helper_spec.rb | 36 +++++++++---------- spec/models/user_spec.rb | 10 +++--- .../requests/lettings_logs_controller_spec.rb | 19 ---------- spec/requests/sales_logs_controller_spec.rb | 6 ---- 9 files changed, 35 insertions(+), 87 deletions(-) diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index f63b80b2e..ae9d6e895 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -4,9 +4,7 @@ module FiltersHelper selected_filters = JSON.parse(session[session_name_for(filter_type)]) return true if selected_filters.blank? && filter == "user" && value == :all - return true if !selected_filters.key?("organisation") && filter == "organisation_select" && value == :all return true if !selected_filters.key?("owning_organisation") && filter == "owning_organisation_select" && value == :all - return true if selected_filters["organisation"].present? && filter == "organisation_select" && value == :specific_org return true if selected_filters["owning_organisation"].present? && filter == "owning_organisation_select" && value == :specific_org return false if selected_filters[filter].blank? @@ -39,11 +37,6 @@ module FiltersHelper JSON.parse(session[session_name_for(filter_type)])[filter] || "" end - def organisations_filter_options(user) - organisation_options = user.support? ? Organisation.all : [user.organisation] + user.organisation.managing_agents - [OpenStruct.new(id: "", name: "Select an option")] + organisation_options.map { |org| OpenStruct.new(id: org.id, name: org.name) } - end - def owning_organisations_filter_options(user) organisation_options = user.support? ? Organisation.all : [user.organisation] + user.organisation.stock_owners [OpenStruct.new(id: "", name: "Select an option")] + organisation_options.map { |org| OpenStruct.new(id: org.id, name: org.name) } diff --git a/app/models/user.rb b/app/models/user.rb index c0305b52a..14b71be64 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -143,8 +143,8 @@ class User < ApplicationRecord end def logs_filters(specific_org: false) - if (support? && !specific_org) || organisation.has_managing_agents? || organisation.has_stock_owners? - %w[status years user organisation owning_organisation bulk_upload_id] + if (support? && !specific_org) || organisation.has_stock_owners? + %w[status years user owning_organisation bulk_upload_id] else %w[status years user bulk_upload_id] end diff --git a/app/services/filter_manager.rb b/app/services/filter_manager.rb index 9a950ca93..005201031 100644 --- a/app/services/filter_manager.rb +++ b/app/services/filter_manager.rb @@ -21,7 +21,6 @@ class FilterManager filters.each do |category, values| next if Array(values).reject(&:empty?).blank? - next if category == "organisation" && all_orgs next if category == "owning_organisation" && all_orgs logs = logs.public_send("filter_by_#{category}", values, user) @@ -54,7 +53,6 @@ class FilterManager new_filters[filter] = params[filter] if params[filter].present? end end - new_filters = new_filters.except("organisation") if params["organisation_select"] == "all" new_filters = new_filters.except("owning_organisation") if params["owning_organisation_select"] == "all" new_filters end diff --git a/app/views/logs/_log_filters.html.erb b/app/views/logs/_log_filters.html.erb index cf83194f2..90e7f5ea7 100644 --- a/app/views/logs/_log_filters.html.erb +++ b/app/views/logs/_log_filters.html.erb @@ -64,26 +64,6 @@ } %> <% end %> - <% if (@current_user.support? || @current_user.organisation.has_managing_agents?) && request.path == "/lettings-logs" %> - <%= render partial: "filters/radio_filter", locals: { - f:, - options: { - "all": { label: "All" }, - "specific_org": { - label: "Specific organisation", - conditional_filter: { - type: "select", - label: "Organisation", - category: "organisation", - options: organisations_filter_options(@current_user), - }, - }, - }, - label: "Organisation", - category: "organisation_select", - } %> - <% end %> - <%= f.govuk_submit "Apply filters", class: "govuk-!-margin-bottom-0" %> <% end %> diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 777e6c287..e7cff9f43 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -708,10 +708,12 @@ RSpec.describe "User Features" do end context "when viewing logs" do - context "when filtering by organisation and then switching back to all organisations", js: true do - let!(:organisation) { FactoryBot.create(:organisation, name: "Filtered Org") } + context "when filtering by owning organisation and then switching back to all organisations", js: true do + let!(:organisation) { FactoryBot.create(:organisation) } + let(:parent_organisation) { FactoryBot.create(:organisation, name: "Filtered Org") } before do + create(:organisation_relationship, child_organisation: organisation, parent_organisation:) allow(SecureRandom).to receive(:random_number).and_return(otp) click_button("Sign in") fill_in("code", with: otp) @@ -720,14 +722,14 @@ RSpec.describe "User Features" do it "clears the previously selected organisation value" do visit("/lettings-logs") - choose("organisation-select-specific-org-field", allow_label_click: true) - expect(page).to have_field("organisation-field", with: "") - find("#organisation-field").click.native.send_keys("F", "i", "l", "t", :down, :enter) + choose("owning-organisation-select-specific-org-field", allow_label_click: true) + 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=&user=all&owning_organisation_select=all&organisation_select=specific_org&organisation=#{organisation.id}") - choose("organisation-select-all-field", allow_label_click: true) + expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&owning_organisation_select=specific_org&owning_organisation=#{parent_organisation.id}") + 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=&user=all&owning_organisation_select=all&organisation_select=all") + expect(page).to have_current_path("/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&owning_organisation_select=all") end end end diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb index 622fd71f8..7c43462c7 100644 --- a/spec/helpers/filters_helper_spec.rb +++ b/spec/helpers/filters_helper_spec.rb @@ -38,34 +38,34 @@ RSpec.describe FiltersHelper do context "when support user is using the organisation filter" do before do - session[:lettings_logs_filters] = { "organisation": "1" }.to_json + session[:lettings_logs_filters] = { "owning_organisation": "1" }.to_json end - it "returns true for the parent organisation_select filter" do - expect(filter_selected?("organisation_select", :specific_org, "lettings_logs")).to be true - expect(filter_selected?("organisation_select", :all, "lettings_logs")).to be false + it "returns true for the parent owning_organisation_select filter" do + expect(filter_selected?("owning_organisation_select", :specific_org, "lettings_logs")).to be true + expect(filter_selected?("owning_organisation_select", :all, "lettings_logs")).to be false end end - context "when support user has not set the organisation_select filter" do + context "when support user has not set the owning_organisation_select filter" do before do session[:lettings_logs_filters] = {}.to_json end it "defaults to all organisations" do - expect(filter_selected?("organisation_select", :all, "lettings_logs")).to be true - expect(filter_selected?("organisation_select", :specific_org, "lettings_logs")).to be false + expect(filter_selected?("owning_organisation_select", :all, "lettings_logs")).to be true + expect(filter_selected?("owning_organisation_select", :specific_org, "lettings_logs")).to be false end end - context "when the specific organisation filter is not set" do + context "when the specific owning organisation filter is not set" do before do session[:lettings_logs_filters] = { "status" => [""], "years" => [""], "user" => "all" }.to_json end it "marks the all options as checked" do - expect(filter_selected?("organisation_select", :all, "lettings_logs")).to be true - expect(filter_selected?("organisation_select", :specific_org, "lettings_logs")).to be false + expect(filter_selected?("owning_organisation_select", :all, "lettings_logs")).to be true + expect(filter_selected?("owning_organisation_select", :specific_org, "lettings_logs")).to be false end end end @@ -85,7 +85,7 @@ RSpec.describe FiltersHelper do end context "when organisation and user are set to all" do - let(:filters) { { "organisation_select" => "all", "user" => "all" } } + let(:filters) { { "owning_organisation_select" => "all", "user" => "all" } } it "returns false" do expect(result).to be_falsey @@ -160,7 +160,7 @@ RSpec.describe FiltersHelper do end end - describe "#organisations_filter_options" do + describe "#owning_organisations_filter_options" do let(:parent_organisation) { FactoryBot.create(:organisation, name: "Parent organisation") } let(:child_organisation) { FactoryBot.create(:organisation, name: "Child organisation") } @@ -170,10 +170,10 @@ RSpec.describe FiltersHelper do end context "with a support user" do - let(:user) { FactoryBot.create(:user, :support, organisation: parent_organisation) } + let(:user) { FactoryBot.create(:user, :support, organisation: child_organisation) } it "returns a list of all organisations" do - expect(organisations_filter_options(user)).to eq([ + expect(owning_organisations_filter_options(user)).to eq([ OpenStruct.new(id: "", name: "Select an option"), OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), OpenStruct.new(id: child_organisation.id, name: "Child organisation"), @@ -183,13 +183,13 @@ RSpec.describe FiltersHelper do end context "with a data coordinator user" do - let(:user) { FactoryBot.create(:user, :data_coordinator, organisation: parent_organisation) } + let(:user) { FactoryBot.create(:user, :data_coordinator, organisation: child_organisation) } - it "returns a list of managing agents and your own organisation" do - expect(organisations_filter_options(user)).to eq([ + it "returns a list of paret orgs and your own organisation" do + expect(owning_organisations_filter_options(user)).to eq([ OpenStruct.new(id: "", name: "Select an option"), - OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), OpenStruct.new(id: child_organisation.id, name: "Child organisation"), + OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), ]) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ef2045753..dae075b06 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -148,11 +148,11 @@ RSpec.describe User, type: :model do context "and their organisation has managing agents" do before do - create(:organisation_relationship, parent_organisation: user.organisation) + create(:organisation_relationship, child_organisation: user.organisation) end - it "can filter lettings logs by user, owning_organisation, year, status and organisation" do - expect(user.logs_filters).to eq(%w[status years user organisation owning_organisation bulk_upload_id]) + it "can filter lettings logs by user, owning_organisation, year and status" do + expect(user.logs_filters).to eq(%w[status years user owning_organisation bulk_upload_id]) end end end @@ -192,8 +192,8 @@ RSpec.describe User, type: :model do }) end - it "can filter lettings logs by user, owning_organisation, year, status and organisation" do - expect(user.logs_filters).to eq(%w[status years user organisation owning_organisation bulk_upload_id]) + it "can filter lettings logs by user, owning_organisation, year and status" do + expect(user.logs_filters).to eq(%w[status years user owning_organisation bulk_upload_id]) end end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index a6e05b94f..5b4c5e3b2 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -354,12 +354,6 @@ RSpec.describe LettingsLogsController, type: :request do expect(page).not_to have_link(completed_lettings_log.id.to_s) end - it "filters on organisation" do - get "/lettings-logs?organisation[]=#{organisation_2.id}", headers:, params: {} - expect(page).to have_link(completed_lettings_log.id.to_s) - expect(page).not_to have_link(in_progress_lettings_log.id.to_s) - end - it "filters on owning organisation" do get "/lettings-logs?owning_organisation[]=#{organisation_2.id}", headers:, params: {} expect(page).to have_link(completed_lettings_log.id.to_s) @@ -784,25 +778,12 @@ RSpec.describe LettingsLogsController, type: :request do sign_in user end - it "does show the organisation filter" do - get "/lettings-logs", headers:, params: {} - expect(page).to have_field("organisation-field") - end - it "shows all logs by default" do get "/lettings-logs", headers:, params: {} expect(page).to have_content(tenant_code_1) expect(page).to have_content(tenant_code_2) end - context "when filtering by organisation" do - it "only show the selected organisations logs" do - get "/lettings-logs?organisation_select=specific_org&organisation=#{org_1.id}", headers:, params: {} - expect(page).to have_content(tenant_code_1) - expect(page).not_to have_content(tenant_code_2) - end - end - context "when the support user has filtered by organisation, then switches back to all organisations" do it "shows all organisations" do get "http://localhost:3000/lettings-logs?%5Byears%5D%5B%5D=&%5Bstatus%5D%5B%5D=&user=all&organisation_select=all&organisation=#{org_1.id}", headers:, params: {} diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 5d24e6b01..218e8b5c4 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -250,12 +250,6 @@ RSpec.describe SalesLogsController, type: :request do expect(page).not_to have_link(completed_sales_log.id.to_s) end - it "filters on organisation" do - get "/sales-logs?organisation[]=#{organisation_2.id}", headers: headers, params: {} - expect(page).to have_link(completed_sales_log.id.to_s) - expect(page).not_to have_link(not_started_sales_log.id.to_s) - end - it "does not reset the filters" do get "/sales-logs?status[]=not_started", headers: headers, params: {} expect(page).to have_link(not_started_sales_log.id.to_s)