Reset created by when owning org changes (#692)

app/models/case_log.rb

@@ -511,9 +511,16 @@ private
     end
   end
 
+  def reset_created_by
+    return unless created_by && owning_organisation
+
+    self.created_by = nil if created_by.organisation != owning_organisation
+  end
+
   def reset_invalidated_dependent_fields!
     return unless form
 
+    reset_created_by
     reset_not_routed_questions
     reset_derived_questions
   end

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

@@ -19,11 +19,8 @@ class Form::Setup::Questions::OwningOrganisationId < ::Form::Question
     end
   end
 
-  def displayed_answer_options(case_log)
+  def displayed_answer_options(_case_log)
-    return answer_options unless case_log.created_by
+    answer_options
-
-    ids = ["", case_log.created_by.organisation.id]
-    answer_options.select { |k, _v| ids.include?(k) }
   end
 
   def label_from_value(value)

spec/factories/case_log.rb

@@ -1,8 +1,8 @@
 FactoryBot.define do
   factory :case_log do
-    owning_organisation { FactoryBot.create(:organisation) }
-    managing_organisation { FactoryBot.create(:organisation) }
     created_by { FactoryBot.create(:user) }
+    owning_organisation { created_by.organisation }
+    managing_organisation { created_by.organisation }
     trait :about_completed do
       renewal { 0 }
       needstype { 1 }

spec/features/form/check_answers_page_spec.rb

@@ -194,6 +194,7 @@ RSpec.describe "Form Check Answers Page" do
           :in_progress,
           owning_organisation: user.organisation,
           managing_organisation: user.organisation,
+          created_by: user,
           needstype: 1,
           tenancycode: nil,
           age1: nil,

spec/features/form/tasklist_page_spec.rb

@@ -10,6 +10,7 @@ RSpec.describe "Task List" do
       :in_progress,
       owning_organisation: user.organisation,
       managing_organisation: user.organisation,
+      created_by: user,
     )
   end
   let(:completed_case_log) do
@@ -18,6 +19,7 @@ RSpec.describe "Task List" do
       :completed,
       owning_organisation: user.organisation,
       managing_organisation: user.organisation,
+      created_by: user,
     )
   end
   let(:empty_case_log) do
@@ -25,6 +27,7 @@ RSpec.describe "Task List" do
       :case_log,
       owning_organisation: user.organisation,
       managing_organisation: user.organisation,
+      created_by: user,
     )
   end
   let(:setup_completed_log) do
@@ -34,6 +37,7 @@ RSpec.describe "Task List" do
       owning_organisation: user.organisation,
       managing_organisation: user.organisation,
       startdate: Time.zone.local(2021, 5, 1),
+      created_by: user,
     )
   end
   let(:id) { case_log.id }

spec/models/case_log_spec.rb

@@ -1778,6 +1778,16 @@ RSpec.describe CaseLog do
         expect(case_log["waityear"]).to eq(2)
       end
     end
+
+    context "when a support user changes the owning organisation of the log" do
+      let(:case_log) { FactoryBot.create(:case_log, created_by: created_by_user) }
+      let(:organisation_2) { FactoryBot.create(:organisation) }
+
+      it "clears the created by user set" do
+        expect { case_log.update!(owning_organisation: organisation_2) }
+          .to change { case_log.reload.created_by }.from(created_by_user).to(nil)
+      end
+    end
   end
 
   describe "tshortfall_unknown?" do

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

@@ -65,19 +65,4 @@ RSpec.describe Form::Setup::Questions::OwningOrganisationId, type: :model do
       expect(question.hidden_in_check_answers?(nil, user)).to be true
     end
   end
-
-  context "when the user is already set" do
-    let(:user) { FactoryBot.create(:user, organisation: organisation_2) }
-    let(:case_log) { FactoryBot.create(:case_log, created_by: user) }
-    let(:expected_answer_options) do
-      {
-        "" => "Select an option",
-        organisation_2.id => organisation_2.name,
-      }
-    end
-
-    it "only displays users that belong to that organisation" do
-      expect(question.displayed_answer_options(case_log)).to eq(expected_answer_options)
-    end
-  end
 end

spec/models/organisation_spec.rb

@@ -68,6 +68,7 @@ RSpec.describe Organisation, type: :model do
           :completed,
           owning_organisation: organisation,
           managing_organisation: other_organisation,
+          created_by: user,
         )
       end
       let!(:managed_case_log) do

spec/models/user_spec.rb

@@ -10,6 +10,7 @@ RSpec.describe User, type: :model do
         :completed,
         owning_organisation: user.organisation,
         managing_organisation: other_organisation,
+        created_by: user,
       )
     end
     let!(:managed_case_log) do

spec/requests/case_logs_controller_spec.rb

@@ -1,9 +1,9 @@
 require "rails_helper"
 
 RSpec.describe CaseLogsController, type: :request do
-  let(:owning_organisation) { FactoryBot.create(:organisation) }
-  let(:managing_organisation) { owning_organisation }
   let(:user) { FactoryBot.create(:user) }
+  let(:owning_organisation) { user.organisation }
+  let(:managing_organisation) { owning_organisation }
   let(:api_username) { "test_user" }
   let(:api_password) { "test_password" }
   let(:basic_credentials) do
@@ -198,15 +198,18 @@ RSpec.describe CaseLogsController, type: :request do
         context "when filtering" do
           context "with status filter" do
             let(:organisation_2) { FactoryBot.create(:organisation) }
+            let(:user_2) { FactoryBot.create(:user, organisation: organisation_2) }
             let!(:in_progress_case_log) do
               FactoryBot.create(:case_log, :in_progress,
                                 owning_organisation: organisation,
-                                managing_organisation: organisation)
+                                managing_organisation: organisation,
+                                created_by: user)
             end
             let!(:completed_case_log) do
               FactoryBot.create(:case_log, :completed,
                                 owning_organisation: organisation_2,
-                                managing_organisation: organisation)
+                                managing_organisation: organisation,
+                                created_by: user_2)
             end
 
             it "shows case logs for multiple selected statuses" do
@@ -272,7 +275,8 @@ RSpec.describe CaseLogsController, type: :request do
               FactoryBot.create(:case_log, :in_progress,
                                 owning_organisation: organisation,
                                 startdate: Time.zone.local(2022, 3, 1),
-                                managing_organisation: organisation)
+                                managing_organisation: organisation,
+                                created_by: user)
             end
             let!(:case_log_2022) do
               FactoryBot.create(:case_log, :completed,
@@ -280,7 +284,8 @@ RSpec.describe CaseLogsController, type: :request do
                                 mrcdate: Time.zone.local(2022, 2, 1),
                                 startdate: Time.zone.local(2022, 12, 1),
                                 tenancy: 6,
-                                managing_organisation: organisation)
+                                managing_organisation: organisation,
+                                created_by: user)
             end
             let!(:case_log_2022_in_progress) do
               FactoryBot.create(:case_log, :in_progress,
@@ -289,7 +294,8 @@ RSpec.describe CaseLogsController, type: :request do
                                 startdate: Time.zone.local(2022, 12, 1),
                                 tenancy: 6,
                                 managing_organisation: organisation,
-                                tenancycode: nil)
+                                tenancycode: nil,
+                                created_by: user)
             end
 
             it "shows case logs for multiple selected statuses and years" do
@@ -321,8 +327,8 @@ RSpec.describe CaseLogsController, type: :request do
         end
 
         context "when using a search query" do
-          let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation) }
+          let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation, created_by: user) }
-          let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation) }
+          let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation, created_by: user) }
           let(:log_total_count) { CaseLog.where(owning_organisation: user.organisation).count }
 
           it "has search results in the title" do
@@ -377,7 +383,7 @@ RSpec.describe CaseLogsController, type: :request do
 
           context "when there are more than 1 page of search results" do
             let(:postcode) { "XX11YY" }
-            let(:logs) { FactoryBot.create_list(:case_log, 30, :completed, owning_organisation: user.organisation, postcode_full: postcode) }
+            let(:logs) { FactoryBot.create_list(:case_log, 30, :completed, owning_organisation: user.organisation, postcode_full: postcode, created_by: user) }
             let(:log_total_count) { CaseLog.where(owning_organisation: user.organisation).count }
 
             it "has title with pagination details for page 1" do
@@ -414,7 +420,7 @@ RSpec.describe CaseLogsController, type: :request do
           context "when search and filter is present" do
             let(:matching_postcode) { log_to_search.postcode_full }
             let(:matching_status) { "in_progress" }
-            let!(:log_matching_filter_and_search) { FactoryBot.create(:case_log, :in_progress, owning_organisation: user.organisation, postcode_full: matching_postcode) }
+            let!(:log_matching_filter_and_search) { FactoryBot.create(:case_log, :in_progress, owning_organisation: user.organisation, postcode_full: matching_postcode, created_by: user) }
 
             it "shows only logs matching both search and filters" do
               get "/logs?search=#{matching_postcode}&status[]=#{matching_status}", headers: headers, params: {}
@@ -732,7 +738,8 @@ RSpec.describe CaseLogsController, type: :request do
         FactoryBot.create(:case_log,
                           :completed,
                           owning_organisation: organisation,
-                          managing_organisation: organisation)
+                          managing_organisation: organisation,
+                          created_by: user)
         get "/logs", headers:, params: {}
       end
 
@@ -768,8 +775,8 @@ RSpec.describe CaseLogsController, type: :request do
         let(:postcode) { "XX1 1TG" }
 
         before do
-          FactoryBot.create(:case_log, :in_progress, postcode_full: postcode, owning_organisation: organisation)
+          FactoryBot.create(:case_log, :in_progress, postcode_full: postcode, owning_organisation: organisation, created_by: user)
-          FactoryBot.create(:case_log, :completed, postcode_full: postcode, owning_organisation: organisation)
+          FactoryBot.create(:case_log, :completed, postcode_full: postcode, owning_organisation: organisation, created_by: user)
         end
 
         it "downloads logs matching both csv and filter logs" do

spec/requests/form_controller_spec.rb

@@ -77,8 +77,8 @@ RSpec.describe FormController, type: :request do
     describe "GET" do
       context "with form pages" do
         context "when forms exist for multiple years" do
-          let(:case_log_year_1) { FactoryBot.create(:case_log, startdate: Time.zone.local(2021, 5, 1), owning_organisation: organisation) }
+          let(:case_log_year_1) { FactoryBot.create(:case_log, startdate: Time.zone.local(2021, 5, 1), owning_organisation: organisation, created_by: user) }
-          let(:case_log_year_2) { FactoryBot.create(:case_log, :about_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation) }
+          let(:case_log_year_2) { FactoryBot.create(:case_log, :about_completed, startdate: Time.zone.local(2022, 5, 1), owning_organisation: organisation, created_by: user) }
 
           it "displays the correct question details for each case log based on form year" do
             get "/logs/#{case_log_year_1.id}/tenant-code-test", headers: headers, params: {}

spec/requests/organisations_controller_spec.rb

@@ -606,8 +606,8 @@ RSpec.describe OrganisationsController, type: :request do
           end
 
           context "when using a search query" do
-            let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation) }
+            let(:logs) { FactoryBot.create_list(:case_log, 3, :completed, owning_organisation: user.organisation, created_by: user) }
-            let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation) }
+            let(:log_to_search) { FactoryBot.create(:case_log, :completed, owning_organisation: user.organisation, created_by: user) }
             let(:log_total_count) { CaseLog.where(owning_organisation: user.organisation).count }
 
             it "has search results in the title" do
@@ -699,7 +699,7 @@ RSpec.describe OrganisationsController, type: :request do
             context "when search and filter is present" do
               let(:matching_postcode) { log_to_search.postcode_full }
               let(:matching_status) { "in_progress" }
-              let!(:log_matching_filter_and_search) { FactoryBot.create(:case_log, :in_progress, owning_organisation: user.organisation, postcode_full: matching_postcode) }
+              let!(:log_matching_filter_and_search) { FactoryBot.create(:case_log, :in_progress, owning_organisation: user.organisation, postcode_full: matching_postcode, created_by: user) }
 
               it "shows only logs matching both search and filters" do
                 get "/organisations/#{organisation.id}/logs?search=#{matching_postcode}&status[]=#{matching_status}", headers: headers, params: {}

spec/requests/schemes_controller_spec.rb

@@ -329,7 +329,8 @@ RSpec.describe SchemesController, type: :request do
           end
 
           it "has correct page 1 of 2 title" do
-            expect(page).to have_title("#{scheme.service_name} (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
+            expected_title = CGI.escapeHTML("#{scheme.service_name} (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
+            expect(page).to have_title(expected_title)
           end
 
           it "has pagination links" do

spec/services/imports/case_logs_field_import_service_spec.rb

@@ -14,17 +14,17 @@ RSpec.describe Imports::CaseLogsFieldImportService do
   let(:case_log_xml) { Nokogiri::XML(case_log_file) }
   let(:remote_folder) { "case_logs" }
   let(:old_user_id) { "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa" }
+  let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
 
   def open_file(directory, filename)
     File.open("#{directory}/#{filename}.xml")
   end
 
   before do
-    # Owning and Managing organisations
+    allow(Organisation).to receive(:find_by).and_return(organisation)
-    FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP")
 
     # Created by users
-    FactoryBot.create(:user, old_user_id:)
+    FactoryBot.create(:user, old_user_id:, organisation:)
 
     # Stub the form handler to use the real form
     allow(FormHandler.instance).to receive(:get_form).with("2021_2022").and_return(real_2021_2022_form)

spec/services/imports/case_logs_import_service_spec.rb

@@ -9,18 +9,19 @@ RSpec.describe Imports::CaseLogsImportService do
   let(:real_2021_2022_form) { Form.new("config/forms/2021_2022.json", "2021_2022") }
   let(:real_2022_2023_form) { Form.new("config/forms/2022_2023.json", "2022_2023") }
   let(:fixture_directory) { "spec/fixtures/softwire_imports/case_logs" }
+  let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
 
   def open_file(directory, filename)
     File.open("#{directory}/#{filename}.xml")
   end
 
   before do
-    # Owning and Managing organisations
+    allow(Organisation).to receive(:find_by).and_return(nil)
-    FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP")
+    allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id.to_i).and_return(organisation)
 
     # Created by users
-    FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa")
+    FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:)
-    FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f")
+    FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f", organisation:)
 
     # Stub the form handler to use the real form
     allow(FormHandler.instance).to receive(:get_form).with("2021_2022").and_return(real_2021_2022_form)