From f9ac59b860abee84b0aad431ab5985e8c5ac4123 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:13:02 +0000 Subject: [PATCH] CLDC-3186 Display reported by question (#2228) * Remove feature toggle * Remove merge organisatiions toggle * Update managing org routing for 2024 * Update bulk upload --- .../sales_log_summary_component.html.erb | 2 +- app/models/form/lettings/pages/stock_owner.rb | 10 +- .../form/lettings/questions/stock_owner.rb | 6 +- .../form/sales/pages/managing_organisation.rb | 18 +- .../form/sales/pages/owning_organisation.rb | 11 +- .../sales/questions/owning_organisation_id.rb | 75 ++++----- .../bulk_upload/sales/year2023/row_parser.rb | 4 +- .../bulk_upload/sales/year2024/row_parser.rb | 13 +- app/services/feature_toggle.rb | 8 - app/views/logs/_log_filters.html.erb | 2 +- app/views/organisations/show.html.erb | 4 +- .../sales/pages/managing_organisation_spec.rb | 154 +++++++++++++++++- spec/requests/sales_logs_controller_spec.rb | 24 --- .../sales/year2024/row_parser_spec.rb | 68 +++++--- 14 files changed, 254 insertions(+), 145 deletions(-) diff --git a/app/components/sales_log_summary_component.html.erb b/app/components/sales_log_summary_component.html.erb index 82acc6e14..5c0668301 100644 --- a/app/components/sales_log_summary_component.html.erb +++ b/app/components/sales_log_summary_component.html.erb @@ -32,7 +32,7 @@
<%= log.owning_organisation&.name %>
<% end %> - <% if log.managing_organisation && FeatureToggle.sales_managing_organisation_enabled? %> + <% if log.managing_organisation %>
Reported by
<%= log.managing_organisation&.name %>
diff --git a/app/models/form/lettings/pages/stock_owner.rb b/app/models/form/lettings/pages/stock_owner.rb index 07fe01327..a653e7fe2 100644 --- a/app/models/form/lettings/pages/stock_owner.rb +++ b/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? - current_user.organisation.stock_owners + current_user.organisation.absorbed_organisations.where(holds_own_stock: true) - else - current_user.organisation.stock_owners - end + stock_owners = current_user.organisation.stock_owners + current_user.organisation.absorbed_organisations.where(holds_own_stock: true) if current_user.organisation.holds_own_stock? - if FeatureToggle.merge_organisations_enabled? && current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?) - return true - end + return true if current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?) return true if stock_owners.count >= 1 log.update!(owning_organisation: current_user.organisation) diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb index c78c4a080..6b5e77c6d 100644 --- a/app/models/form/lettings/questions/stock_owner.rb +++ b/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? - user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true) - else - user.organisation.stock_owners - end + stock_owners = user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true) if user.organisation.holds_own_stock? stock_owners.count.zero? diff --git a/app/models/form/sales/pages/managing_organisation.rb b/app/models/form/sales/pages/managing_organisation.rb index 0c9ed7337..3d8e59383 100644 --- a/app/models/form/sales/pages/managing_organisation.rb +++ b/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 diff --git a/app/models/form/sales/pages/owning_organisation.rb b/app/models/form/sales/pages/owning_organisation.rb index d8787a456..f0c9e4e68 100644 --- a/app/models/form/sales/pages/owning_organisation.rb +++ b/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? - 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 + stock_owners = current_user.organisation.stock_owners.where(holds_own_stock: true) + current_user.organisation.absorbed_organisations.where(holds_own_stock: true) if current_user.organisation.holds_own_stock? - if FeatureToggle.merge_organisations_enabled? && current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?) - return true - end + return true if current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?) return true if stock_owners.count >= 1 log.update!(owning_organisation: current_user.organisation) diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index 47a10993d..a4c700e05 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/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? - answer_opts[log.owning_organisation.id] = log.owning_organisation.name - end + if log.owning_organisation_id.present? + answer_opts[log.owning_organisation.id] = log.owning_organisation.name + end - recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period - if !user.support? && user.organisation.holds_own_stock? - answer_opts[user.organisation.id] = 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)})" - else - "#{user.organisation.name} (Your organisation)" - end - end + recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period + if !user.support? && user.organisation.holds_own_stock? + answer_opts[user.organisation.id] = 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)})" + else + "#{user.organisation.name} (Your organisation)" + end + end - if user.support? - Organisation.where(holds_own_stock: true).find_each do |org| - 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 - 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)})" - else - answer_opts[org.id] = org.name - end - end - else - recently_absorbed_organisations.each do |absorbed_org| - answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock? + if user.support? + Organisation.where(holds_own_stock: true).find_each do |org| + 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 + 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)})" + else + answer_opts[org.id] = org.name end end - - answer_opts else - Organisation.select(:id, :name).each_with_object(answer_opts) do |organisation, hsh| - hsh[organisation.id] = organisation.name - hsh + 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 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? - stock_owners.count.zero? - else - stock_owners.count <= 1 - end + if user.organisation.holds_own_stock? + stock_owners.count.zero? 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 - else - false - end + true end def merged_organisation_label(name, merge_date) diff --git a/app/services/bulk_upload/sales/year2023/row_parser.rb b/app/services/bulk_upload/sales/year2023/row_parser.rb index 93f15395f..0759d7845 100644 --- a/app/services/bulk_upload/sales/year2023/row_parser.rb +++ b/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 diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index 80748c0bc..4e0f689a4 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/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) - - created_by&.organisation || bulk_upload.user.organisation + Organisation.find_by_id_on_multiple_fields(field_2) 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? - errors.add(:field_3, "This user belongs to an organisation that does not have a relationship with the owning organisation", category: :setup) + if errors[:field_2].blank? + errors.add(:field_2, "This organisation does not have a relationship with the owning organisation", category: :setup) end end end diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb index 38c1372e1..e00e89b90 100644 --- a/app/services/feature_toggle.rb +++ b/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 diff --git a/app/views/logs/_log_filters.html.erb b/app/views/logs/_log_filters.html.erb index 16053c805..ea7496bdd 100644 --- a/app/views/logs/_log_filters.html.erb +++ b/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: { diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 21f722920..e2e07e28c 100644 --- a/app/views/organisations/show.html.erb +++ b/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? %> -

To report a merge or update your organisation details, <%= govuk_link_to "contact the helpdesk", GlobalConstants::HELPDESK_URL %>.

- <% end %> +

To report a merge or update your organisation details, <%= govuk_link_to "contact the helpdesk", GlobalConstants::HELPDESK_URL %>.

<%= render partial: "organisations/merged_organisation_details" %>
diff --git a/spec/models/form/sales/pages/managing_organisation_spec.rb b/spec/models/form/sales/pages/managing_organisation_spec.rb index 0a34554cb..8c411401d 100644 --- a/spec/models/form/sales/pages/managing_organisation_spec.rb +++ b/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 - let(:log) { create(:lettings_log) } + describe "#routed_to? with 2023 logs" do + 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 diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index a9bf07883..6ad991934 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/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 diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb index 2656ff1c0..8866d3de2 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/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 - let(:managing_agent) { create(:organisation) } - let(:user) { create(:user, organisation: managing_agent) } - let(:attributes) { setup_section_params } + context "when blank" do + let(:attributes) { { bulk_upload:, field_2: "" } } - before do - create(:organisation_relationship, child_organisation: managing_agent, parent_organisation: owning_org) + 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_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) - expect(parser.log.managing_organisation_id).to be(managing_agent.id) + 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 user is part of an organisation not affiliated with owning org" do - let(:unaffiliated_org) { create(:organisation) } - let(:user) { create(:user, organisation: unaffiliated_org) } - let(:attributes) { setup_section_params } + context "when not affiliated with managing org" do + let(:unaffiliated_org) { create(:organisation, :with_old_visible_id) } + + 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