CLDC-2390 CYA address redesign (#1696)

* feat: update cya design lettings

* feat: update cya sales

* feat: update some tests

* feat: fix error labels for address questions

* feat: fix error labels for address questions and update row parsers

* feat: update tests

* feat: update tests

* feat: rename display_label

app/helpers/question_view_helper.rb

@@ -7,7 +7,7 @@ module QuestionViewHelper
 
   def legend(question, page_header, conditional)
     {
-      text: [question.question_number_string(conditional:), question.header.html_safe].compact.join(" - "),
+      text: [question.question_number_string(hidden: conditional || question.hide_question_number_on_page), question.header.html_safe].compact.join(" - "),
       size: label_size(page_header, conditional, question),
       tag: label_tag(page_header, conditional),
     }

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

@@ -2,29 +2,20 @@ class Form::Lettings::Questions::AddressLine1 < ::Form::Question
   def initialize(id, hsh, page)
     super
     @id = "address_line1"
-    @check_answer_label = "Address"
     @header = "Address line 1"
+    @error_label = "Address line 1"
     @type = "text"
     @plain_label = true
-    @check_answer_label = "Q12 - Address"
+    @check_answer_label = "Address lines 1 and 2"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
+    @question_number = 12
+    @hide_question_number_on_page = true
   end
 
   def answer_label(log, _current_user = nil)
     [
       log.address_line1,
       log.address_line2,
-      log.postcode_full,
-      log.town_or_city,
-      log.county,
     ].select(&:present?).join("\n")
   end
-
-  def get_extra_check_answer_value(log)
-    return unless log.is_la_inferred?
-
-    la = LocalAuthority.find_by(code: log.la)&.name
-
-    la.presence
-  end
 end

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

@@ -5,10 +5,9 @@ class Form::Lettings::Questions::County < ::Form::Question
     @header = "County (optional)"
     @type = "text"
     @plain_label = true
+    @check_answer_label = "County"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
-  end
+    @question_number = 12
-
+    @hide_question_number_on_page = true
-  def hidden_in_check_answers?(_log = nil, _current_user = nil)
-    true
   end
 end

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

@@ -17,10 +17,9 @@ class Form::Lettings::Questions::PostcodeForFullAddress < ::Form::Question
       },
     }
     @plain_label = true
+    @check_answer_label = "Postcode"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
-  end
+    @question_number = 12
-
+    @hide_question_number_on_page = true
-  def hidden_in_check_answers?(_log = nil, _current_user = nil)
-    true
   end
 end

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

@@ -5,10 +5,9 @@ class Form::Lettings::Questions::TownOrCity < ::Form::Question
     @header = "Town or city"
     @type = "text"
     @plain_label = true
+    @check_answer_label = "Town or city"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
-  end
+    @question_number = 12
-
+    @hide_question_number_on_page = true
-  def hidden_in_check_answers?(_log = nil, _current_user = nil)
-    true
   end
 end

app/models/form/question.rb

@@ -4,7 +4,7 @@ class Form::Question
                 :conditional_for, :readonly, :answer_options, :page, :check_answer_label,
                 :inferred_answers, :hidden_in_check_answers, :inferred_check_answers_value,
                 :guidance_partial, :prefix, :suffix, :requires_js, :fields_added, :derived,
-                :check_answers_card_number, :unresolved_hint_text, :question_number, :plain_label
+                :check_answers_card_number, :unresolved_hint_text, :question_number, :hide_question_number_on_page, :plain_label, :error_label
 
   module GuidancePosition
     TOP = 1
@@ -41,7 +41,9 @@ class Form::Question
       @check_answers_card_number = hsh["check_answers_card_number"] || 0
       @unresolved_hint_text = hsh["unresolved_hint_text"]
       @question_number = hsh["question_number"]
+      @hide_question_number_on_page = hsh["hide_question_number_on_page"] || false
       @plain_label = hsh["plain_label"]
+      @error_label = hsh["error_label"]
       @disable_clearing_if_not_routed_or_dynamic_answer_options = hsh["disable_clearing_if_not_routed_or_dynamic_answer_options"]
     end
   end
@@ -194,15 +196,15 @@ class Form::Question
     type == "radio" && RADIO_REFUSED_VALUE[id.to_sym]&.include?(value)
   end
 
-  def display_label
+  def error_display_label
-    check_answer_label || header || id.humanize
+    error_label || check_answer_label || header || id.humanize
   end
 
   def unanswered_error_message
     return I18n.t("validations.declaration.missing") if id == "declaration"
     return I18n.t("validations.privacynotice.missing") if id == "privacynotice"
 
-    I18n.t("validations.not_answered", question: display_label.downcase)
+    I18n.t("validations.not_answered", question: error_display_label.downcase)
   end
 
   def suffix_label(log)
@@ -241,8 +243,8 @@ class Form::Question
     selected_answer_option_is_derived?(log) || has_inferred_check_answers_value?(log)
   end
 
-  def question_number_string(conditional: false)
+  def question_number_string(hidden: false)
-    if @question_number && !conditional && form.start_date.year >= 2023
+    if @question_number && !hidden && form.start_date.year >= 2023
       "Q#{@question_number}"
     end
   end

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

@@ -2,29 +2,20 @@ class Form::Sales::Questions::AddressLine1 < ::Form::Question
   def initialize(id, hsh, page)
     super
     @id = "address_line1"
-    @check_answer_label = "Address"
     @header = "Address line 1"
+    @error_label = "Address line 1"
     @type = "text"
     @plain_label = true
-    @check_answer_label = "Q15 - Address"
+    @check_answer_label = "Address lines 1 and 2"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
+    @question_number = 15
+    @hide_question_number_on_page = true
   end
 
   def answer_label(log, _current_user = nil)
     [
       log.address_line1,
       log.address_line2,
-      log.postcode_full,
-      log.town_or_city,
-      log.county,
     ].select(&:present?).join("\n")
   end
-
-  def get_extra_check_answer_value(log)
-    return unless log.is_la_inferred?
-
-    la = LocalAuthority.find_by(code: log.la)&.name
-
-    la.presence
-  end
 end

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

@@ -5,10 +5,9 @@ class Form::Sales::Questions::County < ::Form::Question
     @header = "County (optional)"
     @type = "text"
     @plain_label = true
+    @check_answer_label = "County"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
-  end
+    @question_number = 15
-
+    @hide_question_number_on_page = true
-  def hidden_in_check_answers?(_log = nil, _current_user = nil)
-    true
   end
 end

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

@@ -17,10 +17,9 @@ class Form::Sales::Questions::PostcodeForFullAddress < ::Form::Question
       },
     }
     @plain_label = true
+    @check_answer_label = "Postcode"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
-  end
+    @question_number = 15
-
+    @hide_question_number_on_page = true
-  def hidden_in_check_answers?(_log = nil, _current_user = nil)
-    true
   end
 end

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

@@ -5,10 +5,9 @@ class Form::Sales::Questions::TownOrCity < ::Form::Question
     @header = "Town or city"
     @type = "text"
     @plain_label = true
+    @check_answer_label = "Town or city"
     @disable_clearing_if_not_routed_or_dynamic_answer_options = true
-  end
+    @question_number = 15
-
+    @hide_question_number_on_page = true
-  def hidden_in_check_answers?(_log = nil, _current_user = nil)
-    true
   end
 end

app/services/bulk_upload/lettings/year2022/row_parser.rb

@@ -774,13 +774,13 @@ private
       if setup_question?(question)
         fields.each do |field|
           if errors.select { |e| fields.include?(e.attribute) }.none?
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase), category: :setup)
+            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase), category: :setup)
           end
         end
       else
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase))
+            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase))
           end
         end
       end

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

@@ -681,14 +681,14 @@ private
       if setup_question?(question)
         fields.each do |field|
           if errors.select { |e| fields.include?(e.attribute) }.none?
-            question_text = question.check_answer_label.presence || question.header.presence || "this question"
+            question_text = question.error_display_label.presence || "this question"
             errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase), category: :setup)
           end
         end
       else
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            question_text = question.check_answer_label.presence || question.header.presence || "this question"
+            question_text = question.error_display_label.presence || "this question"
             errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase))
           end
         end

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

@@ -1057,9 +1057,9 @@ private
       fields.each do |field|
         unless errors.any? { |e| fields.include?(e.attribute) }
           if setup_question?(question)
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase), category: :setup)
+            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase), category: :setup)
           else
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase))
+            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase))
           end
         end
       end

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

@@ -1210,13 +1210,13 @@ private
       if setup_question?(question)
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase), category: :setup)
+            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase), category: :setup)
           end
         end
       else
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            errors.add(field, I18n.t("validations.not_answered", question: question.check_answer_label&.downcase))
+            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase))
           end
         end
       end

spec/helpers/question_view_helper_spec.rb

@@ -53,6 +53,10 @@ RSpec.describe QuestionViewHelper do
         def plain_label
           nil
         end
+
+        def hide_question_number_on_page
+          false
+        end
       end
     end
 
@@ -98,6 +102,10 @@ RSpec.describe QuestionViewHelper do
           def plain_label
             true
           end
+
+          def hide_question_number_on_page
+            false
+          end
         end
       end
 

spec/models/form/lettings/questions/address_line1_spec.rb

@@ -19,12 +19,16 @@ RSpec.describe Form::Lettings::Questions::AddressLine1, type: :model do
     expect(question.header).to eq("Address line 1")
   end
 
-  it "has the correct question_number" do
+  it "has the correct error label" do
-    expect(question.question_number).to be_nil
+    expect(question.error_label).to eq("Address line 1")
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to eq("Q12 - Address")
+    expect(question.check_answer_label).to eq("Address lines 1 and 2")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(12)
   end
 
   it "has the correct type" do
@@ -46,34 +50,4 @@ RSpec.describe Form::Lettings::Questions::AddressLine1, type: :model do
   it "has the correct check_answers_card_number" do
     expect(question.check_answers_card_number).to be_nil
   end
-
-  describe "has the correct get_extra_check_answer_value" do
-    context "when la is not present" do
-      let(:log) { create(:lettings_log, la: nil) }
-
-      it "returns nil" do
-        expect(question.get_extra_check_answer_value(log)).to be_nil
-      end
-    end
-
-    context "when la is present but not inferred" do
-      let(:log) { create(:lettings_log, la: "E09000003", is_la_inferred: false) }
-
-      it "returns nil" do
-        expect(question.get_extra_check_answer_value(log)).to be_nil
-      end
-    end
-
-    context "when la is present and inferred" do
-      let(:log) { create(:lettings_log, la: "E09000003") }
-
-      before do
-        allow(log).to receive(:is_la_inferred?).and_return(true)
-      end
-
-      it "returns the la" do
-        expect(question.get_extra_check_answer_value(log)).to eq("Barnet")
-      end
-    end
-  end
 end

spec/models/form/lettings/questions/county_spec.rb

@@ -20,7 +20,11 @@ RSpec.describe Form::Lettings::Questions::County, type: :model do
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to be_nil
+    expect(question.check_answer_label).to eq("County")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(12)
   end
 
   it "has the correct type" do
@@ -42,8 +46,4 @@ RSpec.describe Form::Lettings::Questions::County, type: :model do
   it "has the correct check_answers_card_number" do
     expect(question.check_answers_card_number).to be_nil
   end
-
-  it "has the correct hidden_in_check_answers" do
-    expect(question.hidden_in_check_answers?).to eq(true)
-  end
 end

spec/models/form/lettings/questions/postcode_for_full_address_spec.rb

@@ -20,7 +20,11 @@ RSpec.describe Form::Lettings::Questions::PostcodeForFullAddress, type: :model d
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to be_nil
+    expect(question.check_answer_label).to eq("Postcode")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(12)
   end
 
   it "has the correct type" do
@@ -55,8 +59,4 @@ RSpec.describe Form::Lettings::Questions::PostcodeForFullAddress, type: :model d
       "value" => "Not known",
     }])
   end
-
-  it "has the correct hidden_in_check_answers" do
-    expect(question.hidden_in_check_answers?).to eq(true)
-  end
 end

spec/models/form/lettings/questions/town_or_city_spec.rb

@@ -20,7 +20,11 @@ RSpec.describe Form::Lettings::Questions::TownOrCity, type: :model do
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to be_nil
+    expect(question.check_answer_label).to eq("Town or city")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(12)
   end
 
   it "has the correct type" do
@@ -42,8 +46,4 @@ RSpec.describe Form::Lettings::Questions::TownOrCity, type: :model do
   it "has the correct check_answers_card_number" do
     expect(question.check_answers_card_number).to be_nil
   end
-
-  it "has the correct hidden_in_check_answers" do
-    expect(question.hidden_in_check_answers?).to eq(true)
-  end
 end

spec/models/form/sales/questions/address_line1_spec.rb

@@ -11,10 +11,6 @@ RSpec.describe Form::Sales::Questions::AddressLine1, type: :model do
     expect(question.page).to eq(page)
   end
 
-  it "has the correct question_number" do
-    expect(question.question_number).to be_nil
-  end
-
   it "has the correct id" do
     expect(question.id).to eq("address_line1")
   end
@@ -23,8 +19,16 @@ RSpec.describe Form::Sales::Questions::AddressLine1, type: :model do
     expect(question.header).to eq("Address line 1")
   end
 
+  it "has the correct error label" do
+    expect(question.error_label).to eq("Address line 1")
+  end
+
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to eq("Q15 - Address")
+    expect(question.check_answer_label).to eq("Address lines 1 and 2")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(15)
   end
 
   it "has the correct type" do
@@ -46,34 +50,4 @@ RSpec.describe Form::Sales::Questions::AddressLine1, type: :model do
   it "has the correct check_answers_card_number" do
     expect(question.check_answers_card_number).to be_nil
   end
-
-  describe "has the correct get_extra_check_answer_value" do
-    context "when la is not present" do
-      let(:log) { create(:sales_log, la: nil) }
-
-      it "returns nil" do
-        expect(question.get_extra_check_answer_value(log)).to be_nil
-      end
-    end
-
-    context "when la is present but not inferred" do
-      let(:log) { create(:sales_log, la: "E09000003", is_la_inferred: false) }
-
-      it "returns nil" do
-        expect(question.get_extra_check_answer_value(log)).to be_nil
-      end
-    end
-
-    context "when la is present and inferred" do
-      let(:log) { create(:sales_log, la: "E09000003") }
-
-      before do
-        allow(log).to receive(:is_la_inferred?).and_return(true)
-      end
-
-      it "returns the la" do
-        expect(question.get_extra_check_answer_value(log)).to eq("Barnet")
-      end
-    end
-  end
 end

spec/models/form/sales/questions/county_spec.rb

@@ -20,7 +20,11 @@ RSpec.describe Form::Sales::Questions::County, type: :model do
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to be_nil
+    expect(question.check_answer_label).to eq("County")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(15)
   end
 
   it "has the correct type" do
@@ -42,8 +46,4 @@ RSpec.describe Form::Sales::Questions::County, type: :model do
   it "has the correct check_answers_card_number" do
     expect(question.check_answers_card_number).to be_nil
   end
-
-  it "has the correct hidden_in_check_answers" do
-    expect(question.hidden_in_check_answers?).to eq(true)
-  end
 end

spec/models/form/sales/questions/postcode_for_full_address_spec.rb

@@ -20,7 +20,11 @@ RSpec.describe Form::Sales::Questions::PostcodeForFullAddress, type: :model do
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to be_nil
+    expect(question.check_answer_label).to eq("Postcode")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(15)
   end
 
   it "has the correct type" do
@@ -55,8 +59,4 @@ RSpec.describe Form::Sales::Questions::PostcodeForFullAddress, type: :model do
       "value" => "Not known",
     }])
   end
-
-  it "has the correct hidden_in_check_answers" do
-    expect(question.hidden_in_check_answers?).to eq(true)
-  end
 end

spec/models/form/sales/questions/town_or_city_spec.rb

@@ -20,7 +20,11 @@ RSpec.describe Form::Sales::Questions::TownOrCity, type: :model do
   end
 
   it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to be_nil
+    expect(question.check_answer_label).to eq("Town or city")
+  end
+
+  it "has the correct question_number" do
+    expect(question.question_number).to eq(15)
   end
 
   it "has the correct type" do
@@ -42,8 +46,4 @@ RSpec.describe Form::Sales::Questions::TownOrCity, type: :model do
   it "has the correct check_answers_card_number" do
     expect(question.check_answers_card_number).to be_nil
   end
-
-  it "has the correct hidden_in_check_answers" do
-    expect(question.hidden_in_check_answers?).to eq(true)
-  end
 end

spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

@@ -340,7 +340,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
 
           it "fetches the question's check_answer_label if it exists, otherwise it gets the question's header" do
             parser.valid?
-            expect(parser.errors[:field_19]).to eql(["You must answer q12 - address"])
+            expect(parser.errors[:field_19]).to eql(["You must answer address line 1"])
             expect(parser.errors[:field_21]).to eql(["You must answer town or city"])
           end
         end
@@ -937,7 +937,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
 
         it "adds appropriate errors" do
           expect(parser.errors[:field_18]).to eql(["You must answer UPRN"])
-          expect(parser.errors[:field_19]).to eql(["You must answer q12 - address"])
+          expect(parser.errors[:field_19]).to eql(["You must answer address line 1"])
           expect(parser.errors[:field_21]).to eql(["You must answer town or city"])
         end
       end