CLDC-2108 Update sales owning organisation dropdown (#1821)

* Display absorbed orgs/absorbed org stock owners

* Only display orgs merged during open collection period

* split guard clauses back

* Do not show stock owners in sales logs owning org options

* Update routed to method

* Display active dates for support

app/models/form/sales/pages/organisation.rb

@@ -10,24 +10,17 @@ class Form::Sales::Pages::Organisation < ::Form::Page
     ]
   end
 
-  def routed_to?(log, current_user)
+  def routed_to?(_log, current_user)
     return false unless current_user
     return true if current_user.support?
 
-    stock_owners = current_user.organisation.stock_owners + current_user.organisation.absorbed_organisations.where(holds_own_stock: true)
+    absorbed_stock_owners = current_user.organisation.absorbed_organisations.where(holds_own_stock: true)
 
     if current_user.organisation.holds_own_stock?
-      if current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?)
+      return true if absorbed_stock_owners.count >= 1
-        return true
-      end
-      return true if stock_owners.count >= 1
-
-      log.update!(owning_organisation: current_user.organisation)
     else
-      return false if stock_owners.count.zero?
+      return false if absorbed_stock_owners.count.zero?
-      return true if stock_owners.count > 1
+      return true if absorbed_stock_owners.count > 1
-
-      log.update!(owning_organisation: stock_owners.first)
     end
 
     false

app/models/form/sales/questions/owning_organisation_id.rb

@@ -15,20 +15,35 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
     return answer_opts unless log
 
     if log.owning_organisation_id.present?
-      answer_opts = answer_opts.merge({ log.owning_organisation.id => log.owning_organisation.name })
+      answer_opts[log.owning_organisation.id] = log.owning_organisation.name
     end
 
+    recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period
     if !user.support? && user.organisation.holds_own_stock?
-      answer_opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)"
+      answer_opts[user.organisation.id] = if recently_absorbed_organisations.exists?
+                                            "#{user.organisation.name} (Your organisation, active as of #{user.organisation.created_at.to_fs(:govuk_date)})"
+                                          else
+                                            "#{user.organisation.name} (Your organisation)"
+                                          end
     end
 
-    user_answer_options = if user.support?
+    if user.support?
-                            Organisation.where(holds_own_stock: true)
+      Organisation.where(holds_own_stock: true).find_each do |org|
-                          else
+        if org.merge_date.present?
-                            user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true)
+          answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
-                          end.pluck(:id, :name).to_h
+        elsif org.absorbed_organisations.merged_during_open_collection_period.exists?
+          answer_opts[org.id] = "#{org.name} (active as of #{org.created_at.to_fs(:govuk_date)})"
+        else
+          answer_opts[org.id] = org.name
+        end
+      end
+    else
+      recently_absorbed_organisations.each do |absorbed_org|
+        answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock?
+      end
+    end
 
-    answer_opts.merge(user_answer_options)
+    answer_opts
   end
 
   def displayed_answer_options(log, user = nil)
@@ -66,4 +81,8 @@ private
   def selected_answer_option_is_derived?(_log)
     true
   end
+
+  def merged_organisation_label(name, merge_date)
+    "#{name} (inactive as of #{merge_date.to_fs(:govuk_date)})"
+  end
 end

app/models/organisation.rb

@@ -32,6 +32,7 @@ class Organisation < ApplicationRecord
 
   scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") }
   scope :search_by, ->(param) { search_by_name(param) }
+  scope :merged_during_open_collection_period, -> { where("merge_date >= ?", FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period) }
 
   has_paper_trail
 

spec/models/form/sales/pages/organisation_spec.rb

@@ -88,8 +88,8 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do
             expect(page.routed_to?(log, user)).to eq(false)
           end
 
-          it "updates owning_organisation_id" do
+          it "does not update owning_organisation_id" do
-            expect { page.routed_to?(log, user) }.to change(log.reload, :owning_organisation).from(nil).to(stock_owner)
+            expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation)
           end
         end
 
@@ -111,11 +111,7 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do
           end
 
           it "is not shown" do
-            expect(page.routed_to?(log, user)).to eq(true)
+            expect(page.routed_to?(log, user)).to eq(false)
-          end
-
-          it "updates owning_organisation_id" do
-            expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation)
           end
         end
       end
@@ -129,12 +125,6 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do
           it "is not shown" do
             expect(page.routed_to?(log, user)).to eq(false)
           end
-
-          it "updates owning_organisation_id to user organisation" do
-            expect {
-              page.routed_to?(log, user)
-            }.to change(log.reload, :owning_organisation).from(nil).to(user.organisation)
-          end
         end
 
         context "with >0 stock_owners" do
@@ -143,12 +133,20 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do
             create(:organisation_relationship, child_organisation: user.organisation)
           end
 
-          it "is shown" do
+          it "is not shown" do
-            expect(page.routed_to?(log, user)).to eq(true)
+            expect(page.routed_to?(log, user)).to eq(false)
           end
+        end
 
-          it "does not update owning_organisation_id" do
+        context "with merged organisations" do
-            expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil)
+          let(:merged_org) { create(:organisation) }
+
+          before do
+            merged_org.update!(absorbing_organisation: user.organisation, merge_date: Time.zone.now)
+          end
+
+          it "is shown" do
+            expect(page.routed_to?(log, user)).to eq(true)
           end
         end
       end

spec/models/form/sales/questions/owning_organisation_id_spec.rb

@@ -70,53 +70,99 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do
             "" => "Select an option",
             owning_org_1.id => "Owning org 1",
             user.organisation.id => "User org (Your organisation)",
-            owning_org_2.id => "Owning org 2",
           }
         end
 
-        it "shows current stock owner at top, followed by user's org (with hint), followed by the stock owners of the user's org" do
+        it "does not show stock owner" do
           user.organisation.update!(holds_own_stock: true)
           expect(question.displayed_answer_options(log, user)).to eq(options)
         end
+      end
 
-        context "when the owning-managing organisation relationship is deleted" do
+      context "when user's org doesn't own stock" do
-          let(:options) do
+        let(:options) do
-            {
+          {
-              "" => "Select an option",
+            "" => "Select an option",
-              user.organisation.id => "User org (Your organisation)",
+            owning_org_1.id => "Owning org 1",
-              owning_org_2.id => "Owning org 2",
+          }
-            }
+        end
-          end
 
-          it "doesn't remove the housing provider from the list of allowed housing providers" do
+        it "shows current log owner" do
-            log.update!(owning_organisation: owning_org_2)
+          user.organisation.update!(holds_own_stock: false)
-            expect(question.displayed_answer_options(log, user)).to eq(options)
+          expect(question.displayed_answer_options(log, user)).to eq(options)
-            org_rel.destroy!
-            expect(question.displayed_answer_options(log, user)).to eq(options)
-          end
         end
       end
 
-      context "when user's org doesn't own stock" do
+      context "when user's org has recently absorbed other orgs" do
+        let(:merged_organisation) { create(:organisation, name: "Merged org") }
         let(:options) do
           {
             "" => "Select an option",
+            user.organisation.id => "User org (Your organisation, active as of 2 February 2021)",
             owning_org_1.id => "Owning org 1",
-            owning_org_2.id => "Owning org 2",
+            merged_organisation.id => "Merged org (inactive as of 2 February 2023)",
           }
         end
 
-        it "shows current stock owner at top, followed by the stock owners of the user's org" do
+        before do
-          user.organisation.update!(holds_own_stock: false)
+          merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation)
+          user.organisation.update!(created_at: Time.zone.local(2021, 2, 2))
+        end
+
+        it "shows merged organisation as an option" do
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
+      end
+
+      context "when user's org has recently absorbed other orgs with parent organisations" do
+        let(:merged_organisation) { create(:organisation, name: "Merged org") }
+        let(:options) do
+          {
+            "" => "Select an option",
+            user.organisation.id => "User org (Your organisation, active as of 2 February 2021)",
+            owning_org_1.id => "Owning org 1",
+            merged_organisation.id => "Merged org (inactive as of 2 February 2023)",
+          }
+        end
+
+        before do
+          org_rel.update!(child_organisation: merged_organisation)
+          merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation)
+          user.organisation.update!(created_at: Time.zone.local(2021, 2, 2))
+        end
+
+        it "does not show merged organisations stock owners as options" do
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
+      end
+
+      context "when user's org has absorbed other orgs with parent organisations during closed collection periods" do
+        let(:merged_organisation) { create(:organisation, name: "Merged org") }
+        let(:options) do
+          {
+            "" => "Select an option",
+            user.organisation.id => "User org (Your organisation)",
+            owning_org_1.id => "Owning org 1",
+          }
+        end
+
+        before do
+          Timecop.freeze(Time.zone.local(2023, 4, 2))
+          org_rel.update!(child_organisation: merged_organisation)
+          merged_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation)
+          user.organisation.update!(created_at: Time.zone.local(2021, 2, 2))
+        end
+
+        it "shows merged organisation as an option" do
           expect(question.displayed_answer_options(log, user)).to eq(options)
         end
       end
     end
 
     context "when user is support" do
-      let(:user) { create(:user, :support) }
+      let(:user) { create(:user, :support, organisation: organisation_1) }
 
-      let(:log) { create(:lettings_log) }
+      let(:log) { create(:lettings_log, created_by: user) }
 
       let(:non_stock_organisation) { create(:organisation, holds_own_stock: false) }
       let(:expected_opts) do
@@ -130,6 +176,48 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do
         expect(question.displayed_answer_options(log, user)).to eq(expected_opts)
         expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id)
       end
+
+      context "when an org has recently absorbed other orgs" do
+        let(:merged_organisation) { create(:organisation, name: "Merged org") }
+        let(:options) do
+          {
+            "" => "Select an option",
+            organisation_1.id => "first test org (active as of 2 February 2021)",
+            organisation_2.id => "second test org",
+            merged_organisation.id => "Merged org (inactive as of 2 February 2023)",
+          }
+        end
+
+        before do
+          merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: organisation_1)
+          organisation_1.update!(created_at: Time.zone.local(2021, 2, 2))
+        end
+
+        it "shows merged organisation as an option" do
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
+      end
+
+      context "when user's org has absorbed other orgs during closed collection periods" do
+        let(:merged_organisation) { create(:organisation, name: "Merged org") }
+        let(:options) do
+          {
+            "" => "Select an option",
+            organisation_1.id => "first test org",
+            organisation_2.id => "second test org",
+          }
+        end
+
+        before do
+          Timecop.freeze(Time.zone.local(2023, 4, 2))
+          merged_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation)
+          user.organisation.update!(created_at: Time.zone.local(2021, 2, 2))
+        end
+
+        it "shows merged organisation as an option" do
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
+      end
     end
   end