CLDC-1723 Overhaul letting log owning & managing org questions & tests (#1140)

* test: check managing org not gone from answer opts when relationship deleted

* feat: add current managing org to answer opts

* feat: check if managing org exists before trying to show it

* wip

* test: improve managing orgs opts test when not support

* test: improve managing orgs opts tests when support

* test: make relationship deletion test consistent with other tests

* test: add "(with hint)" to managing org opts test descriptions

* test: refactor managing orgs opts tests for support user case

* fix: don't call user in get_answer_label in CYA component

* style: reorder instance vars and remove old comments in managing_organisation.rb

* refactor: ensure label_from_value always accepts log & nil as args

* lint

* test: pass in log and user in housing provider opts test for support user

* test: update housing provider opts tests for non-support user

* feat: update housing provider answer opts to include current HP in db

* style: add space after user definition

* test: make context definition more human-readable

* test: refactor housing providers opts tests (not support user)

* test: check housing prov. still selectable after deleting relationship

* fix: define log and current_user instance vars in label_from_value (housing prov.)

* lint

* test: update lettings log feature tests

to differentiate between different numbers of stock owners when acting as a data coordinator

* test: check owning & managing orgs set correctly when a log is created

* test: add line breaks and start context descriptions with and (not if)

* test: artificially reference org_rel2 to avoid lint offense

* feat: don't set log owning org as user's org if that org doesn't hold stock

* test: improve test context descriptions in lettings_log_spec

* test: finish overhauling owning and managing org tests in lettings_log_spec

* test: change let! to let where possible in spec/features/lettings_log_spec.rb

* test: change let! to let where possible in spec/models/form/lettings/questions/managing_organisation_spec.rb

* test: change let! to let where possible in spec/models/form/lettings/questions/stock_owner_spec.rb

* test: remove if statement from "coordinator user's org doesn't hold stock" managing org test

* test: remove if statement from "coordinator user's org does hold stock" no managing orgs managing org test

* test: remove if statement from "coordinator user's org does hold stock" >=1 managing orgs managing org test

plus refactor previous test

* test: explicitly reference org rels in "coordinator user's org doesn't hold stock" managing org test

* test: don't create vars inside other vars (for tests edited/created in this branch)

* chore: save schema changes after migration

Co-authored-by: Phil Lee <asmega@users.noreply.github.com>

app/components/check_answers_summary_list_card_component.rb

@@ -13,7 +13,7 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base
   end
 
   def get_answer_label(question)
-    question.answer_label(log).presence || "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
+    question.answer_label(log, user).presence || "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
   end
 
   def check_answers_card_title(question)

app/controllers/logs_controller.rb

@@ -60,8 +60,9 @@ private
   end
 
   def org_params
+    owning_organisation_id = current_user.organisation.holds_own_stock? ? current_user.organisation.id : nil
     {
-      "owning_organisation_id" => current_user.organisation.id,
+      "owning_organisation_id" => owning_organisation_id,
       "managing_organisation_id" => current_user.organisation.id,
       "created_by_id" => current_user.id,
     }

app/helpers/check_answers_helper.rb

@@ -43,6 +43,6 @@ private
   end
 
   def get_answer_label(question, lettings_log)
-    question.answer_label(lettings_log).presence || "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
+    question.answer_label(lettings_log, current_user).presence || "<span class=\"app-!-colour-muted\">You didn’t answer this question</span>".html_safe
   end
 end

app/models/form/common/questions/created_by_id.rb

@@ -24,7 +24,7 @@ class Form::Common::Questions::CreatedById < ::Form::Question
     answer_options.select { |k, _v| user_ids.include?(k) }
   end
 
-  def label_from_value(value)
+  def label_from_value(value, _log = nil, _user = nil)
     return unless value
 
     answer_options[value]

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

@@ -21,7 +21,7 @@ class Form::Common::Questions::OwningOrganisationId < ::Form::Question
     answer_options
   end
 
-  def label_from_value(value)
+  def label_from_value(value, _log = nil, _user = nil)
     return unless value
 
     answer_options[value]

app/models/form/lettings/questions/created_by_id.rb

@@ -27,7 +27,7 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question
     answer_options.select { |k, _v| user_ids.include?(k) }
   end
 
-  def label_from_value(value)
+  def label_from_value(value, _log = nil, _user = nil)
     return unless value
 
     answer_options[value]

app/models/form/lettings/questions/managing_organisation.rb

@@ -12,10 +12,15 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question
 
   def answer_options
     opts = { "" => "Select an option" }
+
     return opts unless ActiveRecord::Base.connected?
     return opts unless current_user
     return opts unless log
 
+    if log.managing_organisation.present?
+      opts = opts.merge({ log.managing_organisation.id => log.managing_organisation.name })
+    end
+
     if current_user.support?
       if log.owning_organisation.holds_own_stock?
         opts[log.owning_organisation.id] = "#{log.owning_organisation.name} (Owning organisation)"
@@ -34,7 +39,10 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question
     answer_options
   end
 
-  def label_from_value(value)
+  def label_from_value(value, log = nil, user = nil)
+    @log = log
+    @current_user = user
+
     return unless value
 
     answer_options[value]

app/models/form/lettings/questions/stock_owner.rb

@@ -11,8 +11,14 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question
 
   def answer_options
     answer_opts = { "" => "Select an option" }
+
     return answer_opts unless ActiveRecord::Base.connected?
     return answer_opts unless current_user
+    return answer_opts unless log
+
+    if log.owning_organisation_id.present?
+      answer_opts = answer_opts.merge({ log.owning_organisation.id => log.owning_organisation.name })
+    end
 
     if !current_user.support? && current_user.organisation.holds_own_stock?
       answer_opts[current_user.organisation.id] = "#{current_user.organisation.name} (Your organisation)"
@@ -28,7 +34,10 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question
     answer_options
   end
 
-  def label_from_value(value)
+  def label_from_value(value, log = nil, user = nil)
+    @log = log
+    @current_user = user
+
     return unless value
 
     answer_options[value]

app/models/form/question.rb

@@ -46,11 +46,11 @@ class Form::Question
   delegate :subsection, to: :page
   delegate :form, to: :subsection
 
-  def answer_label(log)
+  def answer_label(log, user = nil)
     return checkbox_answer_label(log) if type == "checkbox"
     return log[id]&.to_formatted_s(:govuk_date).to_s if type == "date"
 
-    answer = label_from_value(log[id]) if log[id].present?
+    answer = label_from_value(log[id], log, user) if log[id].present?
     answer_label = [prefix, format_value(answer), suffix_label(log)].join("") if answer
 
     inferred_answer_value(log) || answer_label
@@ -151,7 +151,7 @@ class Form::Question
     end
   end
 
-  def label_from_value(value)
+  def label_from_value(value, _log = nil, _user = nil)
     return unless value
 
     label = case type

db/schema.rb

@@ -490,16 +490,16 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_16_151942) do
     t.integer "mortgagelender"
     t.string "mortgagelenderother"
     t.integer "mortlen"
-    t.integer "extrabor"
-    t.integer "hhmemb"
-    t.integer "totadult"
-    t.integer "totchild"
-    t.integer "hhtype"
     t.string "pcode1"
     t.string "pcode2"
     t.integer "pcodenk"
     t.string "postcode_full"
     t.boolean "is_la_inferred"
+    t.integer "extrabor"
+    t.integer "hhmemb"
+    t.integer "totadult"
+    t.integer "totchild"
+    t.integer "hhtype"
     t.integer "hodate_check"
     t.bigint "bulk_upload_id"
     t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id"

spec/features/lettings_log_spec.rb

@@ -58,7 +58,8 @@ RSpec.describe "Lettings Log Features" do
   end
 
   context "when the signed is user is a Support user" do
-    let(:support_user) { create(:user, :support, last_sign_in_at: Time.zone.now) }
+    let(:organisation) { create(:organisation, name: "User org") }
+    let(:support_user) { create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) }
     let(:devise_notify_mailer) { DeviseNotifyMailer.new }
     let(:notify_client) { instance_double(Notifications::Client) }
     let(:mfa_template_id) { User::MFA_TEMPLATE_ID }
@@ -78,7 +79,7 @@ RSpec.describe "Lettings Log Features" do
     end
 
     context "when completing the setup lettings log section", :aggregate_failure do
-      it "includes the organisation and created by questions" do
+      it "includes the owning organisation and created by questions" do
         visit("/lettings-logs")
         click_button("Create a new lettings log")
         click_link("Set up this lettings log")
@@ -88,14 +89,116 @@ RSpec.describe "Lettings Log Features" do
         click_button("Save and continue")
         log_id = page.current_path.scan(/\d/).join
         visit("lettings-logs/#{log_id}/setup/check-answers")
-        expect(page).to have_content("Stock owner #{support_user.organisation.name}")
+        expect(page).to have_content("Stock owner User org")
         expect(page).to have_content("You have answered 2 of 8 questions")
       end
     end
+
+    context "when the owning organisation question isn't answered" do
+      it "doesn't show the managing agent question" do
+        visit("/lettings-logs")
+        click_button("Create a new lettings log")
+        click_link("Set up this lettings log")
+        log_id = page.current_path.scan(/\d/).join
+        click_link("Skip for now")
+        expect(page).not_to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+      end
+    end
+
+    context "when the owning organisation question is answered" do
+      context "and the owning organisation doesn't hold stock" do
+        let(:managing_org) { create(:organisation, name: "Managing org") }
+        let!(:org_rel) { create(:organisation_relationship, parent_organisation: support_user.organisation, child_organisation: managing_org) }
+
+        before do
+          support_user.organisation.update!(holds_own_stock: false)
+        end
+
+        it "shows the managing organisation question" do
+          visit("/lettings-logs")
+          click_button("Create a new lettings log")
+          click_link("Set up this lettings log")
+          log_id = page.current_path.scan(/\d/).join
+          select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field")
+          click_button("Save and continue")
+          expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+          select(managing_org.name, from: "lettings-log-managing-organisation-id-field")
+          click_button("Save and continue")
+          visit("lettings-logs/#{log_id}/setup/check-answers")
+          expect(page).to have_content("Managing agent Managing org")
+          expect(support_user.organisation.managing_agents).to eq([org_rel.child_organisation])
+        end
+      end
+
+      context "and the owning organisation does hold stock" do
+        before do
+          support_user.organisation.update!(holds_own_stock: true)
+        end
+
+        context "and the owning organisation has no managing agents" do
+          it "doesn't show the managing organisation question" do
+            visit("/lettings-logs")
+            click_button("Create a new lettings log")
+            click_link("Set up this lettings log")
+            log_id = page.current_path.scan(/\d/).join
+            select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field")
+            click_button("Save and continue")
+            expect(page).not_to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+            visit("lettings-logs/#{log_id}/setup/check-answers")
+            expect(page).not_to have_content("Managing agent ")
+          end
+        end
+
+        context "and the owning organisation has 1 or more managing agents" do
+          let(:managing_org1) { create(:organisation, name: "Managing org 1") }
+          let!(:org_rel1) { create(:organisation_relationship, parent_organisation: support_user.organisation, child_organisation: managing_org1) }
+
+          it "does show the managing organisation question" do
+            visit("/lettings-logs")
+            click_button("Create a new lettings log")
+            click_link("Set up this lettings log")
+            log_id = page.current_path.scan(/\d/).join
+            select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field")
+            click_button("Save and continue")
+            expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+            select(managing_org1.name, from: "lettings-log-managing-organisation-id-field")
+            click_button("Save and continue")
+            visit("lettings-logs/#{log_id}/setup/check-answers")
+            expect(page).to have_content("Managing agent Managing org 1")
+          end
+
+          context "and the owning organisation has 2 or more managing agents" do
+            let(:managing_org2) { create(:organisation, name: "Managing org 2") }
+            let!(:org_rel2) { create(:organisation_relationship, parent_organisation: support_user.organisation, child_organisation: managing_org2) }
+
+            context "and the organisation relationship for the selected managing agent is deleted" do
+              it "doesn't change the CYA page text to be 'You didn't answer this question'" do
+                visit("/lettings-logs")
+                click_button("Create a new lettings log")
+                click_link("Set up this lettings log")
+                log_id = page.current_path.scan(/\d/).join
+                select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field")
+                click_button("Save and continue")
+                expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+                select(managing_org1.name, from: "lettings-log-managing-organisation-id-field")
+                click_button("Save and continue")
+                visit("lettings-logs/#{log_id}/setup/check-answers")
+                expect(page).to have_content("Managing agent Managing org 1")
+                org_rel1.destroy!
+                visit("lettings-logs/#{log_id}/setup/check-answers")
+                expect(page).to have_content("Managing agent Managing org 1")
+                expect(support_user.organisation.managing_agents).to eq([org_rel2.child_organisation])
+              end
+            end
+          end
+        end
+      end
+    end
   end
 
   context "when the signed is user is not a Support user" do
-    let(:user) { create(:user) }
+    let(:organisation) { create(:organisation, name: "User org") }
+    let(:user) { create(:user, :data_coordinator, name: "User name", organisation:) }
     let(:devise_notify_mailer) { DeviseNotifyMailer.new }
     let(:notify_client) { instance_double(Notifications::Client) }
 
@@ -110,15 +213,125 @@ RSpec.describe "Lettings Log Features" do
     end
 
     context "when completing the setup log section" do
-      it "does not include the organisation and created by questions" do
+      context "and there is at most 1 potential stock owner" do
-        visit("/lettings-logs")
+        it "does not include the owning organisation and created by questions" do
-        click_button("Create a new lettings log")
+          visit("/lettings-logs")
-        click_link("Set up this lettings log")
+          click_button("Create a new lettings log")
-        log_id = page.current_path.scan(/\d/).join
+          click_link("Set up this lettings log")
-        expect(page).to have_current_path("/lettings-logs/#{log_id}/needs-type")
+          log_id = page.current_path.scan(/\d/).join
-        visit("lettings-logs/#{log_id}/setup/check-answers")
+          expect(page).to have_current_path("/lettings-logs/#{log_id}/needs-type")
-        expect(page).not_to have_content("Owning organisation #{user.organisation.name}")
+          visit("lettings-logs/#{log_id}/setup/check-answers")
-        expect(page).not_to have_content("Log owner #{user.name}")
+          expect(page).not_to have_content("Stock owner ")
+          expect(page).not_to have_content("Log owner ")
+        end
+      end
+
+      context "and there are 2 or more potential stock owners" do
+        let(:owning_org1) { create(:organisation, name: "Owning org 1") }
+        let!(:org_rel1) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org1) }
+
+        it "does include the owning organisation question" do
+          visit("/lettings-logs")
+          click_button("Create a new lettings log")
+          click_link("Set up this lettings log")
+          log_id = page.current_path.scan(/\d/).join
+          expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner")
+          visit("lettings-logs/#{log_id}/setup/check-answers")
+          expect(page).to have_content("Stock owner User org")
+        end
+
+        context "and there are 3 or more potential stock owners" do
+          let(:owning_org2) { create(:organisation, name: "Owning org 2") }
+          let!(:org_rel2) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org2) }
+
+          context "and the organisation relationship for the selected stock owner is deleted" do
+            it "doesn't change the CYA page text to be 'You didn't answer this question'" do
+              visit("/lettings-logs")
+              click_button("Create a new lettings log")
+              click_link("Set up this lettings log")
+              log_id = page.current_path.scan(/\d/).join
+              expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner")
+              select(owning_org1.name, from: "lettings-log-owning-organisation-id-field")
+              click_button("Save and continue")
+              visit("lettings-logs/#{log_id}/setup/check-answers")
+              expect(page).to have_content("Stock owner Owning org 1")
+              org_rel1.destroy!
+              visit("lettings-logs/#{log_id}/setup/check-answers")
+              expect(page).to have_content("Stock owner Owning org 1")
+              expect(user.organisation.stock_owners).to eq([org_rel2.parent_organisation])
+            end
+          end
+        end
+      end
+
+      context "when the current user's organisation doesn't hold stock" do
+        let(:owning_org1) { create(:organisation, name: "Owning org 1") }
+        let(:owning_org2) { create(:organisation, name: "Owning org 2") }
+        let!(:org_rel1) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org1) }
+        let!(:org_rel2) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org2) }
+
+        it "shows the managing organisation question" do
+          user.organisation.update!(holds_own_stock: false)
+          visit("/lettings-logs")
+          click_button("Create a new lettings log")
+          click_link("Set up this lettings log")
+          log_id = page.current_path.scan(/\d/).join
+          expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner")
+          select(owning_org1.name, from: "lettings-log-owning-organisation-id-field")
+          click_button("Save and continue")
+          expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+          select(user.organisation.name, from: "lettings-log-managing-organisation-id-field")
+          click_button("Save and continue")
+          visit("lettings-logs/#{log_id}/setup/check-answers")
+          expect(page).to have_content("Managing agent User org")
+          expect(user.organisation.stock_owners).to eq([org_rel1.parent_organisation, org_rel2.parent_organisation])
+        end
+      end
+
+      context "when the current user's organisation does hold stock" do
+        let!(:owning_org) { create(:organisation, name: "Owning org") }
+        let!(:org_rel1) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org) }
+
+        before do
+          user.organisation.update!(holds_own_stock: true)
+        end
+
+        context "and the user's organisation has no managing agents" do
+          it "doesn't show the managing organisation question" do
+            visit("/lettings-logs")
+            click_button("Create a new lettings log")
+            click_link("Set up this lettings log")
+            log_id = page.current_path.scan(/\d/).join
+            expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner")
+            select(owning_org.name, from: "lettings-log-owning-organisation-id-field")
+            click_button("Save and continue")
+            expect(page).not_to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+            visit("lettings-logs/#{log_id}/setup/check-answers")
+            expect(page).not_to have_content("Managing agent ")
+            expect(user.organisation.stock_owners).to eq([org_rel1.parent_organisation])
+          end
+        end
+
+        context "and the user's organisation has 1 or more managing agents" do
+          let(:managing_org) { create(:organisation, name: "Managing org") }
+          let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: managing_org) }
+
+          it "does show the managing organisation question" do
+            visit("/lettings-logs")
+            click_button("Create a new lettings log")
+            click_link("Set up this lettings log")
+            log_id = page.current_path.scan(/\d/).join
+            expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner")
+            select(user.organisation.name, from: "lettings-log-owning-organisation-id-field")
+            click_button("Save and continue")
+            expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation")
+            select(managing_org.name, from: "lettings-log-managing-organisation-id-field")
+            click_button("Save and continue")
+            visit("lettings-logs/#{log_id}/setup/check-answers")
+            expect(page).to have_content("Managing agent Managing org")
+            expect(user.organisation.managing_agents).to eq([org_rel2.child_organisation])
+          end
+        end
       end
     end
 

spec/models/form/lettings/questions/managing_organisation_spec.rb

@@ -52,86 +52,97 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do
       end
     end
 
-    context "when user not support and owns own stock" do
+    context "when user is not support" do
-      let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) }
+      let(:user_org) { create(:organisation, name: "User org") }
+      let(:user) { create(:user, :data_coordinator, organisation: user_org) }
 
-      let(:log) { create(:lettings_log) }
+      let(:managing_org1) { create(:organisation, name: "Managing org 1") }
-      let!(:org_rel1) { create(:organisation_relationship, parent_organisation: user.organisation) }
+      let(:managing_org2) { create(:organisation, name: "Managing org 2") }
-      let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation) }
+      let(:managing_org3) { create(:organisation, name: "Managing org 3") }
 
-      let(:options) do
+      let(:log) { create(:lettings_log, managing_organisation: managing_org1) }
-        {
+      let!(:org_rel1) { create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: managing_org2) }
-          "" => "Select an option",
+      let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: managing_org3) }
-          user.organisation.id => "#{user.organisation.name} (Your organisation)",
-          org_rel1.child_organisation.id => org_rel1.child_organisation.name,
-          org_rel2.child_organisation.id => org_rel2.child_organisation.name,
-        }
-      end
-
-      it "shows managing agents with own org at the top" do
-        expect(question.displayed_answer_options(log, user)).to eq(options)
-      end
-    end
-
-    context "when user not support and does not own stock" do
-      let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) }
-
-      let(:log) { create(:lettings_log) }
-      let!(:org_rel1) { create(:organisation_relationship, parent_organisation: user.organisation) }
-      let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation) }
 
       let(:options) do
         {
           "" => "Select an option",
-          user.organisation.id => "#{user.organisation.name} (Your organisation)",
+          log.managing_organisation.id => "Managing org 1",
-          org_rel1.child_organisation.id => org_rel1.child_organisation.name,
+          user.organisation.id => "User org (Your organisation)",
-          org_rel2.child_organisation.id => org_rel2.child_organisation.name,
+          org_rel1.child_organisation.id => "Managing org 2",
+          org_rel2.child_organisation.id => "Managing org 3",
         }
       end
 
-      it "shows managing agents with own org at the top" do
+      it "shows current managing agent at top, followed by user's org (with hint), followed by the managing agents of the user's org" do
         expect(question.displayed_answer_options(log, user)).to eq(options)
       end
     end
 
-    context "when support user and org does not own own stock" do
+    context "when user is support" do
       let(:user) { create(:user, :support) }
-      let(:log_owning_org) { create(:organisation, holds_own_stock: false) }
+      let(:log_owning_org) { create(:organisation, name: "Owning org") }
-      let(:log) { create(:lettings_log, owning_organisation: log_owning_org) }
+
-      let!(:org_rel1) { create(:organisation_relationship, parent_organisation: log_owning_org) }
+      let(:managing_org1) { create(:organisation, name: "Managing org 1") }
-      let!(:org_rel2) { create(:organisation_relationship, parent_organisation: log_owning_org) }
+      let(:managing_org2) { create(:organisation, name: "Managing org 2") }
-
+      let(:managing_org3) { create(:organisation, name: "Managing org 3") }
-      let(:options) do
+
-        {
+      let(:log) { create(:lettings_log, owning_organisation: log_owning_org, managing_organisation: managing_org1, created_by: nil) }
-          "" => "Select an option",
+      let!(:org_rel1) { create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: managing_org2) }
-          org_rel1.child_organisation.id => org_rel1.child_organisation.name,
+      let!(:org_rel2) { create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: managing_org3) }
-          org_rel2.child_organisation.id => org_rel2.child_organisation.name,
+
-        }
+      context "when org owns stock" do
+        let(:options) do
+          {
+            "" => "Select an option",
+            log.managing_organisation.id => "Managing org 1",
+            log_owning_org.id => "Owning org (Owning organisation)",
+            org_rel1.child_organisation.id => "Managing org 2",
+            org_rel2.child_organisation.id => "Managing org 3",
+          }
+        end
+
+        it "shows current managing agent at top, followed by the current owning organisation (with hint), followed by the managing agents of the current owning organisation" do
+          log_owning_org.update!(holds_own_stock: true)
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
       end
 
-      it "shows owning org managing agents with hint" do
+      context "when org does not own stock" do
-        expect(question.displayed_answer_options(log, user)).to eq(options)
+        let(:options) do
+          {
+            "" => "Select an option",
+            log.managing_organisation.id => "Managing org 1",
+            org_rel1.child_organisation.id => "Managing org 2",
+            org_rel2.child_organisation.id => "Managing org 3",
+          }
+        end
+
+        it "shows current managing agent at top, followed by the managing agents of the current owning organisation" do
+          log_owning_org.update!(holds_own_stock: false)
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
       end
     end
 
-    context "when support user and org does own stock" do
+    context "when the owning-managing organisation relationship is deleted" do
       let(:user) { create(:user, :support) }
-      let(:log_owning_org) { create(:organisation, holds_own_stock: true) }
+
-      let(:log) { create(:lettings_log, owning_organisation: log_owning_org) }
+      let(:owning_org) { create(:organisation, name: "Owning org", holds_own_stock: true) }
-      let!(:org_rel1) { create(:organisation_relationship, parent_organisation: log_owning_org) }
+      let(:managing_org) { create(:organisation, name: "Managing org", holds_own_stock: false) }
-      let!(:org_rel2) { create(:organisation_relationship, parent_organisation: log_owning_org) }
+      let(:org_rel) { create(:organisation_relationship, parent_organisation: owning_org, child_organisation: managing_org) }
+      let(:log) { create(:lettings_log, owning_organisation: owning_org, managing_organisation: managing_org, created_by: nil) }
 
       let(:options) do
         {
           "" => "Select an option",
-          log_owning_org.id => "#{log_owning_org.name} (Owning organisation)",
+          owning_org.id => "Owning org (Owning organisation)",
-          org_rel1.child_organisation.id => org_rel1.child_organisation.name,
+          managing_org.id => "Managing org",
-          org_rel2.child_organisation.id => org_rel2.child_organisation.name,
         }
       end
 
-      it "shows owning org managing agents
+      it "doesn't remove the managing org from the list of allowed managing orgs" do
-      " do
+        org_rel.destroy!
         expect(question.displayed_answer_options(log, user)).to eq(options)
       end
     end

spec/models/form/lettings/questions/stock_owner_spec.rb

@@ -42,28 +42,68 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
       end
     end
 
-    context "when user not support and owns own stock" do
+    context "when user is not support" do
-      let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) }
+      let(:user_org) { create(:organisation, name: "User org") }
-      let(:options) do
+      let(:user) { create(:user, :data_coordinator, organisation: user_org) }
-        {
+
-          "" => "Select an option",
+      let(:owning_org_1) { create(:organisation, name: "Owning org 1") }
-          user.organisation.id => "#{user.organisation.name} (Your organisation)",
+      let(:owning_org_2) { create(:organisation, name: "Owning org 2") }
-        }
+      let!(:org_rel) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2) }
-      end
+      let(:log) { create(:lettings_log, owning_organisation: owning_org_1) }
+
+      context "when user's org owns stock" do
+        let(:options) 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
 
-      before do
+        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
-        question.current_user = user
+          user.organisation.update!(holds_own_stock: true)
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
+
+        context "when the owning-managing organisation relationship is deleted" do
+          let(:options) do
+            {
+              "" => "Select an option",
+              user.organisation.id => "User org (Your organisation)",
+              owning_org_2.id => "Owning org 2",
+            }
+          end
+
+          it "doesn't remove the housing provider from the list of allowed housing providers" do
+            log.update!(owning_organisation: owning_org_2)
+            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
 
-      it "shows stock owners with own org at the top" do
+      context "when user's org doesn't own stock" do
-        expect(question.answer_options).to eq(options)
+        let(:options) do
+          {
+            "" => "Select an option",
+            owning_org_1.id => "Owning org 1",
+            owning_org_2.id => "Owning org 2",
+          }
+        end
+
+        it "shows current stock owner at top, followed by the stock owners of the user's org" do
+          user.organisation.update!(holds_own_stock: false)
+          expect(question.displayed_answer_options(log, user)).to eq(options)
+        end
       end
     end
 
-    context "when user support" do
+    context "when user is support" do
-      before do
+      let(:user) { create(:user, :support) }
-        question.current_user = create(:user, :support)
+
-      end
+      let(:log) { create(:lettings_log) }
 
       let(:expected_opts) do
         Organisation.all.each_with_object(options) do |organisation, hsh|
@@ -73,7 +113,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do
       end
 
       it "shows all orgs" do
-        expect(question.answer_options).to eq(expected_opts)
+        expect(question.displayed_answer_options(log, user)).to eq(expected_opts)
       end
     end
   end

spec/requests/lettings_logs_controller_spec.rb

@@ -143,6 +143,64 @@ RSpec.describe LettingsLogsController, type: :request do
         expect(whodunnit_actor).to be_a(User)
         expect(whodunnit_actor.id).to eq(user.id)
       end
+
+      context "when creating a new log" do
+        context "when the user is support" do
+          let(:organisation) { FactoryBot.create(:organisation) }
+          let(:support_user) { FactoryBot.create(:user, :support, organisation:) }
+
+          before do
+            allow(support_user).to receive(:need_two_factor_authentication?).and_return(false)
+            sign_in support_user
+            post "/lettings-logs", headers:
+          end
+
+          it "sets the managing org and stock-owning org as nil" do
+            created_id = response.location.match(/[0-9]+/)[0]
+            lettings_log = LettingsLog.find_by(id: created_id)
+            expect(lettings_log.owning_organisation).to eq(nil)
+            expect(lettings_log.managing_organisation).to eq(nil)
+          end
+        end
+
+        context "when the user is not support" do
+          context "when the user's org holds stock" do
+            let(:organisation) { FactoryBot.create(:organisation, name: "User org", holds_own_stock: true) }
+            let(:user) { FactoryBot.create(:user, :data_coordinator, organisation:) }
+
+            before do
+              RequestHelper.stub_http_requests
+              sign_in user
+              post "/lettings-logs", headers:
+            end
+
+            it "sets the managing org and stock-owning org as the user's org" do
+              created_id = response.location.match(/[0-9]+/)[0]
+              lettings_log = LettingsLog.find_by(id: created_id)
+              expect(lettings_log.owning_organisation.name).to eq("User org")
+              expect(lettings_log.managing_organisation.name).to eq("User org")
+            end
+          end
+
+          context "when the user's org doesn't hold stock" do
+            let(:organisation) { FactoryBot.create(:organisation, name: "User org", holds_own_stock: false) }
+            let(:user) { FactoryBot.create(:user, :data_coordinator, organisation:) }
+
+            before do
+              RequestHelper.stub_http_requests
+              sign_in user
+              post "/lettings-logs", headers:
+            end
+
+            it "sets the managing org as the user's org but the stock-owning org as nil" do
+              created_id = response.location.match(/[0-9]+/)[0]
+              lettings_log = LettingsLog.find_by(id: created_id)
+              expect(lettings_log.owning_organisation).to eq(nil)
+              expect(lettings_log.managing_organisation.name).to eq("User org")
+            end
+          end
+        end
+      end
     end
   end