Assign house_number and address_lines consistently (#281)

* Assign house_number and address_lines consistently

https://eaflood.atlassian.net/browse/WC-500

Addresses created in the renewals journey can display as having a duplicate house number when they are used by waste-carriers-frontend, for example:

123
123 Waste Street
Wastetown
AB1 2CD

This is because the renewals engine assigns the values of address_lines in a different way. For consistency's sake, we should change this to match how waste-carriers-frontend does it.

* Update spec to match desired result

The first `line` should be assigned to `house_number` and all subsequent lines should be assigned to `address_lines`.

* Rewrite assign_address_lines to pass spec

Rather than trying to work out which lines we want ourselves, we just use the `lines` values from the data sent by os-places-address-lookup. We also get `house_number` from the lines instead of the `buildingNumber` value. This should give us a result which is consistent with the frontend.

* Rename method to assign_house_number_and_address_lines

This is more accurate about what it's now doing.

* Include house_number in displayable_address

This is no longer included in `address_line_1` so we need to add it separately.

* Create displayable_address in ApplicationHelper

We display addresses multiple times, so it's more efficient to have a shared helper method to deal with this.

* Use displayable_address from ApplicationHelper in check your answers page

Now that we have a shared method, we can replace the logic specific to the check your answers form object and view.

* Use displayable_address from ApplicationHelper in mailer

We can now remove the duplicated logic from the mailer and update the template to use the new method.

app/forms/waste_carriers_engine/check_your_answers_form.rb

@@ -4,17 +4,16 @@ class CheckYourAnswersForm < BaseForm
     include CanLimitNumberOfRelevantPeople
     include CanNavigateFlexibly
 
-    attr_accessor :business_type, :company_name, :company_no, :contact_address, :contact_address_display, :contact_email,
+    attr_accessor :business_type, :company_name, :company_no, :contact_address, :contact_email,
                   :declared_convictions, :first_name, :last_name, :location, :main_people, :phone_number,
-                  :registered_address, :registered_address_display, :registration_type, :relevant_people, :tier
+                  :registered_address, :registration_type, :relevant_people, :tier
 
     def initialize(transient_registration)
       super
       self.business_type = @transient_registration.business_type
       self.company_name = @transient_registration.company_name
       self.company_no = @transient_registration.company_no
       self.contact_address = @transient_registration.contact_address
-      self.contact_address_display = displayable_address(contact_address)
       self.contact_email = @transient_registration.contact_email
       self.declared_convictions = @transient_registration.declared_convictions
       self.first_name = @transient_registration.first_name
@@ -23,7 +22,6 @@ def initialize(transient_registration)
       self.main_people = @transient_registration.main_people
       self.phone_number = @transient_registration.phone_number
       self.registered_address = @transient_registration.registered_address
-      self.registered_address_display = displayable_address(registered_address)
       self.registration_type = @transient_registration.registration_type
       self.relevant_people = @transient_registration.relevant_people
       self.tier = @transient_registration.tier
@@ -62,18 +60,6 @@ def contact_name
 
     private
 
-    def displayable_address(address)
-      return [] unless address.present?
-      # Get all the possible address lines, then remove the blank ones
-      [address.address_line_1,
-       address.address_line_2,
-       address.address_line_3,
-       address.address_line_4,
-       address.town_city,
-       address.postcode,
-       address.country].reject
-    end
-
     def should_be_renewed
       business_type_change_valid? && same_company_no?
     end

app/helpers/waste_carriers_engine/application_helper.rb

@@ -52,6 +52,20 @@ def dashboard_link(current_user)
       # "#{root}/user/#{id}/registrations"
     end
 
+    def displayable_address(address)
+      return [] unless address.present?
+
+      # Get all the possible address lines, then remove the blank ones
+      [address.house_number,
+       address.address_line_1,
+       address.address_line_2,
+       address.address_line_3,
+       address.address_line_4,
+       address.town_city,
+       address.postcode,
+       address.country].reject(&:blank?)
+    end
+
     private
 
     def title_text

app/mailers/waste_carriers_engine/renewal_mailer.rb

@@ -5,7 +5,6 @@ class RenewalMailer < ActionMailer::Base
 
     def send_renewal_complete_email(registration)
       @registration = registration
-      @address_lines = displayable_address(@registration.registered_address)
 
       mail(to: @registration.contact_email,
            from: "#{Rails.configuration.email_service_name} <#{Rails.configuration.email_service_email}>",
@@ -39,17 +38,5 @@ def renewal_received_template
         "send_renewal_received_pending_conviction_check_email"
       end
     end
-
-    def displayable_address(address)
-      return [] unless address.present?
-      # Get all the possible address lines, then remove the blank ones
-      [address.address_line_1,
-       address.address_line_2,
-       address.address_line_3,
-       address.address_line_4,
-       address.town_city,
-       address.postcode,
-       address.country].reject
-    end
   end
 end

app/models/waste_carriers_engine/address.rb

@@ -55,7 +55,6 @@ def self.create_from_os_places_data(data)
 
       address[:uprn] = data["uprn"]
       address[:address_mode] = "address-results"
-      address[:house_number] = data["buildingNumber"]
       address[:dependent_locality] = data["dependentLocality"]
       address[:administrative_area] = data["administrativeArea"]
       address[:town_city] = data["town"]
@@ -67,22 +66,19 @@ def self.create_from_os_places_data(data)
       address[:easting] = data["easting"]
       address[:northing] = data["northing"]
 
-      address.assign_address_lines(data)
+      address.assign_house_number_and_address_lines(data)
 
       address
     end
 
-    def assign_address_lines(data)
-      lines = []
-      lines << data["organisationName"]
-      lines << data["subBuildingName"]
-      lines << data["buildingName"]
-      lines << [data["buildingNumber"], data["thoroughfareName"]].join(" ")
-      address_attributes = %i[address_line_1
+    def assign_house_number_and_address_lines(data)
+      lines = data["lines"]
+      address_attributes = %i[house_number
+                              address_line_1
                               address_line_2
                               address_line_3
                               address_line_4]
-      lines.reject!(&:empty?)
+      lines.reject!(&:blank?)
 
       # Assign lines one at a time until we run out of lines to assign
       write_attribute(address_attributes.shift, lines.shift) until lines.empty?

app/views/waste_carriers_engine/check_your_answers_forms/new.html.erb

@@ -67,7 +67,7 @@
       <h2 class="heading-medium"><%= t(".subheading_5.#{@check_your_answers_form.business_type}") %></h2>
 
       <ul class="list">
-        <% @check_your_answers_form.registered_address_display.each do |line| %>
+        <% displayable_address(@check_your_answers_form.registered_address).each do |line| %>
           <li><%= line %></li>
         <% end %>
       </ul>
@@ -110,7 +110,7 @@
       </ul>
 
       <ul class="list">
-        <% @check_your_answers_form.contact_address_display.each do |line| %>
+        <% displayable_address(@check_your_answers_form.contact_address).each do |line| %>
           <li><%= line %></li>
         <% end %>
       </ul>

app/views/waste_carriers_engine/renewal_mailer/send_renewal_complete_email.html.erb

@@ -109,7 +109,7 @@
                 <%= @registration.first_name %> <%= @registration.last_name %>
               </p>
 
-              <% @address_lines.each do |line| %>
+              <% displayable_address(@registration.registered_address).each do |line| %>
                 <p style="font-size: 19px; line-height: 1.315789474; margin: 0;">
                   <%= line %>
                 </p>

spec/helpers/waste_carriers_engine/application_helper_spec.rb

@@ -58,5 +58,22 @@ module WasteCarriersEngine
         expect(helper.dashboard_link(user)).to eq(expected_url)
       end
     end
+
+    describe "displayable_address" do
+      let(:address) do
+        build(:address,
+              house_number: "5",
+              address_line_1: "Foo Terrace",
+              address_line_2: "Bar Street",
+              town_city: "Bazville",
+              postcode: "AB1 2CD",
+              country: "Quxland")
+      end
+
+      it "returns the correct value" do
+        expected_address = ["5", "Foo Terrace", "Bar Street", "Bazville", "AB1 2CD", "Quxland"]
+        expect(displayable_address(address)).to eq(expected_address)
+      end
+    end
   end
 end

spec/models/waste_carriers_engine/address_spec.rb

@@ -4,34 +4,52 @@ module WasteCarriersEngine
   RSpec.describe Address, type: :model do
     let(:address) { build(:address) }
 
-    describe "assign_address_lines" do
+    describe "assign_house_number_and_address_lines" do
       context "when it is given address data" do
         let(:data) do
           {
-            "organisationName" => "Foo Industries",
-            "subBuildingName" => "Suite 5",
-            "buildingName" => "The Bar Building",
-            "buildingNumber" => "5",
-            "thoroughfareName" => "Baz Street"
+            "lines" => [
+              "FOO HOUSE",
+              "BAR BUILDINGS",
+              "BAZ STREET",
+              "QUX CORNER",
+              "QUUX VILLAGE"
+            ]
           }
         end
 
-        it "should assign the correct address lines" do
-          address.assign_address_lines(data)
-          expect(address[:address_line_1]).to eq("Foo Industries")
-          expect(address[:address_line_2]).to eq("Suite 5")
-          expect(address[:address_line_3]).to eq("The Bar Building")
-          expect(address[:address_line_4]).to eq("5 Baz Street")
+        context "when all the lines are used" do
+          before do
+            address.assign_house_number_and_address_lines(data)
+          end
+
+          it "should assign the correct house_number" do
+            expect(address[:house_number]).to eq("FOO HOUSE")
+          end
+
+          it "should assign the correct address_lines" do
+            expect(address[:address_line_1]).to eq("BAR BUILDINGS")
+            expect(address[:address_line_2]).to eq("BAZ STREET")
+            expect(address[:address_line_3]).to eq("QUX CORNER")
+            expect(address[:address_line_4]).to eq("QUUX VILLAGE")
+          end
         end
 
-        context "when not all address fields are used" do
-          before { data["buildingName"] = "" }
+        context "when the lines are not all used" do
+          before do
+            data["lines"] = ["FOO BUILDINGS", "BAR STREET"]
+            address.assign_house_number_and_address_lines(data)
+          end
+
+          it "should assign the correct house_number" do
+            expect(address[:house_number]).to eq("FOO BUILDINGS")
+          end
 
           it "should skip blank fields when assigning lines" do
-            address.assign_address_lines(data)
-            expect(address[:address_line_1]).to eq("Foo Industries")
-            expect(address[:address_line_2]).to eq("Suite 5")
-            expect(address[:address_line_3]).to eq("5 Baz Street")
+            expect(address[:address_line_1]).to eq("BAR STREET")
+            expect(address[:address_line_2].present?).to eq(false)
+            expect(address[:address_line_3].present?).to eq(false)
+            expect(address[:address_line_4].present?).to eq(false)
           end
         end
       end