From b7dfc3c3a9d3cf9f47748a2b40ba69f34dcd508e Mon Sep 17 00:00:00 2001 From: irisfaraway Date: Tue, 10 Apr 2018 09:47:20 +0100 Subject: [PATCH 1/4] Select contact address from postcode lookup results https://eaflood.atlassian.net/browse/WC-112 Once users have entered a postcode to search with, we should let them choose the address they want from the results and save that as their contact address. This should work the same way as the registered address earlier in the form. We should go with the same inheritance model we've used for the people forms, postcode forms and manual address forms. From 69a936e9ea320012d3480569bc64a0ca2f07ad8a Mon Sep 17 00:00:00 2001 From: irisfaraway Date: Tue, 10 Apr 2018 10:28:03 +0100 Subject: [PATCH 2/4] Set up ContactAddressForm This is currently a copy of CompanyAddressForm with the relevant name changes. The next step is refactoring to reduce duplication. --- .../contact_address_forms_controller.rb | 7 ++ app/forms/contact_address_form.rb | 55 ++++++++- app/views/contact_address_forms/new.html.erb | 38 +++++-- .../forms/contact_address_forms/en.yml | 10 ++ config/routes.rb | 5 + spec/factories/forms/contact_address_form.rb | 28 +++++ spec/forms/contact_address_forms_spec.rb | 67 ++++++++--- ..._registration_contact_address_form_spec.rb | 4 + spec/requests/contact_address_forms_spec.rb | 107 +++++++++++++++++- 9 files changed, 285 insertions(+), 36 deletions(-) diff --git a/app/controllers/contact_address_forms_controller.rb b/app/controllers/contact_address_forms_controller.rb index 76bde10b6..fb57f1208 100644 --- a/app/controllers/contact_address_forms_controller.rb +++ b/app/controllers/contact_address_forms_controller.rb @@ -6,4 +6,11 @@ def new def create super(ContactAddressForm, "contact_address_form") end + + def skip_to_manual_address + set_transient_registration(params[:reg_identifier]) + + @transient_registration.skip_to_manual_address! if form_matches_state? + redirect_to_correct_form + end end diff --git a/app/forms/contact_address_form.rb b/app/forms/contact_address_form.rb index bc93cf38c..f888c9802 100644 --- a/app/forms/contact_address_form.rb +++ b/app/forms/contact_address_form.rb @@ -1,17 +1,62 @@ class ContactAddressForm < BaseForm - # TODO: Define accessible attributes, eg attr_accessor :field + attr_accessor :business_type + attr_accessor :temp_contact_postcode + attr_accessor :temp_addresses + attr_accessor :temp_address + attr_accessor :addresses def initialize(transient_registration) super - # TODO: Define params to get from transient_registration, eg self.field = @transient_registration.field + # We only use this for the correct microcopy + self.business_type = @transient_registration.business_type + self.temp_contact_postcode = @transient_registration.temp_contact_postcode + + look_up_addresses + preselect_existing_address end def submit(params) # Assign the params for validation and pass them to the BaseForm method for updating - # TODO: Define allowed params, eg self.field = params[:field] - # TODO: Include attributes to update in the attributes hash, eg { field: field } - attributes = {} + self.addresses = add_or_replace_address(params[:temp_address]) + attributes = { addresses: addresses } super(attributes, params[:reg_identifier]) end + + validates :addresses, presence: true + + private + + # Look up addresses based on the temp_contact_postcode + def look_up_addresses + if temp_contact_postcode.present? + address_finder = AddressFinderService.new(temp_contact_postcode) + self.temp_addresses = address_finder.search_by_postcode + else + self.temp_addresses = [] + end + end + + # If an address has already been assigned to the transient registration, pre-select it + def preselect_existing_address + return unless @transient_registration.addresses.present? + current_address = @transient_registration.contact_address + return unless current_address.uprn.present? + selected_address = temp_addresses.detect { |address| address["uprn"] == current_address.uprn.to_s } + self.temp_address = selected_address["uprn"] if selected_address.present? + end + + def add_or_replace_address(selected_address_uprn) + return if selected_address_uprn.blank? + + data = temp_addresses.detect { |address| address["uprn"] == selected_address_uprn } + address = Address.create_from_os_places_data(data) + address.assign_attributes(address_type: "CONTACT") + + # Update the transient object's nested addresses, replacing any existing registered address + updated_addresses = @transient_registration.addresses + updated_addresses.delete(@transient_registration.contact_address) if @transient_registration.contact_address + updated_addresses << address + updated_addresses + end end diff --git a/app/views/contact_address_forms/new.html.erb b/app/views/contact_address_forms/new.html.erb index c25566a05..3c92993d9 100644 --- a/app/views/contact_address_forms/new.html.erb +++ b/app/views/contact_address_forms/new.html.erb @@ -1,24 +1,40 @@ <%= render("shared/back", back_path: back_contact_address_forms_path(@contact_address_form.reg_identifier)) %> -<% if @contact_address_form.errors.any? %> - -

<%= t(".error_heading") %>

- - <% @contact_address_form.errors.full_messages.each do |message| %> -
  • <%= message %>
  • - <% end %> - -<% else %> - +
    <%= form_for(@contact_address_form) do |f| %> <%= render("shared/errors", object: @contact_address_form) %>

    <%= t(".heading") %>

    +
    + + <%= @contact_address_form.temp_contact_postcode %> + <%= link_to(t(".postcode_change_link"), back_contact_address_forms_path(@contact_address_form.reg_identifier)) %> +
    + + <% if @contact_address_form.errors[:addresses].any? %> +
    + <% else %> +
    + <% end %> +
    + <%= f.label :temp_address, t(".address_label"), class: "form-label" %> + <%= f.select(:temp_address, + options_for_select(@contact_address_form.temp_addresses.map { |a| [a["partial"], a["uprn"]] }, @contact_address_form.temp_address), + { include_blank: t(".address_blank_option", count: @contact_address_form.temp_addresses.length ) }, + { class: "form-control" }) %> +
    +
    + +
    + <%= link_to(t(".manual_address_link"), skip_to_manual_address_contact_address_forms_path(@contact_address_form.reg_identifier)) %> +
    + <%= f.hidden_field :reg_identifier, value: @contact_address_form.reg_identifier %>
    <%= f.submit t(".next_button"), class: "button" %>
    <% end %> -<% end %> + <%= render("shared/os_terms_footer") %> +
    diff --git a/config/locales/forms/contact_address_forms/en.yml b/config/locales/forms/contact_address_forms/en.yml index 454b54d0c..52b842f5c 100644 --- a/config/locales/forms/contact_address_forms/en.yml +++ b/config/locales/forms/contact_address_forms/en.yml @@ -2,6 +2,14 @@ en: contact_address_forms: new: heading: What's the address of the person we should contact? + postcode_label: Postcode + postcode_change_link: Change + address_label: Address + address_blank_option: + zero: No addresses found + one: 1 address found + other: "%{count} addresses found" + manual_address_link: "I can't find the address in the list" error_heading: Something is wrong next_button: Continue activemodel: @@ -9,6 +17,8 @@ en: models: contact_address_form: attributes: + addresses: + blank: "You must select an address" reg_identifier: invalid_format: "The registration ID is not in a valid format" no_registration: "There is no registration matching this ID" diff --git a/config/routes.rb b/config/routes.rb index fa809543c..eca0dcf41 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -276,6 +276,11 @@ to: "contact_address_forms#go_back", as: "back", on: :collection + + get "skip_to_manual_address/:reg_identifier", + to: "contact_address_forms#skip_to_manual_address", + as: "skip_to_manual_address", + on: :collection end resources :contact_address_manual_forms, diff --git a/spec/factories/forms/contact_address_form.rb b/spec/factories/forms/contact_address_form.rb index 662884d6d..1167ca536 100644 --- a/spec/factories/forms/contact_address_form.rb +++ b/spec/factories/forms/contact_address_form.rb @@ -1,6 +1,34 @@ FactoryBot.define do factory :contact_address_form do trait :has_required_data do + temp_address "340116" + temp_addresses [{ + "moniker" => "340116", + "uprn" => "340116", + "lines" => ["NATURAL ENGLAND", "DEANERY ROAD"], + "town" => "BRISTOL", + "postcode" => "BS1 5AH", + "easting" => "358205", + "northing" => "172708", + "country" => "", + "dependentLocality" => "", + "dependentThroughfare" => "", + "administrativeArea" => "BRISTOL", + "localAuthorityUpdateDate" => "", + "royalMailUpdateDate" => "", + "partial" => "NATURAL ENGLAND, HORIZON HOUSE, DEANERY ROAD, BRISTOL, BS1 5AH", + "subBuildingName" => "", + "buildingName" => "HORIZON HOUSE", + "thoroughfareName" => "DEANERY ROAD", + "organisationName" => "NATURAL ENGLAND", + "buildingNumber" => "", + "postOfficeBoxNumber" => "", + "departmentName" => "", + "doubleDependentLocality" => "" + }] + + addresses { [build(:address, :has_required_data, :contact)] } + initialize_with { new(create(:transient_registration, :has_required_data, workflow_state: "contact_address_form")) } end end diff --git a/spec/forms/contact_address_forms_spec.rb b/spec/forms/contact_address_forms_spec.rb index 6eae03775..9145f9001 100644 --- a/spec/forms/contact_address_forms_spec.rb +++ b/spec/forms/contact_address_forms_spec.rb @@ -1,13 +1,21 @@ require "rails_helper" RSpec.describe ContactAddressForm, type: :model do + # Stub the address search so we have JSON to use + before do + address_json = build(:contact_address_form, :has_required_data).temp_addresses + allow_any_instance_of(AddressFinderService).to receive(:search_by_postcode).and_return(address_json) + end + describe "#submit" do context "when the form is valid" do let(:contact_address_form) { build(:contact_address_form, :has_required_data) } - let(:valid_params) { { reg_identifier: contact_address_form.reg_identifier } } + let(:valid_params) { { reg_identifier: contact_address_form.reg_identifier, temp_address: contact_address_form.temp_address } } it "should submit" do - expect(contact_address_form.submit(valid_params)).to eq(true) + VCR.use_cassette("contact_postcode_form_valid_postcode") do + expect(contact_address_form.submit(valid_params)).to eq(true) + end end end @@ -16,28 +24,22 @@ let(:invalid_params) { { reg_identifier: "foo" } } it "should not submit" do - expect(contact_address_form.submit(invalid_params)).to eq(false) + VCR.use_cassette("contact_postcode_form_valid_postcode") do + expect(contact_address_form.submit(invalid_params)).to eq(false) + end end end end - describe "#reg_identifier" do - context "when a valid transient registration exists" do - let(:transient_registration) do - create(:transient_registration, - :has_required_data, - workflow_state: "contact_address_form") - end - # Don't use FactoryBot for this as we need to make sure it initializes with a specific object - let(:contact_address_form) { ContactAddressForm.new(transient_registration) } + context "when a form with a valid transient registration exists" do + let(:contact_address_form) { build(:contact_address_form, :has_required_data) } + describe "#reg_identifier" do context "when a reg_identifier meets the requirements" do - before(:each) do - contact_address_form.reg_identifier = transient_registration.reg_identifier - end - it "is valid" do - expect(contact_address_form).to be_valid + VCR.use_cassette("contact_postcode_form_valid_postcode") do + expect(contact_address_form).to be_valid + end end end @@ -46,6 +48,20 @@ contact_address_form.reg_identifier = "" end + it "is not valid" do + VCR.use_cassette("contact_postcode_form_valid_postcode") do + expect(contact_address_form).to_not be_valid + end + end + end + end + + describe "#addresses" do + context "when no address is selected" do + before(:each) do + contact_address_form.addresses = nil + end + it "is not valid" do expect(contact_address_form).to_not be_valid end @@ -53,6 +69,23 @@ end end + context "when a form with a valid transient registration exists and the transient registration already has an address" do + let(:transient_registration) do + build(:transient_registration, + :has_postcode, + :has_addresses, + workflow_state: "contact_address_form") + end + # Don't use FactoryBot for this as we need to make sure it initializes with a specific object + let(:contact_address_form) { ContactAddressForm.new(transient_registration) } + + describe "#temp_address" do + it "pre-selects the address" do + expect(contact_address_form.temp_address).to eq(transient_registration.addresses.where(address_type: "CONTACT").first.uprn.to_s) + end + end + end + describe "#transient_registration" do context "when the transient registration is invalid" do let(:transient_registration) do diff --git a/spec/models/workflow_states/transient_registration_contact_address_form_spec.rb b/spec/models/workflow_states/transient_registration_contact_address_form_spec.rb index 6c77cb9d1..3a77e6477 100644 --- a/spec/models/workflow_states/transient_registration_contact_address_form_spec.rb +++ b/spec/models/workflow_states/transient_registration_contact_address_form_spec.rb @@ -16,6 +16,10 @@ it "changes to :check_your_answers_form after the 'next' event" do expect(transient_registration).to transition_from(:contact_address_form).to(:check_your_answers_form).on_event(:next) end + + it "changes to :contact_address_manual_form after the 'skip_to_manual_address' event" do + expect(transient_registration).to transition_from(:contact_address_form).to(:contact_address_manual_form).on_event(:skip_to_manual_address) + end end end end diff --git a/spec/requests/contact_address_forms_spec.rb b/spec/requests/contact_address_forms_spec.rb index 966e28011..3ecd69590 100644 --- a/spec/requests/contact_address_forms_spec.rb +++ b/spec/requests/contact_address_forms_spec.rb @@ -1,6 +1,12 @@ require "rails_helper" RSpec.describe "ContactAddressForms", type: :request do + # Stub the address search so we have JSON to use + before do + address_json = build(:contact_address_form, :has_required_data).temp_addresses + allow_any_instance_of(AddressFinderService).to receive(:search_by_postcode).and_return(address_json) + end + describe "GET new_contact_address_path" do context "when a valid user is signed in" do let(:user) { create(:user) } @@ -12,6 +18,7 @@ let(:transient_registration) do create(:transient_registration, :has_required_data, + :has_postcode, account_email: user.email, workflow_state: "contact_address_form") end @@ -26,6 +33,7 @@ let(:transient_registration) do create(:transient_registration, :has_required_data, + :has_postcode, account_email: user.email, workflow_state: "renewal_start_form") end @@ -49,6 +57,7 @@ let(:transient_registration) do create(:transient_registration, :has_required_data, + :has_postcode, account_email: user.email, workflow_state: "contact_address_form") end @@ -56,12 +65,14 @@ context "when valid params are submitted" do let(:valid_params) { { - reg_identifier: transient_registration[:reg_identifier] + reg_identifier: transient_registration[:reg_identifier], + temp_address: "340116" } } it "updates the transient registration" do - # TODO: Add test once data is submitted through the form + post contact_address_forms_path, contact_address_form: valid_params + expect(transient_registration.reload.contact_address.uprn.to_s).to eq(valid_params[:temp_address]) end it "returns a 302 response" do @@ -73,6 +84,39 @@ post contact_address_forms_path, contact_address_form: valid_params expect(response).to redirect_to(new_check_your_answers_form_path(transient_registration[:reg_identifier])) end + + context "when the transient registration already has addresses" do + let(:transient_registration) do + create(:transient_registration, + :has_required_data, + :has_addresses, + account_email: user.email, + workflow_state: "contact_address_manual_form") + end + + it "should have have the same number of addresses before and after submitting" do + number_of_addresses = transient_registration.addresses.count + post contact_address_manual_forms_path, contact_address_manual_form: valid_params + expect(transient_registration.reload.addresses.count).to eq(number_of_addresses) + end + + it "removes the old contact address" do + old_contact_address = transient_registration.contact_address + post contact_address_manual_forms_path, contact_address_manual_form: valid_params + expect(transient_registration.reload.contact_address).to_not eq(old_contact_address) + end + + it "adds the new contact address" do + post contact_address_manual_forms_path, contact_address_manual_form: valid_params + expect(transient_registration.reload.contact_address.address_line_1).to eq(valid_params[:address_line_1]) + end + + it "does not modify the existing registered address" do + old_registered_address = transient_registration.registered_address + post contact_address_manual_forms_path, contact_address_manual_form: valid_params + expect(transient_registration.reload.registered_address).to eq(old_registered_address) + end + end end context "when invalid params are submitted" do @@ -98,6 +142,7 @@ let(:transient_registration) do create(:transient_registration, :has_required_data, + :has_postcode, account_email: user.email, workflow_state: "renewal_start_form") end @@ -109,7 +154,8 @@ } it "does not update the transient registration" do - # TODO: Add test once data is submitted through the form + post contact_address_forms_path, contact_address_form: valid_params + expect(transient_registration.reload.addresses.count).to eq(0) end it "returns a 302 response" do @@ -136,6 +182,7 @@ let(:transient_registration) do create(:transient_registration, :has_required_data, + :has_postcode, account_email: user.email, workflow_state: "contact_address_form") end @@ -157,6 +204,7 @@ let(:transient_registration) do create(:transient_registration, :has_required_data, + :has_postcode, account_email: user.email, workflow_state: "renewal_start_form") end @@ -175,4 +223,57 @@ end end end + + describe "GET skip_to_manual_address_contact_address_forms_path" do + context "when a valid user is signed in" do + let(:user) { create(:user) } + before(:each) do + sign_in(user) + end + + context "when a valid transient registration exists" do + let(:transient_registration) do + create(:transient_registration, + :has_required_data, + :has_postcode, + account_email: user.email, + workflow_state: "contact_address_form") + end + + context "when the skip_to_manual_address action is triggered" do + it "returns a 302 response" do + get skip_to_manual_address_contact_address_forms_path(transient_registration[:reg_identifier]) + expect(response).to have_http_status(302) + end + + it "redirects to the contact_address_manual form" do + get skip_to_manual_address_contact_address_forms_path(transient_registration[:reg_identifier]) + expect(response).to redirect_to(new_contact_address_manual_form_path(transient_registration[:reg_identifier])) + end + end + end + + context "when the transient registration is in the wrong state" do + let(:transient_registration) do + create(:transient_registration, + :has_required_data, + :has_postcode, + account_email: user.email, + workflow_state: "renewal_start_form") + end + + context "when the skip_to_manual_address action is triggered" do + it "returns a 302 response" do + get skip_to_manual_address_contact_address_forms_path(transient_registration[:reg_identifier]) + expect(response).to have_http_status(302) + end + + it "redirects to the correct form for the state" do + get skip_to_manual_address_contact_address_forms_path(transient_registration[:reg_identifier]) + expect(response).to redirect_to(new_renewal_start_form_path(transient_registration[:reg_identifier])) + end + end + end + end + end end From f329369b8858e8d16e094c704298004a67b30e25 Mon Sep 17 00:00:00 2001 From: irisfaraway Date: Tue, 10 Apr 2018 12:00:42 +0100 Subject: [PATCH 3/4] Refactor to inherit from AddressForm CompanyAddressForm and ContactAddressForm are very similar, so any shared methods should be inherited from the AddressForm instead. --- app/controllers/address_forms_controller.rb | 8 +++ .../company_address_forms_controller.rb | 9 +-- .../contact_address_forms_controller.rb | 9 +-- app/forms/address_form.rb | 68 +++++++++++++++++++ app/forms/company_address_form.rb | 48 ++----------- app/forms/contact_address_form.rb | 51 ++------------ 6 files changed, 92 insertions(+), 101 deletions(-) create mode 100644 app/controllers/address_forms_controller.rb create mode 100644 app/forms/address_form.rb diff --git a/app/controllers/address_forms_controller.rb b/app/controllers/address_forms_controller.rb new file mode 100644 index 000000000..d7083478d --- /dev/null +++ b/app/controllers/address_forms_controller.rb @@ -0,0 +1,8 @@ +class AddressFormsController < FormsController + def skip_to_manual_address + set_transient_registration(params[:reg_identifier]) + + @transient_registration.skip_to_manual_address! if form_matches_state? + redirect_to_correct_form + end +end diff --git a/app/controllers/company_address_forms_controller.rb b/app/controllers/company_address_forms_controller.rb index c46e24c6c..1ca7be8c5 100644 --- a/app/controllers/company_address_forms_controller.rb +++ b/app/controllers/company_address_forms_controller.rb @@ -1,4 +1,4 @@ -class CompanyAddressFormsController < FormsController +class CompanyAddressFormsController < AddressFormsController def new super(CompanyAddressForm, "company_address_form") end @@ -6,11 +6,4 @@ def new def create super(CompanyAddressForm, "company_address_form") end - - def skip_to_manual_address - set_transient_registration(params[:reg_identifier]) - - @transient_registration.skip_to_manual_address! if form_matches_state? - redirect_to_correct_form - end end diff --git a/app/controllers/contact_address_forms_controller.rb b/app/controllers/contact_address_forms_controller.rb index fb57f1208..539ddc47e 100644 --- a/app/controllers/contact_address_forms_controller.rb +++ b/app/controllers/contact_address_forms_controller.rb @@ -1,4 +1,4 @@ -class ContactAddressFormsController < FormsController +class ContactAddressFormsController < AddressFormsController def new super(ContactAddressForm, "contact_address_form") end @@ -6,11 +6,4 @@ def new def create super(ContactAddressForm, "contact_address_form") end - - def skip_to_manual_address - set_transient_registration(params[:reg_identifier]) - - @transient_registration.skip_to_manual_address! if form_matches_state? - redirect_to_correct_form - end end diff --git a/app/forms/address_form.rb b/app/forms/address_form.rb new file mode 100644 index 000000000..08e9f0de4 --- /dev/null +++ b/app/forms/address_form.rb @@ -0,0 +1,68 @@ +class AddressForm < BaseForm + attr_accessor :temp_addresses + attr_accessor :temp_address + attr_accessor :addresses + + def submit(params) + # Assign the params for validation and pass them to the BaseForm method for updating + self.addresses = add_or_replace_address(params[:temp_address]) + attributes = { addresses: addresses } + + super(attributes, params[:reg_identifier]) + end + + validates :addresses, presence: true + + private + + # Look up addresses based on the temp_postcode + def look_up_addresses + if temp_postcode.present? + address_finder = AddressFinderService.new(temp_postcode) + self.temp_addresses = address_finder.search_by_postcode + else + self.temp_addresses = [] + end + end + + # If an address has already been assigned to the transient registration, pre-select it + def preselect_existing_address + return unless @transient_registration.addresses.present? + return unless saved_address.uprn.present? + selected_address = temp_addresses.detect { |address| address["uprn"] == saved_address.uprn.to_s } + self.temp_address = selected_address["uprn"] if selected_address.present? + end + + def add_or_replace_address(selected_address_uprn) + return if selected_address_uprn.blank? + + data = temp_addresses.detect { |address| address["uprn"] == selected_address_uprn } + address = Address.create_from_os_places_data(data) + address.assign_attributes(address_type: address_type) + + # Update the transient object's nested addresses, replacing any existing address of the same type + updated_addresses = @transient_registration.addresses + updated_addresses.delete(saved_address) if saved_address + updated_addresses << address + updated_addresses + end + + # Methods which are called in this class but defined in subclasses + # We should throw descriptive errors in case an additional subclass of ManualAddressForm is ever added + + def temp_postcode + implemented_in_subclass + end + + def saved_address + implemented_in_subclass + end + + def address_type + implemented_in_subclass + end + + def implemented_in_subclass + raise NotImplementedError, "This #{self.class} cannot respond to:" + end +end diff --git a/app/forms/company_address_form.rb b/app/forms/company_address_form.rb index a3833f2b6..cc3919702 100644 --- a/app/forms/company_address_form.rb +++ b/app/forms/company_address_form.rb @@ -1,9 +1,6 @@ -class CompanyAddressForm < BaseForm +class CompanyAddressForm < AddressForm attr_accessor :business_type attr_accessor :temp_company_postcode - attr_accessor :temp_addresses - attr_accessor :temp_address - attr_accessor :addresses def initialize(transient_registration) super @@ -15,48 +12,17 @@ def initialize(transient_registration) preselect_existing_address end - def submit(params) - # Assign the params for validation and pass them to the BaseForm method for updating - self.addresses = add_or_replace_address(params[:temp_address]) - attributes = { addresses: addresses } - - super(attributes, params[:reg_identifier]) - end - - validates :addresses, presence: true - private - # Look up addresses based on the temp_company_postcode - def look_up_addresses - if temp_company_postcode.present? - address_finder = AddressFinderService.new(temp_company_postcode) - self.temp_addresses = address_finder.search_by_postcode - else - self.temp_addresses = [] - end + def temp_postcode + temp_company_postcode end - # If an address has already been assigned to the transient registration, pre-select it - def preselect_existing_address - return unless @transient_registration.addresses.present? - current_address = @transient_registration.registered_address - return unless current_address.uprn.present? - selected_address = temp_addresses.detect { |address| address["uprn"] == current_address.uprn.to_s } - self.temp_address = selected_address["uprn"] if selected_address.present? + def saved_address + @transient_registration.contact_address end - def add_or_replace_address(selected_address_uprn) - return if selected_address_uprn.blank? - - data = temp_addresses.detect { |address| address["uprn"] == selected_address_uprn } - address = Address.create_from_os_places_data(data) - address.assign_attributes(address_type: "REGISTERED") - - # Update the transient object's nested addresses, replacing any existing registered address - updated_addresses = @transient_registration.addresses - updated_addresses.delete(@transient_registration.registered_address) if @transient_registration.registered_address - updated_addresses << address - updated_addresses + def address_type + "REGISTERED" end end diff --git a/app/forms/contact_address_form.rb b/app/forms/contact_address_form.rb index f888c9802..3ce8e735b 100644 --- a/app/forms/contact_address_form.rb +++ b/app/forms/contact_address_form.rb @@ -1,62 +1,25 @@ -class ContactAddressForm < BaseForm - attr_accessor :business_type +class ContactAddressForm < AddressForm attr_accessor :temp_contact_postcode - attr_accessor :temp_addresses - attr_accessor :temp_address - attr_accessor :addresses def initialize(transient_registration) super - # We only use this for the correct microcopy - self.business_type = @transient_registration.business_type self.temp_contact_postcode = @transient_registration.temp_contact_postcode look_up_addresses preselect_existing_address end - def submit(params) - # Assign the params for validation and pass them to the BaseForm method for updating - self.addresses = add_or_replace_address(params[:temp_address]) - attributes = { addresses: addresses } - - super(attributes, params[:reg_identifier]) - end - - validates :addresses, presence: true - private - # Look up addresses based on the temp_contact_postcode - def look_up_addresses - if temp_contact_postcode.present? - address_finder = AddressFinderService.new(temp_contact_postcode) - self.temp_addresses = address_finder.search_by_postcode - else - self.temp_addresses = [] - end + def temp_postcode + temp_contact_postcode end - # If an address has already been assigned to the transient registration, pre-select it - def preselect_existing_address - return unless @transient_registration.addresses.present? - current_address = @transient_registration.contact_address - return unless current_address.uprn.present? - selected_address = temp_addresses.detect { |address| address["uprn"] == current_address.uprn.to_s } - self.temp_address = selected_address["uprn"] if selected_address.present? + def saved_address + @transient_registration.contact_address end - def add_or_replace_address(selected_address_uprn) - return if selected_address_uprn.blank? - - data = temp_addresses.detect { |address| address["uprn"] == selected_address_uprn } - address = Address.create_from_os_places_data(data) - address.assign_attributes(address_type: "CONTACT") - - # Update the transient object's nested addresses, replacing any existing registered address - updated_addresses = @transient_registration.addresses - updated_addresses.delete(@transient_registration.contact_address) if @transient_registration.contact_address - updated_addresses << address - updated_addresses + def address_type + "CONTACT" end end From b0b3c4cb79b226616ed50ee73016fe453170348d Mon Sep 17 00:00:00 2001 From: irisfaraway Date: Tue, 10 Apr 2018 12:24:23 +0100 Subject: [PATCH 4/4] Move address select to shared view --- app/views/company_address_forms/new.html.erb | 14 +------------- app/views/contact_address_forms/new.html.erb | 14 +------------- app/views/shared/_select_address.html.erb | 13 +++++++++++++ config/locales/en.yml | 6 ++++++ config/locales/forms/company_address_forms/en.yml | 5 ----- config/locales/forms/contact_address_forms/en.yml | 5 ----- 6 files changed, 21 insertions(+), 36 deletions(-) create mode 100644 app/views/shared/_select_address.html.erb diff --git a/app/views/company_address_forms/new.html.erb b/app/views/company_address_forms/new.html.erb index a77019ff4..fd59dd0bd 100644 --- a/app/views/company_address_forms/new.html.erb +++ b/app/views/company_address_forms/new.html.erb @@ -12,19 +12,7 @@ <%= link_to(t(".postcode_change_link"), back_company_address_forms_path(@company_address_form.reg_identifier)) %>
    - <% if @company_address_form.errors[:addresses].any? %> -
    - <% else %> -
    - <% end %> -
    - <%= f.label :temp_address, t(".address_label"), class: "form-label" %> - <%= f.select(:temp_address, - options_for_select(@company_address_form.temp_addresses.map { |a| [a["partial"], a["uprn"]] }, @company_address_form.temp_address), - { include_blank: t(".address_blank_option", count: @company_address_form.temp_addresses.length ) }, - { class: "form-control" }) %> -
    -
    + <%= render("shared/select_address", form: @company_address_form, f: f) %>
    <%= link_to(t(".manual_address_link"), skip_to_manual_address_company_address_forms_path(@company_address_form.reg_identifier)) %> diff --git a/app/views/contact_address_forms/new.html.erb b/app/views/contact_address_forms/new.html.erb index 3c92993d9..b890f67f0 100644 --- a/app/views/contact_address_forms/new.html.erb +++ b/app/views/contact_address_forms/new.html.erb @@ -12,19 +12,7 @@ <%= link_to(t(".postcode_change_link"), back_contact_address_forms_path(@contact_address_form.reg_identifier)) %>
    - <% if @contact_address_form.errors[:addresses].any? %> -
    - <% else %> -
    - <% end %> -
    - <%= f.label :temp_address, t(".address_label"), class: "form-label" %> - <%= f.select(:temp_address, - options_for_select(@contact_address_form.temp_addresses.map { |a| [a["partial"], a["uprn"]] }, @contact_address_form.temp_address), - { include_blank: t(".address_blank_option", count: @contact_address_form.temp_addresses.length ) }, - { class: "form-control" }) %> -
    -
    + <%= render("shared/select_address", form: @contact_address_form, f: f) %>
    <%= link_to(t(".manual_address_link"), skip_to_manual_address_contact_address_forms_path(@contact_address_form.reg_identifier)) %> diff --git a/app/views/shared/_select_address.html.erb b/app/views/shared/_select_address.html.erb new file mode 100644 index 000000000..a7c2f4c55 --- /dev/null +++ b/app/views/shared/_select_address.html.erb @@ -0,0 +1,13 @@ +<% if form.errors[:addresses].any? %> +
    +<% else %> +
    +<% end %> +
    + <%= f.label :temp_address, t(".address_label"), class: "form-label" %> + <%= f.select(:temp_address, + options_for_select(form.temp_addresses.map { |a| [a["partial"], a["uprn"]] }, form.temp_address), + { include_blank: t(".address_blank_option", count: form.temp_addresses.length ) }, + { class: "form-control" }) %> +
    +
    diff --git a/config/locales/en.yml b/config/locales/en.yml index 94bb4f640..f2a9b6c6d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -38,6 +38,12 @@ en: person_name: first_name: First name last_name: Last name + select_address: + address_label: Address + address_blank_option: + zero: No addresses found + one: 1 address found + other: "%{count} addresses found" # Custom error pages invalid_reg_identifier_heading: The registration number you entered is not valid diff --git a/config/locales/forms/company_address_forms/en.yml b/config/locales/forms/company_address_forms/en.yml index c81be0217..bacabea80 100644 --- a/config/locales/forms/company_address_forms/en.yml +++ b/config/locales/forms/company_address_forms/en.yml @@ -8,11 +8,6 @@ en: heading_soleTrader: What's the address of the business? postcode_label: Postcode postcode_change_link: Change - address_label: Address - address_blank_option: - zero: No addresses found - one: 1 address found - other: "%{count} addresses found" manual_address_link: "I can't find the address in the list" error_heading: Something is wrong next_button: Continue diff --git a/config/locales/forms/contact_address_forms/en.yml b/config/locales/forms/contact_address_forms/en.yml index 52b842f5c..2321999b2 100644 --- a/config/locales/forms/contact_address_forms/en.yml +++ b/config/locales/forms/contact_address_forms/en.yml @@ -4,11 +4,6 @@ en: heading: What's the address of the person we should contact? postcode_label: Postcode postcode_change_link: Change - address_label: Address - address_blank_option: - zero: No addresses found - one: 1 address found - other: "%{count} addresses found" manual_address_link: "I can't find the address in the list" error_heading: Something is wrong next_button: Continue