CLDC-4025: Move Sales Log organisation to first question (#3074)

* move organisation questions to be first

* fix tests

* fix tests after merge

* document this function

* explicitly write out years for clarity

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

@@ -72,5 +72,5 @@ private
     true
   end
 
-  QUESTION_NUMBER_FROM_YEAR = { 2024 => 2, 2025 => 3 }.freeze
+  QUESTION_NUMBER_FROM_YEAR = { 2024 => 2, 2025 => 2 }.freeze
 end

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, 2025 => 1 }.freeze
 end

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, 2025 => 3 }.freeze
 end

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),

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?(/((?<!la)_known)|(_check)|(_asked)|nationality_all_group|nationality_all_buyer2_group/) }
       ordered_questions.reject! { |q| q.id.match?(/organisation_id|created_by|assigned_to|soctenant|has_management_fee|management_fee|grant|discount|has_mscharge|mscharge|extrabor/) } if @year >= 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

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

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)