Validate the user belongs to either the managing or owning organisation (#1055)

* Validate that the user belongs to either the managing or owning organisation

* do not reset created_by and remove user_organisation_chosen?

* Do not default managing organisation to owning organisation

* validate user belongs to organisation

* update tests to specify created_by

* clear create_by for support users

* refactor

* typo

* rebase lint

* Add `updated_by` to logs to track who was the last person to update a log

- Reset `created_by` automatically when a form is updated and the owner does not belong to the managing or owning organisation

* move reset_invalidated_dependent_fields!, update schema file and fix tests

Co-authored-by: James Rose <james@jbpr.net>

app/controllers/form_controller.rb

@@ -10,7 +10,7 @@ class FormController < ApplicationController
       responses_for_page = responses_for_page(@page)
       mandatory_questions_with_no_response = mandatory_questions_with_no_response(responses_for_page)
 
-      if mandatory_questions_with_no_response.empty? && @log.update(responses_for_page)
+      if mandatory_questions_with_no_response.empty? && @log.update(responses_for_page.merge(updated_by: current_user))
         session[:errors] = session[:fields] = nil
         redirect_to(successful_redirect_path)
       else

app/models/derived_variables/lettings_log_variables.rb

@@ -9,8 +9,6 @@ module DerivedVariables::LettingsLogVariables
   end
 
   def set_derived_fields!
-    # TODO: Remove once we support parent/child relationships
-    self.managing_organisation_id ||= owning_organisation_id
     if rsnvac.present?
       self.newprop = has_first_let_vacancy_reason? ? 1 : 2
     end

app/models/lettings_log.rb

@@ -570,7 +570,6 @@ private
     super
 
     reset_invalid_unresolved_log_fields!
-    reset_created_by
     reset_scheme
     reset_derived_questions
   end

app/models/log.rb

@@ -4,6 +4,7 @@ class Log < ApplicationRecord
   belongs_to :owning_organisation, class_name: "Organisation", optional: true
   belongs_to :managing_organisation, class_name: "Organisation", optional: true
   belongs_to :created_by, class_name: "User", optional: true
+  belongs_to :updated_by, class_name: "User", optional: true
   before_save :update_status!
 
   STATUS = { "not_started" => 0, "in_progress" => 1, "completed" => 2 }.freeze
@@ -72,15 +73,19 @@ private
     subsection_statuses.all? { |status| not_started_statuses.include?(status) }
   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
 
     form.reset_not_routed_questions(self)
+
+    reset_created_by!
+  end
+
+  def reset_created_by!
+    return unless updated_by&.support?
+    return if owning_organisation.blank? || managing_organisation.blank? || created_by.blank?
+    return if created_by&.organisation == managing_organisation || created_by&.organisation == owning_organisation
+
+    update!(created_by: nil)
   end
 end

app/models/validations/setup_validations.rb

@@ -16,6 +16,15 @@ module Validations::SetupValidations
     scheme_during_startdate_validation(record, :scheme_id)
   end
 
+  def validate_organisation(record)
+    created_by, managing_organisation, owning_organisation = record.values_at("created_by", "managing_organisation", "owning_organisation")
+    unless [created_by, managing_organisation, owning_organisation].any?(&:blank?) || created_by.organisation == managing_organisation || created_by.organisation == owning_organisation
+      record.errors.add :created_by, I18n.t("validations.setup.created_by.invalid")
+      record.errors.add :owning_organisation_id, I18n.t("validations.setup.owning_organisation.invalid")
+      record.errors.add :managing_organisation_id, I18n.t("validations.setup.managing_organisation.invalid")
+    end
+  end
+
 private
 
   def intermediate_product_rent_type?(record)

app/services/csv/lettings_log_csv_service.rb

@@ -1,6 +1,6 @@
 module Csv
   class LettingsLogCsvService
-    CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check first_time_property_let_as_social_housing renttype needstype postcode_known is_la_inferred totchild totelder totadult net_income_known is_carehome previous_la_known is_previous_la_inferred age1_known age2_known age3_known age4_known age5_known age6_known age7_known age8_known letting_allocation_unknown details_known_2 details_known_3 details_known_4 details_known_5 details_known_6 details_known_7 details_known_8 rent_type_detail wrent wscharge wpschrge wsupchrg wtcharge wtshortfall rent_value_check old_form_id old_id retirement_value_check tshortfall_known pregnancy_value_check hhtype new_old vacdays la prevloc unresolved].freeze
+    CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check first_time_property_let_as_social_housing renttype needstype postcode_known is_la_inferred totchild totelder totadult net_income_known is_carehome previous_la_known is_previous_la_inferred age1_known age2_known age3_known age4_known age5_known age6_known age7_known age8_known letting_allocation_unknown details_known_2 details_known_3 details_known_4 details_known_5 details_known_6 details_known_7 details_known_8 rent_type_detail wrent wscharge wpschrge wsupchrg wtcharge wtshortfall rent_value_check old_form_id old_id retirement_value_check tshortfall_known pregnancy_value_check hhtype new_old vacdays la prevloc unresolved updated_by_id].freeze
 
     def initialize(user)
       @user = user

config/locales/en.yml

@@ -157,6 +157,12 @@ en:
           deactivated: "%{name} was deactivated on %{date} and was not available on the day you entered"
           reactivating_soon: "The scheme %{name} is not available until %{date}. Select another scheme or edit the tenancy start date"
           activating_soon: "%{name} is not available until %{date}. Enter a tenancy start date after %{date}"
+      owning_organisation:
+        invalid: "Please select owning organisation or managing organisation that you belong to"
+      managing_organisation:
+        invalid: "Please select owning organisation or managing organisation that you belong to"
+      created_by:
+        invalid: "Please select owning organisation or managing organisation that you belong to"
 
     property:
       mrcdate:

db/migrate/20221207141947_add_updated_by_to_logs.rb

@@ -0,0 +1,10 @@
+class AddUpdatedByToLogs < ActiveRecord::Migration[7.0]
+  def change
+    change_table :lettings_logs, bulk: true do |t|
+      t.belongs_to :updated_by, class_name: "User"
+    end
+    change_table :sales_logs, bulk: true do |t|
+      t.belongs_to :updated_by, class_name: "User"
+    end
+  end
+end

db/schema.rb

@@ -250,12 +250,14 @@ ActiveRecord::Schema[7.0].define(version: 2022_12_13_085819) do
     t.integer "housingneeds_type"
     t.integer "housingneeds_other"
     t.boolean "unresolved"
+    t.bigint "updated_by_id"
     t.index ["created_by_id"], name: "index_lettings_logs_on_created_by_id"
     t.index ["location_id"], name: "index_lettings_logs_on_location_id"
     t.index ["managing_organisation_id"], name: "index_lettings_logs_on_managing_organisation_id"
     t.index ["old_id"], name: "index_lettings_logs_on_old_id", unique: true
     t.index ["owning_organisation_id"], name: "index_lettings_logs_on_owning_organisation_id"
     t.index ["scheme_id"], name: "index_lettings_logs_on_scheme_id"
+    t.index ["updated_by_id"], name: "index_lettings_logs_on_updated_by_id"
   end
 
   create_table "location_deactivation_periods", force: :cascade do |t|
@@ -403,6 +405,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_12_13_085819) do
     t.integer "savings"
     t.integer "prevown"
     t.string "sex3"
+    t.bigint "updated_by_id"
     t.integer "details_known_1"
     t.integer "income1_value_check"
     t.integer "mortgage"
@@ -411,6 +414,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_12_13_085819) do
     t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id"
     t.index ["managing_organisation_id"], name: "index_sales_logs_on_managing_organisation_id"
     t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id"
+    t.index ["updated_by_id"], name: "index_sales_logs_on_updated_by_id"
   end
 
   create_table "scheme_deactivation_periods", force: :cascade do |t|

spec/features/form/check_answers_page_spec.rb

@@ -13,8 +13,7 @@ RSpec.describe "Form Check Answers Page" do
     FactoryBot.create(
       :lettings_log,
       :in_progress,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
       needstype: 2,
       scheme:,
       location:,
@@ -26,8 +25,7 @@ RSpec.describe "Form Check Answers Page" do
       previous_la_known: 1,
       prevloc: "E09000033",
       is_previous_la_inferred: false,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
   let(:completed_lettings_log) do
@@ -36,6 +34,7 @@ RSpec.describe "Form Check Answers Page" do
       :completed,
       owning_organisation: user.organisation,
       managing_organisation: user.organisation,
+      created_by: user,
       startdate: Time.zone.local(2021, 5, 1),
     )
   end
@@ -190,8 +189,7 @@ RSpec.describe "Form Check Answers Page" do
         FactoryBot.create(
           :lettings_log,
           :in_progress,
-          owning_organisation: user.organisation,
+          created_by: user,
-          managing_organisation: user.organisation,
           tenancycode: "123",
           age1: 35,
           sex1: "M",
@@ -203,8 +201,7 @@ RSpec.describe "Form Check Answers Page" do
         FactoryBot.create(
           :lettings_log,
           :in_progress,
-          owning_organisation: user.organisation,
+          created_by: user,
-          managing_organisation: user.organisation,
           tenancycode: "123",
           age1: 35,
           sex1: "M",
@@ -218,8 +215,7 @@ RSpec.describe "Form Check Answers Page" do
         FactoryBot.create(
           :lettings_log,
           :in_progress,
-          owning_organisation: user.organisation,
+          created_by: user,
-          managing_organisation: user.organisation,
           tenancycode: "123",
           age1: 35,
           sex1: "M",
@@ -236,8 +232,6 @@ RSpec.describe "Form Check Answers Page" do
         FactoryBot.create(
           :lettings_log,
           :in_progress,
-          owning_organisation: user.organisation,
-          managing_organisation: user.organisation,
           created_by: user,
           needstype: 1,
           tenancycode: nil,

spec/features/form/checkboxes_spec.rb

@@ -9,8 +9,7 @@ RSpec.describe "Checkboxes" do
     FactoryBot.create(
       :lettings_log,
       :in_progress,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
   let(:id) { lettings_log.id }

spec/features/form/conditional_questions_spec.rb

@@ -8,16 +8,14 @@ RSpec.describe "Form Conditional Questions" do
     FactoryBot.create(
       :lettings_log,
       :in_progress,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
   let(:sales_log) do
     FactoryBot.create(
       :sales_log,
       :completed,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
   let(:id) { lettings_log.id }

spec/features/form/page_routing_spec.rb

@@ -8,8 +8,7 @@ RSpec.describe "Form Page Routing" do
     FactoryBot.create(
       :lettings_log,
       :in_progress,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
   let(:id) { lettings_log.id }
@@ -120,6 +119,7 @@ RSpec.describe "Form Page Routing" do
           :lettings_log,
           owning_organisation: user.organisation,
           managing_organisation: user.organisation,
+          created_by: user,
           needstype: 2,
         )
       end

spec/features/form/progressive_total_field_spec.rb

@@ -8,8 +8,7 @@ RSpec.describe "Accessible Automcomplete" do
     FactoryBot.create(
       :lettings_log,
       :in_progress,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
 

spec/features/form/saving_data_spec.rb

@@ -8,8 +8,7 @@ RSpec.describe "Form Saving Data" do
     FactoryBot.create(
       :lettings_log,
       :in_progress,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
   let(:id) { lettings_log.id }
@@ -17,8 +16,7 @@ RSpec.describe "Form Saving Data" do
     FactoryBot.create(
       :lettings_log, :in_progress,
       housingneeds_a: 1,
-      owning_organisation: user.organisation,
+      created_by: user
-      managing_organisation: user.organisation
     )
   end
   let(:question_answers) do

spec/features/form/validations_spec.rb

@@ -8,24 +8,21 @@ RSpec.describe "validations" do
     FactoryBot.create(
       :lettings_log,
       :in_progress,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
       renewal: 0,
     )
   end
   let(:empty_lettings_log) do
     FactoryBot.create(
       :lettings_log,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
   let(:completed_without_declaration) do
     FactoryBot.create(
       :lettings_log,
       :completed,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
       status: 1,
       declaration: nil,
       startdate: Time.zone.local(2021, 5, 1),
@@ -124,8 +121,7 @@ RSpec.describe "validations" do
           :lettings_log,
           :in_progress,
           ecstat1: 1,
-          owning_organisation: user.organisation,
+          created_by: user,
-          managing_organisation: user.organisation,
         )
       end
       let(:income_over_soft_limit) { 750 }

spec/features/organisation_spec.rb

@@ -136,8 +136,8 @@ RSpec.describe "User Features" do
 
     context "when viewing lettings logs for specific organisation" do
       let(:first_log) { organisation.lettings_logs.first }
-      let!(:log_to_search) { FactoryBot.create(:lettings_log, owning_organisation: user.organisation, managing_organisation_id: organisation.id) }
+      let!(:log_to_search) { FactoryBot.create(:lettings_log, created_by: user) }
-      let!(:other_logs) { FactoryBot.create_list(:lettings_log, 4, owning_organisation_id: organisation.id, managing_organisation_id: organisation.id) }
+      let!(:other_logs) { FactoryBot.create_list(:lettings_log, 4, created_by: user) }
       let(:number_of_lettings_logs) { LettingsLog.count }
 
       before do

spec/fixtures/files/lettings_logs_download.csv

@@ -1,2 +1,2 @@
-id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,rent_value_check,old_form_id,lar,irproduct,old_id,joint,illness_type_0,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate
+id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,rent_value_check,old_form_id,lar,irproduct,old_id,joint,illness_type_0,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,updated_by_id,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_managing_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate
-{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,No,DLUHC,DLUHC,2021,Supported housing,,2 October 2021,London Affordable Rent,,,,,,,,,,,,,,,,,,,,No,,,,,No,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,,9,1,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,Bungalow,Fitted with equipment and adaptations,Westminster,{location_startdate}
+{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,No,DLUHC,DLUHC,2021,Supported housing,,2 October 2021,London Affordable Rent,,,,,,,,,,,,,,,,,,,,No,,,,,No,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,,9,1,,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},Missing,No,DLUHC,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,Bungalow,Fitted with equipment and adaptations,Westminster,{location_startdate}

spec/helpers/interruption_screen_helper_spec.rb

@@ -12,8 +12,7 @@ RSpec.describe InterruptionScreenHelper do
       ecstat1: 1,
       earnings: 750,
       incfreq: 1,
-      owning_organisation: user.organisation,
+      created_by: user,
-      managing_organisation: user.organisation,
     )
   end
 

spec/jobs/email_csv_job_spec.rb

@@ -19,8 +19,7 @@ describe EmailCsvJob do
     let!(:lettings_log) do
       FactoryBot.create(
         :lettings_log,
-        owning_organisation: organisation,
+        created_by: user,
-        managing_organisation: organisation,
         ecstat1: 1,
       )
     end
@@ -31,8 +30,6 @@ describe EmailCsvJob do
     before do
       FactoryBot.create(:lettings_log,
                         :completed,
-                        owning_organisation: organisation,
-                        managing_organisation: organisation,
                         created_by: user,
                         startdate: Time.zone.local(2021, 5, 1))
 

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

@@ -71,6 +71,7 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
     let(:lettings_log) do
       create(
         :lettings_log,
+        created_by: user_2,
         owning_organisation: user_2.organisation,
         managing_organisation: user_3.organisation,
       )

spec/models/lettings_log_spec.rb

@@ -2,9 +2,9 @@ require "rails_helper"
 require "shared/shared_examples_for_derived_fields"
 
 RSpec.describe LettingsLog do
-  let(:owning_organisation) { FactoryBot.create(:organisation) }
   let(:different_managing_organisation) { FactoryBot.create(:organisation) }
   let(:created_by_user) { FactoryBot.create(:user) }
+  let(:owning_organisation) { created_by_user.organisation }
   let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") }
 
   before do
@@ -2013,11 +2013,6 @@ RSpec.describe LettingsLog do
       let(:lettings_log) { FactoryBot.create(:lettings_log, created_by: created_by_user) }
       let(:organisation_2) { FactoryBot.create(:organisation) }
 
-      it "clears the created by user set" do
-        expect { lettings_log.update!(owning_organisation: organisation_2) }
-          .to change { lettings_log.reload.created_by }.from(created_by_user).to(nil)
-      end
-
       context "when the organisation selected doesn't match the scheme set" do
         let(:scheme) { FactoryBot.create(:scheme, owning_organisation: created_by_user.organisation) }
         let(:location) { FactoryBot.create(:location, scheme:) }
@@ -2377,10 +2372,10 @@ RSpec.describe LettingsLog do
       let(:organisation_3) { FactoryBot.create(:organisation) }
 
       before do
-        FactoryBot.create(:lettings_log, :in_progress, owning_organisation: organisation_1, managing_organisation: organisation_1)
+        FactoryBot.create(:lettings_log, :in_progress, owning_organisation: organisation_1, managing_organisation: organisation_1, created_by: nil)
-        FactoryBot.create(:lettings_log, :completed, owning_organisation: organisation_1, managing_organisation: organisation_2)
+        FactoryBot.create(:lettings_log, :completed, owning_organisation: organisation_1, managing_organisation: organisation_2, created_by: nil)
-        FactoryBot.create(:lettings_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_1)
+        FactoryBot.create(:lettings_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_1, created_by: nil)
-        FactoryBot.create(:lettings_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_2)
+        FactoryBot.create(:lettings_log, :completed, owning_organisation: organisation_2, managing_organisation: organisation_2, created_by: nil)
       end
 
       it "filters by given organisation id" do

spec/models/organisation_spec.rb

@@ -151,7 +151,6 @@ RSpec.describe Organisation, type: :model do
         FactoryBot.create(
           :lettings_log,
           :completed,
-          owning_organisation: organisation,
           managing_organisation: other_organisation,
           created_by: user,
         )
@@ -159,8 +158,7 @@ RSpec.describe Organisation, type: :model do
       let!(:managed_lettings_log) do
         FactoryBot.create(
           :lettings_log,
-          owning_organisation: other_organisation,
+          created_by: user,
-          managing_organisation: organisation,
         )
       end
 

spec/models/user_spec.rb

@@ -8,7 +8,6 @@ RSpec.describe User, type: :model do
       FactoryBot.create(
         :lettings_log,
         :completed,
-        owning_organisation: user.organisation,
         managing_organisation: other_organisation,
         created_by: user,
       )
@@ -16,8 +15,8 @@ RSpec.describe User, type: :model do
     let!(:managed_lettings_log) do
       FactoryBot.create(
         :lettings_log,
+        created_by: user,
         owning_organisation: other_organisation,
-        managing_organisation: user.organisation,
       )
     end
 

spec/models/validations/setup_validations_spec.rb

@@ -224,4 +224,64 @@ RSpec.describe Validations::SetupValidations do
       end
     end
   end
+
+  describe "#validate_organisation" do
+    let(:user) { FactoryBot.create(:user) }
+    let(:other_organisation) { FactoryBot.create(:organisation) }
+
+    it "validates if neither managing nor owning organisation is the same as created_by user organisation" do
+      record.created_by = user
+      record.owning_organisation = other_organisation
+      record.managing_organisation = other_organisation
+
+      setup_validator.validate_organisation(record)
+      expect(record.errors["created_by"]).to include(I18n.t("validations.setup.created_by.invalid"))
+      expect(record.errors["owning_organisation_id"]).to include(I18n.t("validations.setup.owning_organisation.invalid"))
+      expect(record.errors["managing_organisation_id"]).to include(I18n.t("validations.setup.managing_organisation.invalid"))
+    end
+
+    it "doesn not validate if either managing or owning organisation is the same as current user organisation" do
+      record.created_by = user
+      record.owning_organisation = user.organisation
+      record.managing_organisation = other_organisation
+
+      setup_validator.validate_organisation(record)
+      expect(record.errors["created_by"]).to be_empty
+      expect(record.errors["owning_organisation_id"]).to be_empty
+      expect(record.errors["managing_organisation_id"]).to be_empty
+    end
+
+    it "does not validate if current user is missing" do
+      record.created_by = nil
+      record.owning_organisation = other_organisation
+      record.managing_organisation = other_organisation
+
+      setup_validator.validate_organisation(record)
+      expect(record.errors["created_by"]).to be_empty
+      expect(record.errors["owning_organisation_id"]).to be_empty
+      expect(record.errors["managing_organisation_id"]).to be_empty
+    end
+
+    it "does not validate if managing organisation is missing" do
+      record.created_by = user
+      record.owning_organisation = other_organisation
+      record.managing_organisation = nil
+
+      setup_validator.validate_organisation(record)
+      expect(record.errors["created_by"]).to be_empty
+      expect(record.errors["owning_organisation_id"]).to be_empty
+      expect(record.errors["managing_organisation_id"]).to be_empty
+    end
+
+    it "does not validate if owning organisation is missing" do
+      record.created_by = user
+      record.owning_organisation = nil
+      record.managing_organisation = other_organisation
+
+      setup_validator.validate_organisation(record)
+      expect(record.errors["created_by"]).to be_empty
+      expect(record.errors["owning_organisation_id"]).to be_empty
+      expect(record.errors["managing_organisation_id"]).to be_empty
+    end
+  end
 end

spec/requests/form_controller_spec.rb

@@ -4,12 +4,12 @@ RSpec.describe FormController, type: :request do
   let(:page) { Capybara::Node::Simple.new(response.body) }
   let(:user) { create(:user) }
   let(:organisation) { user.organisation }
-  let(:other_organisation) { create(:organisation) }
+  let(:other_user) { create(:user) }
+  let(:other_organisation) { other_user.organisation }
   let!(:unauthorized_lettings_log) do
     create(
       :lettings_log,
-      owning_organisation: other_organisation,
+      created_by: other_user,
-      managing_organisation: other_organisation,
     )
   end
   let(:setup_complete_lettings_log) do
@@ -18,16 +18,14 @@ RSpec.describe FormController, type: :request do
       :about_completed,
       status: 1,
       startdate: Time.zone.local(2021, 10, 10),
-      owning_organisation: organisation,
+      created_by: user,
-      managing_organisation: organisation,
     )
   end
   let(:completed_lettings_log) do
     create(
       :lettings_log,
       :completed,
-      owning_organisation: organisation,
+      created_by: user,
-      managing_organisation: organisation,
       startdate: Time.zone.local(2021, 5, 1),
     )
   end
@@ -43,8 +41,7 @@ RSpec.describe FormController, type: :request do
     let!(:lettings_log) do
       create(
         :lettings_log,
-        owning_organisation: organisation,
+        created_by: user,
-        managing_organisation: organisation,
       )
     end
 
@@ -68,12 +65,134 @@ RSpec.describe FormController, type: :request do
     end
   end
 
+  context "when signed in as a support user" do
+    let!(:lettings_log) do
+      create(
+        :lettings_log,
+        created_by: user,
+      )
+    end
+    let(:page) { Capybara::Node::Simple.new(response.body) }
+    let(:managing_organisation) { create(:organisation) }
+    let(:managing_organisation_too) { create(:organisation) }
+    let(:housing_provider) { create(:organisation) }
+    let(:support_user) { create(:user, :support) }
+
+    before do
+      organisation.housing_providers << housing_provider
+      organisation.managing_agents << managing_organisation
+      organisation.managing_agents << managing_organisation_too
+      organisation.reload
+      allow(support_user).to receive(:need_two_factor_authentication?).and_return(false)
+      sign_in support_user
+    end
+
+    context "with invalid organisation answers" do
+      let(:params) do
+        {
+          id: lettings_log.id,
+          lettings_log: {
+            page: "managing_organisation",
+            managing_organisation_id: other_organisation.id,
+          },
+        }
+      end
+
+      before do
+        lettings_log.update!(owning_organisation: housing_provider, created_by: user, managing_organisation: organisation)
+        lettings_log.reload
+      end
+
+      it "resets created by and renders the next page" do
+        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by")
+        follow_redirect!
+        lettings_log.reload
+        expect(lettings_log.created_by).to eq(nil)
+      end
+    end
+
+    context "with valid owning organisation" do
+      let(:params) do
+        {
+          id: lettings_log.id,
+          lettings_log: {
+            page: "managing_organisation",
+            managing_organisation_id: other_organisation.id,
+          },
+        }
+      end
+
+      before do
+        lettings_log.update!(owning_organisation: organisation, created_by: user, managing_organisation: organisation)
+        lettings_log.reload
+      end
+
+      it "does not reset created by" do
+        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/created-by")
+        follow_redirect!
+        lettings_log.reload
+        expect(lettings_log.created_by).to eq(user)
+      end
+    end
+
+    context "with valid managing organisation" do
+      let(:params) do
+        {
+          id: lettings_log.id,
+          lettings_log: {
+            page: "housing_provider",
+            owning_organisation_id: housing_provider.id,
+          },
+        }
+      end
+
+      before do
+        lettings_log.update!(owning_organisation: organisation, created_by: user, managing_organisation: organisation)
+        lettings_log.reload
+      end
+
+      it "does not reset created by" do
+        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
+        follow_redirect!
+        lettings_log.reload
+        expect(lettings_log.created_by).to eq(user)
+      end
+    end
+
+    context "with only adding the housing provider" do
+      let(:params) do
+        {
+          id: lettings_log.id,
+          lettings_log: {
+            page: "housing_provider",
+            owning_organisation_id: housing_provider.id,
+          },
+        }
+      end
+
+      before do
+        lettings_log.update!(owning_organisation: nil, created_by: user, managing_organisation: nil)
+        lettings_log.reload
+      end
+
+      it "does not reset created by" do
+        post "/lettings-logs/#{lettings_log.id}/form", params: params
+        expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
+        follow_redirect!
+        lettings_log.reload
+        expect(lettings_log.created_by).to eq(user)
+      end
+    end
+  end
+
   context "when a user is signed in" do
     let!(:lettings_log) do
       create(
         :lettings_log,
-        owning_organisation: organisation,
+        created_by: user,
-        managing_organisation: organisation,
       )
     end
 
@@ -156,8 +275,7 @@ RSpec.describe FormController, type: :request do
             create(
               :lettings_log,
               startdate: Time.zone.local(2022, 12, 1),
-              owning_organisation: organisation,
+              created_by: user,
-              managing_organisation: organisation,
             )
           end
           let(:headers) { { "Accept" => "text/html" } }
@@ -220,12 +338,12 @@ RSpec.describe FormController, type: :request do
     describe "Submit Form" do
       context "with a form page" do
         let(:user) { create(:user) }
+        let(:support_user) { FactoryBot.create(:user, :support) }
         let(:organisation) { user.organisation }
         let(:lettings_log) do
           create(
             :lettings_log,
-            owning_organisation: organisation,
+            created_by: user,
-            managing_organisation: organisation,
           )
         end
         let(:page_id) { "person_1_age" }
@@ -298,6 +416,38 @@ RSpec.describe FormController, type: :request do
           end
         end
 
+        context "with invalid organisation answers" do
+          let(:page) { Capybara::Node::Simple.new(response.body) }
+          let(:managing_organisation) { create(:organisation) }
+          let(:managing_organisation_too) { create(:organisation) }
+          let(:housing_provider) { create(:organisation) }
+          let(:params) do
+            {
+              id: lettings_log.id,
+              lettings_log: {
+                page: "managing_organisation",
+                managing_organisation_id: other_organisation.id,
+              },
+            }
+          end
+
+          before do
+            organisation.housing_providers << housing_provider
+            organisation.managing_agents << managing_organisation
+            organisation.managing_agents << managing_organisation_too
+            organisation.reload
+            lettings_log.update!(owning_organisation: housing_provider, created_by: user, managing_organisation: organisation)
+            lettings_log.reload
+          end
+
+          it "re-renders the same page with errors if validation fails" do
+            post "/lettings-logs/#{lettings_log.id}/form", params: params
+            expect(response).to redirect_to("/lettings-logs/#{lettings_log.id}/managing-organisation")
+            follow_redirect!
+            expect(page).to have_content("There is a problem")
+          end
+        end
+
         context "with valid answers" do
           let(:answer) { 20 }
           let(:params) do
@@ -534,12 +684,11 @@ RSpec.describe FormController, type: :request do
 
       context "with lettings logs that are not owned or managed by your organisation" do
         let(:answer) { 25 }
-        let(:other_organisation) { create(:organisation) }
+        let(:other_user) { create(:user) }
         let(:unauthorized_lettings_log) do
           create(
             :lettings_log,
-            owning_organisation: other_organisation,
+            created_by: other_user,
-            managing_organisation: other_organisation,
           )
         end
 

spec/requests/lettings_logs_controller_spec.rb

@@ -150,20 +150,19 @@ RSpec.describe LettingsLogsController, type: :request do
     let(:page) { Capybara::Node::Simple.new(response.body) }
     let(:user) { FactoryBot.create(:user) }
     let(:organisation) { user.organisation }
-    let(:other_organisation) { FactoryBot.create(:organisation) }
+    let(:other_user) { FactoryBot.create(:user) }
+    let(:other_organisation) { other_user.organisation }
     let!(:lettings_log) do
       FactoryBot.create(
         :lettings_log,
-        owning_organisation: organisation,
+        created_by: user,
-        managing_organisation: organisation,
         tenancycode: "LC783",
       )
     end
     let!(:unauthorized_lettings_log) do
       FactoryBot.create(
         :lettings_log,
-        owning_organisation: other_organisation,
+        created_by: other_user,
-        managing_organisation: other_organisation,
         tenancycode: "UA984",
       )
     end
@@ -266,9 +265,8 @@ RSpec.describe LettingsLogsController, type: :request do
           context "with year filter" do
             let!(:lettings_log_2021) do
               FactoryBot.create(:lettings_log, :in_progress,
-                                owning_organisation: organisation,
+                                created_by: user,
-                                startdate: Time.zone.local(2022, 3, 1),
+                                startdate: Time.zone.local(2022, 3, 1))
-                                managing_organisation: organisation)
             end
             let!(:lettings_log_2022) do
               lettings_log = FactoryBot.build(:lettings_log, :completed,
@@ -567,7 +565,7 @@ RSpec.describe LettingsLogsController, type: :request do
 
         context "when there are more than 20 logs" do
           before do
-            FactoryBot.create_list(:lettings_log, 25, owning_organisation: organisation, managing_organisation: organisation)
+            FactoryBot.create_list(:lettings_log, 25, created_by: user)
           end
 
           context "when on the first page" do
@@ -699,8 +697,7 @@ RSpec.describe LettingsLogsController, type: :request do
               FactoryBot.create(
                 :lettings_log,
                 :conditional_section_complete,
-                owning_organisation: organisation,
+                created_by: user,
-                managing_organisation: organisation,
               )
             end
 
@@ -756,8 +753,7 @@ RSpec.describe LettingsLogsController, type: :request do
     context "when accessing the check answers page" do
       let(:postcode_lettings_log) do
         FactoryBot.create(:lettings_log,
-                          owning_organisation: organisation,
+                          created_by: user,
-                          managing_organisation: organisation,
                           postcode_known: "No")
       end
       let(:id) { postcode_lettings_log.id }
@@ -771,8 +767,7 @@ RSpec.describe LettingsLogsController, type: :request do
 
       it "shows the inferred la" do
         lettings_log = FactoryBot.create(:lettings_log,
-                                         owning_organisation: organisation,
+                                         created_by: user,
-                                         managing_organisation: organisation,
                                          postcode_known: 1,
                                          postcode_full: "PO5 3TE")
         id = lettings_log.id
@@ -1096,8 +1091,7 @@ RSpec.describe LettingsLogsController, type: :request do
       let!(:lettings_log) do
         FactoryBot.create(
           :lettings_log,
-          owning_organisation:,
+          created_by: user,
-          managing_organisation: owning_organisation,
           ecstat1: 1,
         )
       end

spec/requests/organisations_controller_spec.rb

@@ -590,8 +590,8 @@ RSpec.describe OrganisationsController, type: :request do
           let(:number_of_org2_lettings_logs) { 4 }
 
           before do
-            FactoryBot.create_list(:lettings_log, number_of_org1_lettings_logs, owning_organisation_id: organisation.id, managing_organisation_id: organisation.id)
+            FactoryBot.create_list(:lettings_log, number_of_org1_lettings_logs, created_by: user)
-            FactoryBot.create_list(:lettings_log, number_of_org2_lettings_logs, owning_organisation_id: unauthorised_organisation.id, managing_organisation_id: unauthorised_organisation.id)
+            FactoryBot.create_list(:lettings_log, number_of_org2_lettings_logs, created_by: nil, owning_organisation_id: unauthorised_organisation.id, managing_organisation_id: unauthorised_organisation.id)
 
             get "/organisations/#{organisation.id}/lettings-logs", headers:, params: {}
           end

spec/services/csv/lettings_log_csv_service_spec.rb

@@ -201,6 +201,7 @@ RSpec.describe Csv::LettingsLogCsvService do
                                    new_old
                                    vacdays
                                    unresolved
+                                   updated_by_id
                                    unittype_sh
                                    scheme_code
                                    scheme_service_name