From fb389414a0449e7e7698dd6f4d8ed15f29d008af Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 3 Aug 2023 10:24:57 +0100 Subject: [PATCH] CLDC-2504 Add owning org select to sales if there are stock owning absorbed orgs (#1815) * feat: wip update sales org select * feat: update routing and hidden in check answers methods * feat: set sales org id as derived * feat: update tests * refactor: linting * feat: update tests and radio options * feat: update test * feat: add value helper test for non-constant value question * refactor: lint * feat: freeze time in unrelated test * refactor: linting --- app/models/form/lettings/pages/stock_owner.rb | 5 +- .../form/lettings/questions/stock_owner.rb | 2 +- app/models/form/question.rb | 27 ++-- app/models/form/sales/pages/organisation.rb | 23 +++- .../sales/questions/owning_organisation_id.rb | 50 +++++-- .../form/sales/questions/postcode_known.rb | 1 + .../completed_2022_23_sales_bulk_upload.csv | 2 +- spec/models/form/question_spec.rb | 52 +++++--- .../form/sales/pages/organisation_spec.rb | 126 ++++++++++++++++-- .../questions/owning_organisation_id_spec.rb | 90 ++++++++++++- .../forms/bulk_upload_sales/year_spec.rb | 6 + spec/requests/sales_logs_controller_spec.rb | 2 + .../bulk_upload/sales/validator_spec.rb | 4 +- .../sales/year2022/row_parser_spec.rb | 4 +- .../sales/year2023/row_parser_spec.rb | 4 +- 15 files changed, 337 insertions(+), 61 deletions(-) diff --git a/app/models/form/lettings/pages/stock_owner.rb b/app/models/form/lettings/pages/stock_owner.rb index d92824f0e..dee5d5f71 100644 --- a/app/models/form/lettings/pages/stock_owner.rb +++ b/app/models/form/lettings/pages/stock_owner.rb @@ -14,9 +14,12 @@ class Form::Lettings::Pages::StockOwner < ::Form::Page return false unless current_user return true if current_user.support? - stock_owners = current_user.organisation.stock_owners + 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 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) diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb index dc6179b21..ef01e6538 100644 --- a/app/models/form/lettings/questions/stock_owner.rb +++ b/app/models/form/lettings/questions/stock_owner.rb @@ -49,7 +49,7 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question def hidden_in_check_answers?(_log, user = nil) return false if user.support? - stock_owners = user.organisation.stock_owners + 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/question.rb b/app/models/form/question.rb index 2f09a061d..c27dca9c2 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -116,20 +116,20 @@ class Form::Question if is_derived_or_has_inferred_check_answers_value?(log) "Change" elsif type == "checkbox" - answer_options.keys.any? { |key| value_is_yes?(log[key]) } ? "Change" : "Answer" + answer_options.keys.any? { |key| value_is_yes?(log[key], log.lettings?) } ? "Change" : "Answer" else log[id].blank? ? "Answer" : "Change" end end def unanswered?(log) - return answer_options.keys.none? { |key| value_is_yes?(log[key]) } if type == "checkbox" + return answer_options.keys.none? { |key| value_is_yes?(log[key], log.lettings?) } if type == "checkbox" log[id].blank? end def completed?(log) - return answer_options.keys.any? { |key| value_is_yes?(log[key]) } if type == "checkbox" + return answer_options.keys.any? { |key| value_is_yes?(log[key], log.lettings?) } if type == "checkbox" log[id].present? || !log.respond_to?(id.to_sym) || has_inferred_display_value?(log) end @@ -166,23 +166,23 @@ class Form::Question label || value.to_s end - def value_is_yes?(value) + def value_is_yes?(value, is_lettings) case type when "checkbox" value == 1 when "radio" - RADIO_YES_VALUE[id.to_sym]&.include?(value) + is_lettings ? RADIO_YES_VALUE_LETTINGS[id.to_sym]&.include?(value) : RADIO_YES_VALUE_SALES[id.to_sym]&.include?(value) else %w[yes].include?(value.downcase) end end - def value_is_no?(value) + def value_is_no?(value, is_lettings) case type when "checkbox" value && value.zero? when "radio" - RADIO_NO_VALUE[id.to_sym]&.include?(value) + is_lettings ? RADIO_NO_VALUE_LETTINGS[id.to_sym]&.include?(value) : RADIO_NO_VALUE_SALES[id.to_sym]&.include?(value) else %w[no].include?(value.downcase) end @@ -272,9 +272,9 @@ private def checkbox_answer_label(log) answer = [] - return "Yes" if id == "declaration" && value_is_yes?(log["declaration"]) + return "Yes" if id == "declaration" && value_is_yes?(log["declaration"], log.lettings?) - answer_options.each { |key, options| value_is_yes?(log[key]) ? answer << options["value"] : nil } + answer_options.each { |key, options| value_is_yes?(log[key], log.lettings?) ? answer << options["value"] : nil } answer.join(", ") end @@ -319,7 +319,7 @@ private RADIO_YES_VALUE = { renewal: [1], postcode_known: [1], - ppcodenk: [1], + pcodenk: [0], previous_la_known: [1], first_time_property_let_as_social_housing: [1], wchair: [1], @@ -343,7 +343,7 @@ private RADIO_NO_VALUE = { renewal: [0], postcode_known: [0], - ppcodenk: [0], + pcodenk: [1], previous_la_known: [0], first_time_property_let_as_social_housing: [0], wchair: [0], @@ -364,6 +364,11 @@ private net_income_value_check: [1], }.freeze + RADIO_YES_VALUE_LETTINGS = RADIO_YES_VALUE.merge({ ppcodenk: [1] }) + RADIO_YES_VALUE_SALES = RADIO_YES_VALUE.merge({ ppcodenk: [0] }) + RADIO_NO_VALUE_LETTINGS = RADIO_NO_VALUE.merge({ ppcodenk: [0] }) + RADIO_NO_VALUE_SALES = RADIO_NO_VALUE.merge({ ppcodenk: [1] }) + RADIO_DONT_KNOW_VALUE = { sheltered: [3], underoccupation_benefitcap: [3], diff --git a/app/models/form/sales/pages/organisation.rb b/app/models/form/sales/pages/organisation.rb index 1d61b86ac..8f6728821 100644 --- a/app/models/form/sales/pages/organisation.rb +++ b/app/models/form/sales/pages/organisation.rb @@ -10,7 +10,26 @@ class Form::Sales::Pages::Organisation < ::Form::Page ] end - def routed_to?(_log, current_user) - !!current_user&.support? + def routed_to?(log, current_user) + return false unless current_user + return true if current_user.support? + + 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 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) + else + return false if stock_owners.count.zero? + return true if stock_owners.count > 1 + + log.update!(owning_organisation: stock_owners.first) + end + + false end end diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index fa838f744..0db5ec1a0 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -7,37 +7,63 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question @type = "select" end - def answer_options + def answer_options(log = nil, user = nil) answer_opts = { "" => "Select an option" } + return answer_opts unless ActiveRecord::Base.connected? + return answer_opts unless user + return answer_opts unless log + + if log.owning_organisation_id.present? + answer_opts = answer_opts.merge({ log.owning_organisation.id => log.owning_organisation.name }) + end - Organisation.select(:id, :name).each_with_object(answer_opts) do |organisation, hsh| - hsh[organisation.id] = organisation.name - hsh + if !user.support? && user.organisation.holds_own_stock? + answer_opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)" end + + user_answer_options = if user.support? + Organisation.where(holds_own_stock: true) + else + user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true) + end.pluck(:id, :name).to_h + + answer_opts.merge(user_answer_options) end - def displayed_answer_options(_log, _user = nil) - answer_options + def displayed_answer_options(log, user = nil) + answer_options(log, user) end - def label_from_value(value, _log = nil, _user = nil) + def label_from_value(value, log = nil, user = nil) return unless value - answer_options[value] + answer_options(log, user)[value] + end + + def derived? + true end - def hidden_in_check_answers?(_log, current_user) - !current_user.support? + def hidden_in_check_answers?(_log, user = nil) + return false if user.support? + + 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 end - def derived? + def enabled true end private def selected_answer_option_is_derived?(_log) - false + true end end diff --git a/app/models/form/sales/questions/postcode_known.rb b/app/models/form/sales/questions/postcode_known.rb index 208f8df22..10a3e2765 100644 --- a/app/models/form/sales/questions/postcode_known.rb +++ b/app/models/form/sales/questions/postcode_known.rb @@ -19,6 +19,7 @@ class Form::Sales::Questions::PostcodeKnown < ::Form::Question }, ], } + @disable_clearing_if_not_routed_or_dynamic_answer_options = true end ANSWER_OPTIONS = { diff --git a/spec/fixtures/files/completed_2022_23_sales_bulk_upload.csv b/spec/fixtures/files/completed_2022_23_sales_bulk_upload.csv index 807a58710..1eb4a01f9 100644 --- a/spec/fixtures/files/completed_2022_23_sales_bulk_upload.csv +++ b/spec/fixtures/files/completed_2022_23_sales_bulk_upload.csv @@ -116,4 +116,4 @@ OR If field 39 = 3 - 9",If field 113 = 2 or 3,If field 113 = 1 or 3,If field 113 = 1 or 2 Bulk upload format and duplicate check,Yes,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, Field number,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100,101,102,103,104,105,106,107,108,109,110,111,112,113,114,115,116,117,118,119,120,121,122,123,124,125 -,22 test BU,22,2,23,,1,32,32,,,,,M,F,,,,,R,,,,,1,2,,,,,12,18,30000,15000,1,1,20000,3,,1,E09000008,A1,1AA,1,,1,1,,3,3,2,1,1,E09000008,CR0,4BB,3,2,2,23,3,22,30,3,22,3,1,1,250000,25,42500,3,20000,,800,200,,,,,,,,,,,,,,,,,3,,,3,,5,1,,,,,,4,20,,,,2,5,1,1,1,,1,1,1,1,0,10,10,1,1,, +,22 test BU,22,2,23,,1,32,32,,,,,M,F,,,,,R,,,,,1,2,,,,,12,18,30000,15000,1,1,20000,3,,1,E09000008,A1,1AA,1,,1,1,,3,3,2,1,1,E09000008,CR0,4BB,3,2,2,23,3,22,30,3,22,3,1,1,250000,25,42500,3,20000,,800,200,,,,,,,,,,,,,,,,,123,,,3,,5,1,,,,,,4,20,,,,2,5,1,1,1,,1,1,1,1,0,10,10,1,1,, diff --git a/spec/models/form/question_spec.rb b/spec/models/form/question_spec.rb index 751523d37..08d08032f 100644 --- a/spec/models/form/question_spec.rb +++ b/spec/models/form/question_spec.rb @@ -60,15 +60,37 @@ RSpec.describe Form::Question, type: :model do end it "has a yes value helper" do - expect(question).to be_value_is_yes("Yes") - expect(question).to be_value_is_yes("YES") - expect(question).not_to be_value_is_yes("random") + expect(question).to be_value_is_yes("Yes", true) + expect(question).to be_value_is_yes("YES", true) + expect(question).not_to be_value_is_yes("random", true) end it "has a no value helper" do - expect(question).to be_value_is_no("No") - expect(question).to be_value_is_no("NO") - expect(question).not_to be_value_is_no("random") + expect(question).to be_value_is_no("No", true) + expect(question).to be_value_is_no("NO", true) + expect(question).not_to be_value_is_no("random", true) + end + + context "when there are different value helper values for lettings and sales" do + context "with a lettings log" do + let(:lettings_log) { FactoryBot.build(:lettings_log, :in_progress) } + let(:question) { Form::Lettings::Questions::Ppcodenk.new(nil, nil, Form::Lettings::Pages::PreviousPostcode.new("previous_postcode", nil, Form::Lettings::Subsections::HouseholdSituation.new(nil, nil, Form::Lettings::Sections::Household))) } + + it "has the correct values" do + expect(question.value_is_yes?(1, lettings_log.lettings?)).to be true + expect(question.value_is_no?(0, lettings_log.lettings?)).to be true + end + end + + context "with a sales log" do + let(:sales_log) { FactoryBot.build(:sales_log, :in_progress) } + let(:question) { Form::Sales::Questions::PreviousPostcodeKnown.new(nil, nil, Form::Sales::Pages::LastAccommodation.new("previous_postcode", nil, Form::Sales::Subsections::HouseholdSituation.new(nil, nil, Form::Sales::Sections::Household))) } + + it "has the correct values" do + expect(question.value_is_yes?(0, sales_log.lettings?)).to be true + expect(question.value_is_no?(1, sales_log.lettings?)).to be true + end + end end context "when type is numeric" do @@ -108,10 +130,10 @@ RSpec.describe Form::Question, type: :model do let(:question_id) { "illness" } it "maps those options" do - expect(question).to be_value_is_yes(1) - expect(question).not_to be_value_is_no(1) + expect(question).to be_value_is_yes(1, true) + expect(question).not_to be_value_is_no(1, true) expect(question).not_to be_value_is_refused(1) - expect(question).to be_value_is_no(2) + expect(question).to be_value_is_no(2, true) expect(question).to be_value_is_dont_know(3) end end @@ -123,8 +145,8 @@ RSpec.describe Form::Question, type: :model do let(:question_id) { "layear" } it "maps those options" do - expect(question).not_to be_value_is_yes(7) - expect(question).not_to be_value_is_no(7) + expect(question).not_to be_value_is_yes(7, true) + expect(question).not_to be_value_is_no(7, true) expect(question).not_to be_value_is_refused(7) expect(question).to be_value_is_dont_know(7) end @@ -209,13 +231,13 @@ RSpec.describe Form::Question, type: :model do end it "can map yes values" do - expect(question).to be_value_is_yes(1) - expect(question).not_to be_value_is_yes(0) + expect(question).to be_value_is_yes(1, true) + expect(question).not_to be_value_is_yes(0, true) end it "can map no values" do - expect(question).to be_value_is_no(0) - expect(question).not_to be_value_is_no(1) + expect(question).to be_value_is_no(0, true) + expect(question).not_to be_value_is_no(1, true) end end diff --git a/spec/models/form/sales/pages/organisation_spec.rb b/spec/models/form/sales/pages/organisation_spec.rb index 62156031f..710edbecc 100644 --- a/spec/models/form/sales/pages/organisation_spec.rb +++ b/spec/models/form/sales/pages/organisation_spec.rb @@ -7,7 +7,6 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do let(:page_definition) { nil } let(:subsection) { instance_double(Form::Subsection) } let(:form) { instance_double(Form) } - let(:lettings_log) { instance_double(LettingsLog) } it "has correct subsection" do expect(page.subsection).to eq(subsection) @@ -33,19 +32,126 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do expect(page.depends_on).to be nil end - context "when the current user is a support user" do - let(:support_user) { FactoryBot.build(:user, :support) } + describe "#routed_to?" do + let(:log) { create(:lettings_log, owning_organisation_id: nil) } - it "is shown" do - expect(page.routed_to?(lettings_log, support_user)).to be true + context "when user nil" do + it "is not shown" do + expect(page.routed_to?(log, nil)).to eq(false) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, nil) }.not_to change(log.reload, :owning_organisation).from(nil) + end end - end - context "when the current user is not a support user" do - let(:user) { FactoryBot.build(:user) } + context "when support" do + let(:user) { create(:user, :support) } + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil) + end + end + + context "when not support" do + context "when does not hold own stock" do + let(:user) do + create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) + end + + context "with 0 stock_owners" do + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation) + end + end + + context "with 1 stock_owners" do + let(:stock_owner) { create(:organisation) } + + before do + create( + :organisation_relationship, + child_organisation: user.organisation, + parent_organisation: stock_owner, + ) + end + + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + + it "updates owning_organisation_id" do + expect { page.routed_to?(log, user) }.to change(log.reload, :owning_organisation).from(nil).to(stock_owner) + end + end + + context "with >1 stock_owners" do + let(:stock_owner1) { create(:organisation) } + let(:stock_owner2) { create(:organisation) } + + before do + create( + :organisation_relationship, + child_organisation: user.organisation, + parent_organisation: stock_owner1, + ) + create( + :organisation_relationship, + child_organisation: user.organisation, + parent_organisation: stock_owner2, + ) + end + + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + + it "updates owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation) + 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 stock_owners" do + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + + it "updates owning_organisation_id to user organisation" do + expect { + page.routed_to?(log, user) + }.to change(log.reload, :owning_organisation).from(nil).to(user.organisation) + end + end + + context "with >0 stock_owners" do + before do + create(:organisation_relationship, child_organisation: user.organisation) + create(:organisation_relationship, child_organisation: user.organisation) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end - it "is not shown" do - expect(page.routed_to?(lettings_log, user)).to be false + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil) + end + end + end end end end diff --git a/spec/models/form/sales/questions/owning_organisation_id_spec.rb b/spec/models/form/sales/questions/owning_organisation_id_spec.rb index f6dc02c27..815a42319 100644 --- a/spec/models/form/sales/questions/owning_organisation_id_spec.rb +++ b/spec/models/form/sales/questions/owning_organisation_id_spec.rb @@ -3,6 +3,7 @@ require "rails_helper" RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do subject(:question) { described_class.new(question_id, question_definition, page) } + let(:user) { FactoryBot.create(:user, :data_coordinator) } let(:question_id) { nil } let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } @@ -43,8 +44,93 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do expect(question.hint_text).to be_nil end - it "has the correct answer options" do - expect(question.answer_options).to eq(expected_answer_options) + describe "answer options" do + let(:options) { { "" => "Select an option" } } + + context "when current_user nil" do + it "shows default options" do + expect(question.answer_options).to eq(options) + end + end + + context "when user is not support" do + let(:user_org) { create(:organisation, name: "User org") } + let(:user) { create(:user, :data_coordinator, organisation: user_org) } + + let(:owning_org_1) { create(:organisation, name: "Owning org 1") } + let(:owning_org_2) { create(:organisation, name: "Owning org 2") } + let!(:org_rel) do + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2) + end + let(:log) { create(:lettings_log, owning_organisation: owning_org_1) } + + context "when user's org owns stock" do + let(:options) do + { + "" => "Select an option", + owning_org_1.id => "Owning org 1", + user.organisation.id => "User org (Your organisation)", + owning_org_2.id => "Owning org 2", + } + end + + it "shows current stock owner at top, followed by user's org (with hint), followed by the stock owners of the user's org" do + user.organisation.update!(holds_own_stock: true) + expect(question.displayed_answer_options(log, user)).to eq(options) + end + + context "when the owning-managing organisation relationship is deleted" do + let(:options) do + { + "" => "Select an option", + user.organisation.id => "User org (Your organisation)", + owning_org_2.id => "Owning org 2", + } + end + + it "doesn't remove the housing provider from the list of allowed housing providers" do + log.update!(owning_organisation: owning_org_2) + expect(question.displayed_answer_options(log, user)).to eq(options) + org_rel.destroy! + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + end + + context "when user's org doesn't own stock" do + let(:options) do + { + "" => "Select an option", + owning_org_1.id => "Owning org 1", + owning_org_2.id => "Owning org 2", + } + end + + it "shows current stock owner at top, followed by the stock owners of the user's org" do + user.organisation.update!(holds_own_stock: false) + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + end + + context "when user is support" do + let(:user) { create(:user, :support) } + + let(:log) { create(:lettings_log) } + + let(:non_stock_organisation) { create(:organisation, holds_own_stock: false) } + let(:expected_opts) do + Organisation.where(holds_own_stock: true).each_with_object(options) do |organisation, hsh| + hsh[organisation.id] = organisation.name + hsh + end + end + + it "shows orgs where organisation holds own stock" do + expect(question.displayed_answer_options(log, user)).to eq(expected_opts) + expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id) + end + end end it "is marked as derived" do diff --git a/spec/models/forms/bulk_upload_sales/year_spec.rb b/spec/models/forms/bulk_upload_sales/year_spec.rb index 6509643ce..e8f2a04e5 100644 --- a/spec/models/forms/bulk_upload_sales/year_spec.rb +++ b/spec/models/forms/bulk_upload_sales/year_spec.rb @@ -3,6 +3,12 @@ require "rails_helper" RSpec.describe Forms::BulkUploadSales::Year do subject(:form) { described_class.new } + around do |example| + Timecop.freeze(Time.zone.now) do + example.run + end + end + describe "#options" do it "returns correct years" do expect(form.options.map(&:id)).to eql([2023, 2022]) diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 41e3bb1d2..abe10af8e 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -65,6 +65,8 @@ RSpec.describe SalesLogsController, type: :request do context "with a request containing invalid json parameters" do let(:params) do { + "owning_organisation_id": owning_organisation.id, + "created_by_id": user.id, "saledate": Time.zone.today, "purchid": "1", "ownershipsch": 1, diff --git a/spec/services/bulk_upload/sales/validator_spec.rb b/spec/services/bulk_upload/sales/validator_spec.rb index 165abf8df..730a2b643 100644 --- a/spec/services/bulk_upload/sales/validator_spec.rb +++ b/spec/services/bulk_upload/sales/validator_spec.rb @@ -4,7 +4,7 @@ RSpec.describe BulkUpload::Sales::Validator do subject(:validator) { described_class.new(bulk_upload:, path:) } let(:user) { create(:user, organisation:) } - let(:organisation) { create(:organisation, old_visible_id: "3") } + let(:organisation) { create(:organisation, old_visible_id: "123") } let(:bulk_upload) { create(:bulk_upload, user:) } let(:path) { file.path } let(:file) { Tempfile.new } @@ -104,7 +104,7 @@ RSpec.describe BulkUpload::Sales::Validator do error = BulkUploadError.find_by(row: "6", field: "field_92", category: "setup") expect(error.field).to eql("field_92") - expect(error.error).to eql("The owning organisation code is incorrect") + expect(error.error).to eql("You must answer owning organisation") expect(error.purchaser_code).to eql("22 test BU") expect(error.row).to eql("6") expect(error.cell).to eql("CO6") diff --git a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb index 37e68b26c..3b18333df 100644 --- a/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2022/row_parser_spec.rb @@ -407,7 +407,7 @@ RSpec.describe BulkUpload::Sales::Year2022::RowParser do it "is not permitted as setup error" do setup_errors = parser.errors.select { |e| e.options[:category] == :setup } - expect(setup_errors.find { |e| e.attribute == :field_92 }.message).to eql("The owning organisation code is incorrect") + expect(setup_errors.find { |e| e.attribute == :field_92 }.message).to eql("You must answer owning organisation") end it "blocks log creation" do @@ -421,7 +421,7 @@ RSpec.describe BulkUpload::Sales::Year2022::RowParser do it "is not permitted as a setup error" do setup_errors = parser.errors.select { |e| e.options[:category] == :setup } - expect(setup_errors.find { |e| e.attribute == :field_92 }.message).to eql("The owning organisation code is incorrect") + expect(setup_errors.find { |e| e.attribute == :field_92 }.message).to eql("You must answer owning organisation") end it "blocks log creation" do diff --git a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb index cddc042c6..e1137ea84 100644 --- a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb @@ -384,7 +384,7 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do let(:attributes) { setup_section_params.merge(field_1: nil) } it "is not permitted as setup error" do - expect(parser.errors.where(:field_1, category: :setup).map(&:message)).to eql(["The owning organisation code is incorrect"]) + expect(parser.errors.where(:field_1, category: :setup).map(&:message)).to eql(["You must answer owning organisation"]) end it "blocks log creation" do @@ -396,7 +396,7 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do let(:attributes) { { bulk_upload:, field_1: "donotexist" } } it "is not permitted as a setup error" do - expect(parser.errors.where(:field_1, category: :setup).map(&:message)).to eql(["The owning organisation code is incorrect"]) + expect(parser.errors.where(:field_1, category: :setup).map(&:message)).to eql(["You must answer owning organisation"]) end it "blocks log creation" do