CLDC-2243 Add owning organisation filter (#1772)

* Add owning organisation filter

* fix tests

* Update filter content

* Remove organisation filter

app/helpers/filters_helper.rb

@@ -4,8 +4,8 @@ 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?
 
     selected_filters[filter].include?(value.to_s)
@@ -37,8 +37,8 @@ module FiltersHelper
     JSON.parse(session[session_name_for(filter_type)])[filter] || ""
   end
 
-  def organisations_filter_options(user)
+  def owning_organisations_filter_options(user)
-    organisation_options = user.support? ? Organisation.all : [user.organisation] + user.organisation.managing_agents
+    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) }
   end
 

app/models/lettings_log.rb

@@ -53,6 +53,7 @@ class LettingsLog < Log
   scope :filter_by_before_startdate, ->(date) { where("lettings_logs.startdate >= ?", date) }
   scope :unresolved, -> { where(unresolved: true) }
   scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) }
+  scope :filter_by_owning_organisation, ->(org, _user = nil) { where(owning_organisation: org) }
   scope :duplicate_logs, lambda { |log|
     visible
       .where(log.slice(*DUPLICATE_LOG_ATTRIBUTES))

app/models/organisation.rb

@@ -118,4 +118,8 @@ class Organisation < ApplicationRecord
   def has_managing_agents?
     managing_agents.count.positive?
   end
+
+  def has_stock_owners?
+    stock_owners.count.positive?
+  end
 end

app/models/sales_log.rb

@@ -41,6 +41,7 @@ class SalesLog < Log
       .or(filter_by_id(param))
   }
   scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org) }
+  scope :filter_by_owning_organisation, ->(org, _user = nil) { where(owning_organisation: org) }
   scope :duplicate_logs, lambda { |log|
     visible.where(log.slice(*DUPLICATE_LOG_ATTRIBUTES))
     .where.not(id: log.id)

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?
+    if (support? && !specific_org) || organisation.has_stock_owners?
-      %w[status years user organisation bulk_upload_id]
+      %w[status years user owning_organisation bulk_upload_id]
     else
       %w[status years user bulk_upload_id]
     end

app/services/filter_manager.rb

@@ -21,7 +21,7 @@ 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)
     end
@@ -53,11 +53,12 @@ class FilterManager
         new_filters[filter] = params[filter] if params[filter].present?
       end
     end
-    params["organisation_select"] == "all" ? new_filters.except("organisation") : new_filters
+    new_filters = new_filters.except("owning_organisation") if params["owning_organisation_select"] == "all"
+    new_filters
   end
 
   def filtered_logs(logs, search_term, filters)
-    all_orgs = params["organisation_select"] == "all"
+    all_orgs = params["organisation_select"] == "all" && params["owning_organisation_select"] == "all"
     FilterManager.filter_logs(logs, search_term, filters, all_orgs, current_user)
   end
 

app/views/logs/_log_filters.html.erb

@@ -52,23 +52,23 @@
                      category: "user",
                    } %>
 
-        <% if (@current_user.support? || @current_user.organisation.has_managing_agents?) && request.path == "/lettings-logs" %>
+         <% if @current_user.support? || @current_user.organisation.stock_owners.count > 1 && request.path == "/lettings-logs" %>
           <%= render partial: "filters/radio_filter", locals: {
             f:,
             options: {
-              "all": { label: "All" },
+              "all": { label: "Any owning organisation" },
               "specific_org": {
-                label: "Specific organisation",
+                label: "Specific owning organisation",
                 conditional_filter: {
                   type: "select",
-                  label: "Organisation",
+                  label: "Owning Organisation",
-                  category: "organisation",
+                  category: "owning_organisation",
-                  options: organisations_filter_options(@current_user),
+                  options: owning_organisations_filter_options(@current_user),
                 },
               },
             },
-            label: "Organisation",
+            label: "Owned by",
-            category: "organisation_select",
+            category: "owning_organisation_select",
           } %>
         <% end %>
 

spec/features/organisation_spec.rb

@@ -194,7 +194,7 @@ RSpec.describe "User Features" do
         end
         check("years-2021-field")
         click_button("Apply filters")
-        expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2021&status[]=&user=all")
+        expect(page).to have_current_path("/organisations/#{org_id}/lettings-logs?years[]=&years[]=2021&status[]=&user=all&owning_organisation_select=all&owning_organisation=")
         expect(page).not_to have_link first_log.id.to_s, href: "/lettings-logs/#{first_log.id}"
       end
     end
@@ -227,7 +227,7 @@ RSpec.describe "User Features" do
         end
         check("years-2021-field")
         click_button("Apply filters")
-        expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2021&status[]=&user=all")
+        expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2021&status[]=&user=all&owning_organisation_select=all&owning_organisation=")
         expect(page).not_to have_link first_log.id.to_s, href: "/sales-logs/#{first_log.id}"
       end
     end

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
+      context "when filtering by owning organisation and then switching back to all organisations", js: true do
-        let!(:organisation) { FactoryBot.create(:organisation, name: "Filtered Org") }
+        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)
+          choose("owning-organisation-select-specific-org-field", allow_label_click: true)
-          expect(page).to have_field("organisation-field", with: "")
+          expect(page).to have_field("owning-organisation-field", with: "")
-          find("#organisation-field").click.native.send_keys("F", "i", "l", "t", :down, :enter)
+          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&organisation_select=specific_org&organisation=#{organisation.id}")
+          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("organisation-select-all-field", allow_label_click: true)
+          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&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

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
+      it "returns true for the parent owning_organisation_select filter" do
-        expect(filter_selected?("organisation_select", :specific_org, "lettings_logs")).to be true
+        expect(filter_selected?("owning_organisation_select", :specific_org, "lettings_logs")).to be true
-        expect(filter_selected?("organisation_select", :all, "lettings_logs")).to be false
+        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?("owning_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", :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?("owning_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", :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
+      it "returns a list of paret orgs and your own organisation" 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"),
+          OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"),
         ])
       end
     end

spec/models/lettings_log_spec.rb

@@ -2769,6 +2769,25 @@ RSpec.describe LettingsLog do
       end
     end
 
+    context "when filtering by owning organisation" do
+      let(:organisation_1) { create(:organisation) }
+      let(:organisation_2) { create(:organisation) }
+      let(:organisation_3) { create(:organisation) }
+
+      before do
+        create(:lettings_log, :in_progress, owning_organisation: organisation_1, managing_organisation: organisation_1, created_by: nil)
+        create(:lettings_log, :completed, owning_organisation: organisation_1, managing_organisation: organisation_2, created_by: nil)
+        create(:lettings_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_1, created_by: nil)
+        create(:lettings_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_2, created_by: nil)
+      end
+
+      it "filters by given owning organisation" do
+        expect(described_class.filter_by_owning_organisation([organisation_1]).count).to eq(2)
+        expect(described_class.filter_by_owning_organisation([organisation_1, organisation_2]).count).to eq(4)
+        expect(described_class.filter_by_owning_organisation([organisation_3]).count).to eq(0)
+      end
+    end
+
     context "when filtering on status" do
       it "allows filtering on a single status" do
         expect(described_class.filter_by_status(%w[in_progress]).count).to eq(2)

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, year, status and organisation" do
+        it "can filter lettings logs by user, owning_organisation, year and status" do
-          expect(user.logs_filters).to eq(%w[status years user organisation bulk_upload_id])
+          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, year, status and organisation" do
+      it "can filter lettings logs by user, owning_organisation, year and status" do
-        expect(user.logs_filters).to eq(%w[status years user organisation bulk_upload_id])
+        expect(user.logs_filters).to eq(%w[status years user owning_organisation bulk_upload_id])
       end
     end
 

spec/requests/lettings_logs_controller_spec.rb

@@ -354,12 +354,18 @@ RSpec.describe LettingsLogsController, type: :request do
               expect(page).not_to have_link(completed_lettings_log.id.to_s)
             end
 
-            it "filters on organisation" do
+            it "filters on owning organisation" do
-              get "/lettings-logs?organisation[]=#{organisation_2.id}", headers:, params: {}
+              get "/lettings-logs?owning_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 "filtering on owning organisation does not return managed orgs" do
+              get "/lettings-logs?owning_organisation[]=#{organisation.id}", headers:, params: {}
+              expect(page).not_to have_link(completed_lettings_log.id.to_s)
+              expect(page).to have_link(in_progress_lettings_log.id.to_s)
+            end
+
             it "does not reset the filters" do
               get "/lettings-logs?status[]=in_progress", headers:, params: {}
               expect(page).to have_link(in_progress_lettings_log.id.to_s)
@@ -780,25 +786,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: {}

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)