CLDC-3186 Display reported by question (#2228)

* Remove feature toggle

* Remove merge organisatiions toggle

* Update managing org routing for 2024

* Update bulk upload

app/components/sales_log_summary_component.html.erb

@@ -32,7 +32,7 @@
               <dd class="app-metadata__definition"><%= log.owning_organisation&.name %></dd>
             </div>
           <% end %>
-          <% if log.managing_organisation && FeatureToggle.sales_managing_organisation_enabled? %>
+          <% if log.managing_organisation %>
             <div class="app-metadata__item">
               <dt class="app-metadata__term">Reported by</dt>
               <dd class="app-metadata__definition"><%= log.managing_organisation&.name %></dd>

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

@@ -14,16 +14,10 @@ class Form::Lettings::Pages::StockOwner < ::Form::Page
     return false unless current_user
     return true if current_user.support?
 
-    stock_owners = if FeatureToggle.merge_organisations_enabled?
+    stock_owners = current_user.organisation.stock_owners + current_user.organisation.absorbed_organisations.where(holds_own_stock: true)
-                     current_user.organisation.stock_owners + current_user.organisation.absorbed_organisations.where(holds_own_stock: true)
-                   else
-                     current_user.organisation.stock_owners
-                   end
 
     if current_user.organisation.holds_own_stock?
-      if FeatureToggle.merge_organisations_enabled? && current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?)
+      return true if current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?)
-        return true
-      end
       return true if stock_owners.count >= 1
 
       log.update!(owning_organisation: current_user.organisation)

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

@@ -67,11 +67,7 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question
   def hidden_in_check_answers?(_log, user = nil)
     return false if user.support?
 
-    stock_owners = if FeatureToggle.merge_organisations_enabled?
+    stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true)
-                     user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true)
-                   else
-                     user.organisation.stock_owners
-                   end
 
     if user.organisation.holds_own_stock?
       stock_owners.count.zero?

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

@@ -12,10 +12,20 @@ class Form::Sales::Pages::ManagingOrganisation < ::Form::Page
 
   def routed_to?(log, current_user)
     return false unless current_user
-    return false unless current_user.support?
-    return false unless FeatureToggle.sales_managing_organisation_enabled?
-    return false unless log.owning_organisation
 
-    log.owning_organisation.managing_agents.count >= 1
+    if form.start_year_after_2024?
+      organisation = current_user.support? ? log.owning_organisation : current_user.organisation
+
+      return false unless organisation
+      return false if log.owning_organisation != organisation && !organisation.holds_own_stock?
+      return true unless organisation.holds_own_stock?
+
+      organisation.managing_agents.count >= 1
+    else
+      return false unless current_user.support?
+      return false unless log.owning_organisation
+
+      log.owning_organisation.managing_agents.count >= 1
+    end
   end
 end

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

@@ -13,19 +13,12 @@ class Form::Sales::Pages::OwningOrganisation < ::Form::Page
   def routed_to?(log, current_user)
     return false unless current_user
     return true if current_user.support?
-    return false unless FeatureToggle.sales_managing_organisation_enabled?
     return true if has_multiple_stock_owners_with_own_stock?(current_user)
 
-    stock_owners = if FeatureToggle.merge_organisations_enabled?
+    stock_owners = current_user.organisation.stock_owners.where(holds_own_stock: true) + current_user.organisation.absorbed_organisations.where(holds_own_stock: true)
-                     current_user.organisation.stock_owners.where(holds_own_stock: true) + current_user.organisation.absorbed_organisations.where(holds_own_stock: true)
-                   else
-                     current_user.organisation.stock_owners.where(holds_own_stock: true)
-                   end
 
     if current_user.organisation.holds_own_stock?
-      if FeatureToggle.merge_organisations_enabled? && current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?)
+      return true if current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?)
-        return true
-      end
       return true if stock_owners.count >= 1
 
       log.update!(owning_organisation: current_user.organisation)

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

@@ -14,7 +14,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
     return answer_opts unless user
     return answer_opts unless log
 
-    if FeatureToggle.sales_managing_organisation_enabled? && !user.support?
+    unless user.support?
       if log.owning_organisation_id.present?
         answer_opts[log.owning_organisation.id] = log.owning_organisation.name
       end
@@ -28,43 +28,36 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
       end
     end
 
-    if FeatureToggle.merge_organisations_enabled?
+    if log.owning_organisation_id.present?
-      if log.owning_organisation_id.present?
+      answer_opts[log.owning_organisation.id] = log.owning_organisation.name
-        answer_opts[log.owning_organisation.id] = log.owning_organisation.name
+    end
-      end
 
-      recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period
+    recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period
-      if !user.support? && user.organisation.holds_own_stock?
+    if !user.support? && user.organisation.holds_own_stock?
-        answer_opts[user.organisation.id] = if recently_absorbed_organisations.exists? && user.organisation.available_from.present?
+      answer_opts[user.organisation.id] = if recently_absorbed_organisations.exists? && user.organisation.available_from.present?
-                                              "#{user.organisation.name} (Your organisation, active as of #{user.organisation.available_from.to_fs(:govuk_date)})"
+                                            "#{user.organisation.name} (Your organisation, active as of #{user.organisation.available_from.to_fs(:govuk_date)})"
-                                            else
+                                          else
-                                              "#{user.organisation.name} (Your organisation)"
+                                            "#{user.organisation.name} (Your organisation)"
-                                            end
+                                          end
-      end
+    end
 
-      if user.support?
+    if user.support?
-        Organisation.where(holds_own_stock: true).find_each do |org|
+      Organisation.where(holds_own_stock: true).find_each do |org|
-          if org.merge_date.present?
+        if org.merge_date.present?
-            answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
+          answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period
-          elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present?
+        elsif org.absorbed_organisations.merged_during_open_collection_period.exists? && org.available_from.present?
-            answer_opts[org.id] = "#{org.name} (active as of #{org.available_from.to_fs(:govuk_date)})"
+          answer_opts[org.id] = "#{org.name} (active as of #{org.available_from.to_fs(:govuk_date)})"
-          else
+        else
-            answer_opts[org.id] = org.name
+          answer_opts[org.id] = org.name
-          end
-        end
-      else
-        recently_absorbed_organisations.each do |absorbed_org|
-          answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock?
         end
       end
-
-      answer_opts
     else
-      Organisation.select(:id, :name).each_with_object(answer_opts) do |organisation, hsh|
+      recently_absorbed_organisations.each do |absorbed_org|
-        hsh[organisation.id] = organisation.name
+        answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock?
-        hsh
       end
     end
+
+    answer_opts
   end
 
   def displayed_answer_options(log, user = nil)
@@ -82,18 +75,14 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
   end
 
   def hidden_in_check_answers?(_log, user = nil)
-    if FeatureToggle.merge_organisations_enabled?
+    return false if user.support?
-      return false if user.support?
 
-      stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true)
+    stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true)
 
-      if user.organisation.holds_own_stock?
+    if user.organisation.holds_own_stock?
-        stock_owners.count.zero?
+      stock_owners.count.zero?
-      else
-        stock_owners.count <= 1
-      end
     else
-      !current_user.support?
+      stock_owners.count <= 1
     end
   end
 
@@ -104,11 +93,7 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question
 private
 
   def selected_answer_option_is_derived?(_log)
-    if FeatureToggle.merge_organisations_enabled?
+    true
-      true
-    else
-      false
-    end
   end
 
   def merged_organisation_label(name, merge_date)

app/services/bulk_upload/sales/year2023/row_parser.rb

@@ -455,12 +455,12 @@ class BulkUpload::Sales::Year2023::RowParser
 
   validate :validate_owning_org_data_given, on: :after_log
   validate :validate_owning_org_exists, on: :after_log
-  validate :validate_owning_org_owns_stock, on: :after_log if FeatureToggle.sales_managing_organisation_enabled?
+  validate :validate_owning_org_owns_stock, on: :after_log
   validate :validate_owning_org_permitted, on: :after_log
 
   validate :validate_created_by_exists, on: :after_log
   validate :validate_created_by_related, on: :after_log
-  validate :validate_managing_org_related, on: :after_log if FeatureToggle.sales_managing_organisation_enabled?
+  validate :validate_managing_org_related, on: :after_log
   validate :validate_relevant_collection_window, on: :after_log
   validate :validate_incomplete_soft_validations, on: :after_log
 

app/services/bulk_upload/sales/year2024/row_parser.rb

@@ -455,12 +455,12 @@ class BulkUpload::Sales::Year2024::RowParser
 
   validate :validate_owning_org_data_given, on: :after_log
   validate :validate_owning_org_exists, on: :after_log
-  validate :validate_owning_org_owns_stock, on: :after_log if FeatureToggle.sales_managing_organisation_enabled?
+  validate :validate_owning_org_owns_stock, on: :after_log
   validate :validate_owning_org_permitted, on: :after_log
 
   validate :validate_created_by_exists, on: :after_log
   validate :validate_created_by_related, on: :after_log
-  validate :validate_managing_org_related, on: :after_log if FeatureToggle.sales_managing_organisation_enabled?
+  validate :validate_managing_org_related, on: :after_log
   validate :validate_relevant_collection_window, on: :after_log
   validate :validate_incomplete_soft_validations, on: :after_log
 
@@ -732,6 +732,7 @@ private
       discount: %i[field_116],
       othtype: %i[field_12],
       owning_organisation_id: %i[field_1],
+      managing_organisation_id: [:field_2],
       created_by: %i[field_3],
       hhregres: %i[field_72],
       hhregresstill: %i[field_73],
@@ -1211,9 +1212,7 @@ private
   end
 
   def managing_organisation
-    return owning_organisation if created_by&.organisation&.absorbed_organisations&.include?(owning_organisation)
+    Organisation.find_by_id_on_multiple_fields(field_2)
-
-    created_by&.organisation || bulk_upload.user.organisation
   end
 
   def nationality_group(nationality_value)
@@ -1228,8 +1227,8 @@ private
     if owning_organisation && managing_organisation && !owning_organisation.can_be_managed_by?(organisation: managing_organisation)
       block_log_creation!
 
-      if errors[:field_3].blank?
+      if errors[:field_2].blank?
-        errors.add(:field_3, "This user belongs to an organisation that does not have a relationship with the owning organisation", category: :setup)
+        errors.add(:field_2, "This organisation does not have a relationship with the owning organisation", category: :setup)
       end
     end
   end

app/services/feature_toggle.rb

@@ -28,10 +28,6 @@ class FeatureToggle
     false
   end
 
-  def self.merge_organisations_enabled?
-    true
-  end
-
   def self.deduplication_flow_enabled?
     true
   end
@@ -47,8 +43,4 @@ class FeatureToggle
   def self.service_moved?
     false
   end
-
-  def self.sales_managing_organisation_enabled?
-    true
-  end
 end

app/views/logs/_log_filters.html.erb

@@ -93,7 +93,7 @@
           } %>
         <% end %>
 
-        <% if (current_user.support? || non_support_with_managing_orgs?) && (user_or_org_lettings_path? || FeatureToggle.sales_managing_organisation_enabled?) %>
+        <% if current_user.support? || non_support_with_managing_orgs? %>
           <%= render partial: "filters/radio_filter", locals: {
             f:,
             options: {

app/views/organisations/show.html.erb

@@ -36,9 +36,7 @@
       <% end %>
       <%= data_sharing_agreement_row(organisation: @organisation, user: current_user, summary_list:) %>
     <% end %>
-    <% if FeatureToggle.merge_organisations_enabled? %>
+    <p>To report a merge or update your organisation details, <%= govuk_link_to "contact the helpdesk", GlobalConstants::HELPDESK_URL %>.</p>
-      <p>To report a merge or update your organisation details, <%= govuk_link_to "contact the helpdesk", GlobalConstants::HELPDESK_URL %>.</p>
-    <% end %>
     <%= render partial: "organisations/merged_organisation_details" %>
   </div>
 </div>

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

@@ -5,9 +5,13 @@ RSpec.describe Form::Sales::Pages::ManagingOrganisation, type: :model do
 
   let(:page_id) { nil }
   let(:page_definition) { nil }
-  let(:subsection) { instance_double(Form::Subsection) }
+  let(:subsection) { instance_double(Form::Subsection, form:) }
   let(:form) { instance_double(Form) }
 
+  before do
+    allow(form).to receive(:start_year_after_2024?).and_return(false)
+  end
+
   it "has correct subsection" do
     expect(page.subsection).to eq(subsection)
   end
@@ -32,8 +36,8 @@ RSpec.describe Form::Sales::Pages::ManagingOrganisation, type: :model do
     expect(page.depends_on).to be nil
   end
 
-  describe "#routed_to?" do
+  describe "#routed_to? with 2023 logs" do
-    let(:log) { create(:lettings_log) }
+    let(:log) { create(:sales_log) }
     let(:organisation) { create(:organisation) }
 
     context "when user nil" do
@@ -54,7 +58,7 @@ RSpec.describe Form::Sales::Pages::ManagingOrganisation, type: :model do
       let(:user) { create(:user, :support) }
 
       context "when owning_organisation not set" do
-        let(:log) { create(:lettings_log, owning_organisation: nil) }
+        let(:log) { create(:sales_log, owning_organisation: nil) }
 
         it "is not shown" do
           expect(page.routed_to?(log, user)).to eq(false)
@@ -103,4 +107,146 @@ RSpec.describe Form::Sales::Pages::ManagingOrganisation, type: :model do
       end
     end
   end
+
+  describe "#routed_to? with 2024 logs" do
+    let(:log) { create(:sales_log) }
+    let(:organisation) { create(:organisation) }
+
+    before do
+      allow(form).to receive(:start_year_after_2024?).and_return(true)
+    end
+
+    context "when user nil" do
+      it "is not shown" do
+        expect(page.routed_to?(log, nil)).to eq(false)
+      end
+    end
+
+    context "when support" do
+      context "when does not hold own stock" do
+        let(:user) do
+          create(:user, :support, organisation: create(:organisation, holds_own_stock: false))
+        end
+        let(:log) { create(:sales_log, owning_organisation: user.organisation) }
+
+        it "is shown" do
+          expect(page.routed_to?(log, user)).to eq(true)
+        end
+      end
+
+      context "when owning_organisation not set" do
+        let(:user) { create(:user, :support) }
+        let(:log) { create(:sales_log, owning_organisation: nil) }
+
+        it "is not shown" do
+          expect(page.routed_to?(log, user)).to eq(false)
+        end
+      end
+
+      context "when holds own stock" do
+        let(:user) do
+          create(:user, :support, organisation: create(:organisation, holds_own_stock: true))
+        end
+
+        context "with 0 managing_agents" do
+          it "is not shown" do
+            expect(page.routed_to?(log, user)).to eq(false)
+          end
+        end
+
+        context "with >1 managing_agents" do
+          before do
+            create(:organisation_relationship, parent_organisation: log.owning_organisation)
+            create(:organisation_relationship, parent_organisation: log.owning_organisation)
+          end
+
+          it "is shown" do
+            expect(page.routed_to?(log, user)).to eq(true)
+          end
+        end
+
+        context "with 1 managing_agents" do
+          let(:managing_agent) { create(:organisation) }
+
+          before do
+            create(
+              :organisation_relationship,
+              child_organisation: managing_agent,
+              parent_organisation: log.owning_organisation,
+            )
+          end
+
+          it "is shown" do
+            expect(page.routed_to?(log, user)).to eq(true)
+          end
+        end
+      end
+    end
+
+    context "when not support" do
+      context "when does not hold own stock" do
+        let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) }
+
+        context "and the user's organisation is selected as owning organisation" do
+          let(:log) { create(:sales_log, owning_organisation: user.organisation) }
+
+          it "is shown" do
+            expect(page.routed_to?(log, user)).to eq(true)
+          end
+        end
+
+        context "and a different than the user's organisation is selected as owning organisation" do
+          let(:stock_owner) { create(:organisation, holds_own_stock: true) }
+          let(:log) { create(:sales_log, owning_organisation: stock_owner) }
+
+          before do
+            create(:organisation_relationship, parent_organisation: stock_owner, child_organisation: user.organisation)
+          end
+
+          it "is not shown" do
+            expect(page.routed_to?(log, user)).to eq(false)
+          end
+        end
+      end
+
+      context "when holds own stock" do
+        let(:user) do
+          create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true))
+        end
+
+        context "with 0 managing_agents" do
+          it "is not shown" do
+            expect(page.routed_to?(log, user)).to eq(false)
+          end
+        end
+
+        context "with >1 managing_agents" do
+          before do
+            create(:organisation_relationship, parent_organisation: user.organisation)
+            create(:organisation_relationship, parent_organisation: user.organisation)
+          end
+
+          it "is shown" do
+            expect(page.routed_to?(log, user)).to eq(true)
+          end
+        end
+
+        context "with 1 managing_agents" do
+          let(:managing_agent) { create(:organisation) }
+
+          before do
+            create(
+              :organisation_relationship,
+              child_organisation: managing_agent,
+              parent_organisation: user.organisation,
+            )
+          end
+
+          it "is shown" do
+            expect(page.routed_to?(log, user)).to eq(true)
+          end
+        end
+      end
+    end
+  end
 end

spec/requests/sales_logs_controller_spec.rb

@@ -161,30 +161,6 @@ RSpec.describe SalesLogsController, type: :request do
               expect(sales_log.managing_organisation.name).to eq("User org")
             end
           end
-
-          context "when the user's org doesn't hold stock and merge_organisations_enabled is false" 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
-              allow(FeatureToggle).to receive(:merge_organisations_enabled?).and_return(false)
-              post "/sales-logs", headers:
-            end
-
-            it "does not set owning organisation" do
-              created_id = response.location.match(/[0-9]+/)[0]
-              sales_log = SalesLog.find_by(id: created_id)
-              expect(sales_log.owning_organisation).to be_nil
-            end
-
-            it "sets managing organisation as the user organisation" do
-              created_id = response.location.match(/[0-9]+/)[0]
-              sales_log = SalesLog.find_by(id: created_id)
-              expect(sales_log.managing_organisation.name).to eq("User org")
-            end
-          end
         end
       end
     end

spec/services/bulk_upload/sales/year2024/row_parser_spec.rb

@@ -9,11 +9,13 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
   let(:bulk_upload) { create(:bulk_upload, :sales, user:, year: 2024) }
   let(:user) { create(:user, organisation: owning_org) }
   let(:owning_org) { create(:organisation, :with_old_visible_id) }
+  let(:managing_org) { create(:organisation, :with_old_visible_id) }
+
   let(:setup_section_params) do
     {
       bulk_upload:,
       field_1: owning_org.old_visible_id, # organisation
-      field_2: owning_org.old_visible_id, # organisation
+      field_2: managing_org.old_visible_id, # organisation
       field_3: user.email, # user
       field_4: now.day.to_s, # sale day
       field_5: now.month.to_s, # sale month
@@ -31,7 +33,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
     {
       bulk_upload:,
       field_1: owning_org.old_visible_id,
-      field_2: owning_org.old_visible_id,
+      field_2: managing_org.old_visible_id,
 
       field_4: "12",
       field_5: "5",
@@ -114,6 +116,8 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
   end
 
   around do |example|
+    create(:organisation_relationship, parent_organisation: owning_org, child_organisation: managing_org)
+
     Timecop.freeze(Time.zone.local(2025, 2, 22)) do
       Singleton.__init__(FormHandler)
       example.run
@@ -287,7 +291,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       it "has errors on correct setup fields" do
         errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute).sort
 
-        expect(errors).to eql(%i[field_1 field_17 field_18 field_4 field_5 field_6 field_8])
+        expect(errors).to eql(%i[field_1 field_17 field_18 field_2 field_4 field_5 field_6 field_8])
       end
     end
 
@@ -303,7 +307,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       it "has errors on correct setup fields" do
         errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute).sort
 
-        expect(errors).to eql(%i[field_1 field_15 field_17 field_18 field_4 field_5 field_6 field_9])
+        expect(errors).to eql(%i[field_1 field_15 field_17 field_18 field_2 field_4 field_5 field_6 field_9])
       end
     end
 
@@ -321,7 +325,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       it "has errors on correct setup fields" do
         errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute).sort
 
-        expect(errors).to eql(%i[field_1 field_16 field_17 field_18 field_4 field_5 field_6])
+        expect(errors).to eql(%i[field_1 field_16 field_17 field_18 field_2 field_4 field_5 field_6])
       end
     end
 
@@ -338,7 +342,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       it "has errors on correct setup fields" do
         errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute).sort
 
-        expect(errors).to eql(%i[field_1 field_10 field_15 field_17 field_18 field_4 field_5 field_6])
+        expect(errors).to eql(%i[field_1 field_10 field_15 field_17 field_18 field_2 field_4 field_5 field_6])
       end
     end
 
@@ -356,7 +360,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       it "has errors on correct setup fields" do
         errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute).sort
 
-        expect(errors).to eql(%i[field_1 field_17 field_18 field_4 field_5 field_6 field_8])
+        expect(errors).to eql(%i[field_1 field_17 field_18 field_2 field_4 field_5 field_6 field_8])
       end
     end
 
@@ -372,7 +376,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       it "has errors on correct setup fields" do
         errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute).sort
 
-        expect(errors).to eql(%i[field_1 field_11 field_13 field_14 field_17 field_18 field_4 field_5 field_6])
+        expect(errors).to eql(%i[field_1 field_11 field_13 field_14 field_17 field_18 field_2 field_4 field_5 field_6])
       end
     end
 
@@ -390,7 +394,7 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       it "has errors on correct setup fields" do
         errors = parser.errors.select { |e| e.options[:category] == :setup }.map(&:attribute).sort
 
-        expect(errors).to eql(%i[field_1 field_12 field_14 field_15 field_17 field_18 field_4 field_5 field_6])
+        expect(errors).to eql(%i[field_1 field_12 field_14 field_15 field_17 field_18 field_2 field_4 field_5 field_6])
       end
     end
 
@@ -1446,39 +1450,55 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
       let(:attributes) { setup_section_params }
 
       context "when user is part of the owning organisation" do
-        it "sets managing organisation to the users organisation" do
+        it "sets managing organisation to the correct organisation" do
           parser.valid?
           expect(parser.log.owning_organisation_id).to be(owning_org.id)
-          expect(parser.log.managing_organisation_id).to be(owning_org.id)
+          expect(parser.log.managing_organisation_id).to be(managing_org.id)
         end
       end
 
-      context "when user is part of an organisation affiliated with owning org" do
+      context "when blank" do
-        let(:managing_agent) { create(:organisation) }
+        let(:attributes) { { bulk_upload:, field_2: "" } }
-        let(:user) { create(:user, organisation: managing_agent) }
-        let(:attributes) { setup_section_params }
 
-        before do
+        it "is not permitted as setup error" do
-          create(:organisation_relationship, child_organisation: managing_agent, parent_organisation: owning_org)
+          parser.valid?
+          setup_errors = parser.errors.select { |e| e.options[:category] == :setup }
+
+          expect(setup_errors.find { |e| e.attribute == :field_2 }.message).to eql("You must answer reported by")
         end
 
+        it "blocks log creation" do
+          parser.valid?
+          expect(parser).to be_block_log_creation
+        end
+      end
+
+      context "when cannot find managing org" do
+        let(:attributes) { { bulk_upload:, field_2: "donotexist" } }
+
         it "is not permitted as setup error" do
           parser.valid?
-          expect(parser.log.owning_organisation_id).to be(owning_org.id)
+          setup_errors = parser.errors.select { |e| e.options[:category] == :setup }
-          expect(parser.log.managing_organisation_id).to be(managing_agent.id)
+
+          expect(setup_errors.find { |e| e.attribute == :field_2 }.message).to eql("You must answer reported by")
+        end
+
+        it "blocks log creation" do
+          parser.valid?
+          expect(parser).to be_block_log_creation
         end
       end
 
-      context "when user is part of an organisation not affiliated with owning org" do
+      context "when not affiliated with managing org" do
-        let(:unaffiliated_org) { create(:organisation) }
+        let(:unaffiliated_org) { create(:organisation, :with_old_visible_id) }
-        let(:user) { create(:user, organisation: unaffiliated_org) }
+
-        let(:attributes) { setup_section_params }
+        let(:attributes) { { bulk_upload:, field_1: owning_org.old_visible_id, field_2: unaffiliated_org.old_visible_id } }
 
         it "is not permitted as setup error" do
           parser.valid?
           setup_errors = parser.errors.select { |e| e.options[:category] == :setup }
 
-          expect(setup_errors.find { |e| e.attribute == :field_3 }.message).to eql("This user belongs to an organisation that does not have a relationship with the owning organisation")
+          expect(setup_errors.find { |e| e.attribute == :field_2 }.message).to eql("This organisation does not have a relationship with the owning organisation")
         end
 
         it "blocks log creation" do