diff --git a/app/models/form/sales/questions/managing_organisation.rb b/app/models/form/sales/questions/managing_organisation.rb index ffe143ab5..b68a359d7 100644 --- a/app/models/form/sales/questions/managing_organisation.rb +++ b/app/models/form/sales/questions/managing_organisation.rb @@ -4,7 +4,7 @@ class Form::Sales::Questions::ManagingOrganisation < ::Form::Question @id = "managing_organisation_id" @derived = true @type = "select" - @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] + @question_number = 2 end def answer_options(log = nil, user = nil) @@ -71,6 +71,4 @@ private def selected_answer_option_is_derived?(_log) true end - - QUESTION_NUMBER_FROM_YEAR = { 2024 => 2, 2025 => 3 }.freeze end diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index d4ece8554..faf72e59e 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -93,5 +93,5 @@ private "#{name} (inactive as of #{merge_date.to_fs(:govuk_date)})" end - QUESTION_NUMBER_FROM_YEAR = { 2023 => nil, 2024 => 1, 2025 => 2 }.freeze + QUESTION_NUMBER_FROM_YEAR = { 2023 => nil, 2024 => 1 }.freeze end diff --git a/app/models/form/sales/questions/sale_date.rb b/app/models/form/sales/questions/sale_date.rb index 8adefd87c..00f0c86dd 100644 --- a/app/models/form/sales/questions/sale_date.rb +++ b/app/models/form/sales/questions/sale_date.rb @@ -6,5 +6,5 @@ class Form::Sales::Questions::SaleDate < ::Form::Question @question_number = QUESTION_NUMBER_FROM_YEAR[form.start_date.year] || QUESTION_NUMBER_FROM_YEAR[QUESTION_NUMBER_FROM_YEAR.keys.max] end - QUESTION_NUMBER_FROM_YEAR = { 2023 => 1, 2024 => 3, 2025 => 1 }.freeze + QUESTION_NUMBER_FROM_YEAR = { 2023 => 1, 2024 => 3 }.freeze end diff --git a/app/models/form/sales/subsections/setup.rb b/app/models/form/sales/subsections/setup.rb index be6c20400..42eea0026 100644 --- a/app/models/form/sales/subsections/setup.rb +++ b/app/models/form/sales/subsections/setup.rb @@ -7,9 +7,9 @@ class Form::Sales::Subsections::Setup < ::Form::Subsection def pages @pages ||= [ - Form::Sales::Pages::SaleDate.new(nil, nil, self), Form::Sales::Pages::OwningOrganisation.new(nil, nil, self), Form::Sales::Pages::ManagingOrganisation.new(nil, nil, self), + Form::Sales::Pages::SaleDate.new(nil, nil, self), Form::Sales::Pages::CreatedBy.new(nil, nil, self), Form::Sales::Pages::PurchaserCode.new(nil, nil, self), Form::Sales::Pages::OwnershipScheme.new(nil, nil, self), diff --git a/app/services/csv/sales_log_csv_service.rb b/app/services/csv/sales_log_csv_service.rb index 9f16a3f8b..0b6612a19 100644 --- a/app/services/csv/sales_log_csv_service.rb +++ b/app/services/csv/sales_log_csv_service.rb @@ -259,6 +259,7 @@ module Csv ordered_questions = FormHandler.instance.ordered_questions_for_year(@year, "sales") ordered_questions.reject! { |q| q.id.match?(/((?= 2025 + order_saledate_question_before_owning_organisation_question(ordered_questions) attributes = insert_checkbox_options(ordered_questions) final_attributes = insert_derived_and_related_attributes(non_question_fields + attributes) order_address_fields_for_support(final_attributes) @@ -321,6 +322,21 @@ module Csv end end + # as part of CLDC-3719 it was decided to move the saledate question to be first in the form + # this caused issues reported in CLDC-4025 where the user only enter saledates for their active organisation + # we decided to move the organisation question back to being first + # however, we did not want to reorder the CSV export as this would disrupt existing users' data pipelines + # so, this function reorders questions back when exporting CSVs + # next year, we can remove this function as we will be reordering the csv fields anyway + def order_saledate_question_before_owning_organisation_question(ordered_questions) + saledate_question_index = ordered_questions.find_index { |q| q.id == "saledate" } + owning_organisation_index = ordered_questions.find_index { |q| q.id == "owning_organisation_id" } + if saledate_question_index && owning_organisation_index + saledate_question = ordered_questions.delete_at(saledate_question_index) + ordered_questions.insert(owning_organisation_index, saledate_question) + end + end + def non_question_fields case @year when 2022 diff --git a/spec/models/form/sales/subsections/setup_spec.rb b/spec/models/form/sales/subsections/setup_spec.rb index f93e7ef5e..87f848c91 100644 --- a/spec/models/form/sales/subsections/setup_spec.rb +++ b/spec/models/form/sales/subsections/setup_spec.rb @@ -29,9 +29,9 @@ RSpec.describe Form::Sales::Subsections::Setup, type: :model do it "has correct pages" do expect(setup.pages.map(&:id)).to eq( %w[ - completion_date owning_organisation managing_organisation + completion_date assigned_to purchaser_code ownership_scheme @@ -56,9 +56,9 @@ RSpec.describe Form::Sales::Subsections::Setup, type: :model do it "has correct pages" do expect(setup.pages.map(&:id)).to eq( %w[ - completion_date owning_organisation managing_organisation + completion_date assigned_to purchaser_code ownership_scheme @@ -87,9 +87,9 @@ RSpec.describe Form::Sales::Subsections::Setup, type: :model do it "has correct pages" do expect(setup.pages.map(&:id)).to eq( %w[ - completion_date owning_organisation managing_organisation + completion_date assigned_to purchaser_code ownership_scheme diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index a6c80abc6..c5c158a92 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -182,7 +182,7 @@ RSpec.describe FormController, type: :request do it "correctly sets owning organisation" do post "/sales-logs/#{sales_log.id}/owning-organisation", params: params - expect(response).to redirect_to("/sales-logs/#{sales_log.id}/assigned-to") + expect(response).to redirect_to("/sales-logs/#{sales_log.id}/completion-date") follow_redirect! sales_log.reload expect(sales_log.owning_organisation).to eq(managing_organisation) @@ -209,7 +209,7 @@ RSpec.describe FormController, type: :request do it "does not reset assigned to" do post "/sales-logs/#{sales_log.id}/owning-organisation", params: params - expect(response).to redirect_to("/sales-logs/#{sales_log.id}/assigned-to") + expect(response).to redirect_to("/sales-logs/#{sales_log.id}/completion-date") follow_redirect! sales_log.reload expect(sales_log.assigned_to).to eq(assigned_to) @@ -238,7 +238,7 @@ RSpec.describe FormController, type: :request do it "does not reset assigned to" do post "/sales-logs/#{sales_log.id}/owning-organisation", params: params - expect(response).to redirect_to("/sales-logs/#{sales_log.id}/assigned-to") + expect(response).to redirect_to("/sales-logs/#{sales_log.id}/completion-date") follow_redirect! sales_log.reload expect(sales_log.assigned_to).to eq(assigned_to)