From c5f255c38436ad05b85c755760712d68ad572aff Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Tue, 8 Oct 2024 18:06:42 +0100 Subject: [PATCH 1/7] Drop custom can_reference_single_document_in_collection association --- .../can_have_registration_attributes.rb | 33 ++++++++-- ...reference_single_document_in_collection.rb | 63 ------------------- .../can_have_registration_attributes.rb | 11 +--- 3 files changed, 29 insertions(+), 78 deletions(-) delete mode 100644 app/models/concerns/waste_carriers_engine/can_reference_single_document_in_collection.rb diff --git a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb index e5a17e369..ba69a00d8 100644 --- a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb +++ b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb @@ -7,7 +7,6 @@ module WasteCarriersEngine module CanHaveRegistrationAttributes extend ActiveSupport::Concern include Mongoid::Document - include CanReferenceSingleDocumentInCollection include CanHaveTier # Rubocop sees a module as a block, and as such is not very forgiving in how @@ -23,11 +22,35 @@ module CanHaveRegistrationAttributes # rubocop:disable Layout/LineLength embeds_many :addresses, class_name: "WasteCarriersEngine::Address" - # This is our own custom association. See CanReferenceSingleDocumentInCollection for details - reference_one :contact_address, collection: :addresses, find_by: { address_type: "POSTAL" } - reference_one :company_address, collection: :addresses, find_by: { address_type: "REGISTERED" } # TODO: Remove this and always use `company_address` rather than `registrered_address` - reference_one :registered_address, collection: :addresses, find_by: { address_type: "REGISTERED" } + # embeds_one :address, as: :registered_address + + # Define helper accessor and assignment methods for each address type + %w[contact company postal registered].each do |address_type| + + define_method(:"#{address_type}_address") do + # handle synonyms + address_type = "postal" if address_type == "contact" + address_type = "registered" if address_type == "company" + + addresses.where(address_type: address_type.upcase).first + end + + define_method(:"#{address_type}_address=") do |address| + # handle synonyms + address_type = "postal" if address_type == "contact" + address_type = "registered" if address_type == "company" + + unless address.blank? || address.address_type == address_type.upcase + raise ArgumentError, "Attempted to set #{address_type} address with address of type #{address.address_type}" + end + + # clear any existing address of this type + addresses.where(address_type: address_type.upcase).first&.destroy! + + addresses << address + end + end embeds_one :conviction_search_result, class_name: "WasteCarriersEngine::ConvictionSearchResult" embeds_many :conviction_sign_offs, class_name: "WasteCarriersEngine::ConvictionSignOff" diff --git a/app/models/concerns/waste_carriers_engine/can_reference_single_document_in_collection.rb b/app/models/concerns/waste_carriers_engine/can_reference_single_document_in_collection.rb deleted file mode 100644 index 70f2bd0e6..000000000 --- a/app/models/concerns/waste_carriers_engine/can_reference_single_document_in_collection.rb +++ /dev/null @@ -1,63 +0,0 @@ -# frozen_string_literal: true - -# This module's aim is to implement a way to reference a single document in a -# collection so that they can then be treated as `has_one` associations. -# In projects using ActiveRecord like WEX we achieve the same functionality -# thanks to ActiveRecord's Relation ability to specify custom associations using -# default scopes. But because here we are using an old version of MongoDB, we -# are also stuck with a version of Mongoid which does not have this ability. -module WasteCarriersEngine - module CanReferenceSingleDocumentInCollection - extend ActiveSupport::Concern - - class_methods do - def reference_one(attribute_name, collection:, find_by:) - define_method(attribute_name) do - retrieve_attribute(attribute_name, collection, find_by) - end - - define_method("#{attribute_name}=") do |new_object| - assign_attribute(attribute_name, collection, new_object, find_by) - end - end - end - - included do - def retrieve_attribute(attribute_name, collection, find_by) - instance_variable_get("@#{attribute_name}") || - fetch_attribute(collection, find_by) - end - - def assign_attribute(attribute_name, collection, new_object, find_by) - # Feth the existing collection - new_collection = public_send(collection) || [] - - # Remove current object, if present, from the collection - new_collection -= [public_send(attribute_name)] - - if new_object.present? - # Assign the params that define this relation to the new object - find_by.each do |key, value| - new_object[key] = value - end - - # Add new object to the collection - new_collection << new_object - end - - # Assign the new collection - public_send("#{collection}=", new_collection) - end - - def fetch_attribute(collection, find_by) - public_send(collection).each do |element| - find_by.each do |key, value| - return element if element.public_send(key) == value - end - end - - nil - end - end - end -end diff --git a/spec/support/shared_examples/can_have_registration_attributes.rb b/spec/support/shared_examples/can_have_registration_attributes.rb index b0bf8eca0..85c93b5b9 100644 --- a/spec/support/shared_examples/can_have_registration_attributes.rb +++ b/spec/support/shared_examples/can_have_registration_attributes.rb @@ -3,15 +3,6 @@ RSpec.shared_examples "Can have registration attributes" do |factory:| let(:resource) { build(factory) } - include_examples( - "Can reference single document in collection", - proc { create(factory, :has_required_data, :has_addresses) }, - :contact_address, - proc { subject.addresses.find_by(address_type: "POSTAL") }, - WasteCarriersEngine::Address.new, - :addresses - ) - describe "#charity?" do test_values = { charity: true, @@ -186,7 +177,7 @@ end describe "#contact_address=" do - let(:contact_address) { build(:address) } + let(:contact_address) { build(:address, :contact) } let(:resource) { build(factory, addresses: []) } it "set an address of type contact" do From 328f649e902820dd6f7732aa5df737caba71aa94 Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Tue, 8 Oct 2024 18:07:14 +0100 Subject: [PATCH 2/7] Make address type mandatory on addresses --- .../waste_carriers_engine/address_lookup_form_base.rb | 4 ++-- .../waste_carriers_engine/company_address_manual_form.rb | 1 + .../waste_carriers_engine/contact_address_manual_form.rb | 1 + app/models/waste_carriers_engine/address.rb | 2 ++ config/locales/forms/company_address_forms/en.yml | 1 + config/locales/forms/contact_address_forms/en.yml | 1 + spec/models/waste_carriers_engine/registration_spec.rb | 8 ++++---- spec/support/shared_examples/search_scope.rb | 2 +- 8 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/forms/waste_carriers_engine/address_lookup_form_base.rb b/app/forms/waste_carriers_engine/address_lookup_form_base.rb index de7ff5096..4f0936c94 100644 --- a/app/forms/waste_carriers_engine/address_lookup_form_base.rb +++ b/app/forms/waste_carriers_engine/address_lookup_form_base.rb @@ -22,10 +22,10 @@ def request_matching_addresses end def create_address(uprn, type) - return {} if uprn.blank? + return nil if uprn.blank? data = temp_addresses.detect { |address| address["uprn"].to_i == uprn.to_i } - return {} unless data + return nil unless data address = Address.create_from_os_places_data(data) address.assign_attributes(address_type: type) diff --git a/app/forms/waste_carriers_engine/company_address_manual_form.rb b/app/forms/waste_carriers_engine/company_address_manual_form.rb index 651914310..a0842fcfc 100644 --- a/app/forms/waste_carriers_engine/company_address_manual_form.rb +++ b/app/forms/waste_carriers_engine/company_address_manual_form.rb @@ -23,6 +23,7 @@ def submit(params) def clean_address # Prefill the existing address unless the postcode has changed from the existing address's postcode transient_registration.company_address = Address.new( + address_type: "REGISTERED", postcode: transient_registration.temp_company_postcode ) end diff --git a/app/forms/waste_carriers_engine/contact_address_manual_form.rb b/app/forms/waste_carriers_engine/contact_address_manual_form.rb index a3f0291e1..b8c399a9b 100644 --- a/app/forms/waste_carriers_engine/contact_address_manual_form.rb +++ b/app/forms/waste_carriers_engine/contact_address_manual_form.rb @@ -23,6 +23,7 @@ def submit(params) def clean_address # Prefill the existing address unless the postcode has changed from the existing address's postcode transient_registration.contact_address = Address.new( + address_type: "POSTAL", postcode: transient_registration.temp_contact_postcode ) end diff --git a/app/models/waste_carriers_engine/address.rb b/app/models/waste_carriers_engine/address.rb index 67c6ca1b2..8b36c692e 100644 --- a/app/models/waste_carriers_engine/address.rb +++ b/app/models/waste_carriers_engine/address.rb @@ -34,6 +34,8 @@ class Address field :firstName, as: :first_name, type: String field :lastName, as: :last_name, type: String + validates :address_type, presence: true + def self.create_from_manual_entry(params, overseas) address = Address.new diff --git a/config/locales/forms/company_address_forms/en.yml b/config/locales/forms/company_address_forms/en.yml index 2af722977..8decfeeab 100644 --- a/config/locales/forms/company_address_forms/en.yml +++ b/config/locales/forms/company_address_forms/en.yml @@ -24,3 +24,4 @@ en: attributes: company_address: blank: "Select an address" + should_be_uk: "Provide a UK contact address" diff --git a/config/locales/forms/contact_address_forms/en.yml b/config/locales/forms/contact_address_forms/en.yml index 0a9476632..fed7d1e95 100644 --- a/config/locales/forms/contact_address_forms/en.yml +++ b/config/locales/forms/contact_address_forms/en.yml @@ -16,3 +16,4 @@ en: attributes: contact_address: blank: "Select an address" + should_be_uk: "Provide a UK contact address" diff --git a/spec/models/waste_carriers_engine/registration_spec.rb b/spec/models/waste_carriers_engine/registration_spec.rb index e6e2db893..f9ffd0c1c 100644 --- a/spec/models/waste_carriers_engine/registration_spec.rb +++ b/spec/models/waste_carriers_engine/registration_spec.rb @@ -284,7 +284,7 @@ module WasteCarriersEngine describe "#address" do context "when a registration has one address" do - let(:address) { build(:address, :has_required_data) } + let(:address) { build(:address, :registered, :has_required_data) } let(:registration) do build(:registration, :has_required_data, @@ -297,8 +297,8 @@ module WasteCarriersEngine end context "when a registration has multiple addresses" do - let(:address_a) { build(:address, :has_required_data) } - let(:address_b) { build(:address, :has_required_data) } + let(:address_a) { build(:address, :registered, :has_required_data) } + let(:address_b) { build(:address, :contact, :has_required_data) } let(:registration) do build(:registration, :has_required_data, @@ -321,7 +321,7 @@ module WasteCarriersEngine context "when registration has an address which has a location" do let(:location) { build(:location) } - let(:address) { build(:address, :has_required_data, location: location) } + let(:address) { build(:address, :registered, :has_required_data, location: location) } let(:registration) do build(:registration, :has_required_data, diff --git a/spec/support/shared_examples/search_scope.rb b/spec/support/shared_examples/search_scope.rb index 029293263..134e61497 100644 --- a/spec/support/shared_examples/search_scope.rb +++ b/spec/support/shared_examples/search_scope.rb @@ -69,7 +69,7 @@ let(:term) { "SW1A 2AA" } let(:matching_postcode_record) do - address = build(:address, postcode: term) + address = build(:address, :contact, postcode: term) create(factory, :has_required_data, addresses: [address]) end From 450d163ebc7fe58477d462b3ad8f49127317b55a Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:57:56 +0100 Subject: [PATCH 3/7] Retire address company_address alias in favour of registered_address --- .../company_address_forms_controller.rb | 2 +- ...company_address_manual_forms_controller.rb | 2 +- .../company_address_form.rb | 11 +++-- .../company_address_manual_form.rb | 12 +++--- .../can_have_registration_attributes.rb | 7 +--- .../company_address_forms/new.html.erb | 4 +- .../forms/company_address_forms/en.yml | 2 +- spec/factories/forms/company_address_form.rb | 2 +- .../company_address_forms_spec.rb | 4 +- .../company_address_manual_forms_spec.rb | 42 +++++++++---------- .../company_address_forms_spec.rb | 10 ++--- .../company_address_manual_forms_spec.rb | 4 +- .../registration_completion_service_spec.rb | 2 +- 13 files changed, 49 insertions(+), 55 deletions(-) diff --git a/app/controllers/waste_carriers_engine/company_address_forms_controller.rb b/app/controllers/waste_carriers_engine/company_address_forms_controller.rb index 34a9d4bd0..59307c083 100644 --- a/app/controllers/waste_carriers_engine/company_address_forms_controller.rb +++ b/app/controllers/waste_carriers_engine/company_address_forms_controller.rb @@ -13,7 +13,7 @@ def create private def transient_registration_attributes - params.fetch(:company_address_form, {}).permit(company_address: [:uprn]) + params.fetch(:company_address_form, {}).permit(registered_address: [:uprn]) end end end diff --git a/app/controllers/waste_carriers_engine/company_address_manual_forms_controller.rb b/app/controllers/waste_carriers_engine/company_address_manual_forms_controller.rb index 38360d412..8a485a4df 100644 --- a/app/controllers/waste_carriers_engine/company_address_manual_forms_controller.rb +++ b/app/controllers/waste_carriers_engine/company_address_manual_forms_controller.rb @@ -16,7 +16,7 @@ def transient_registration_attributes params .fetch(:company_address_manual_form, {}) .permit( - company_address: %i[house_number address_line_1 address_line_2 town_city postcode country] + registered_address: %i[house_number address_line_1 address_line_2 town_city postcode country] ) end diff --git a/app/forms/waste_carriers_engine/company_address_form.rb b/app/forms/waste_carriers_engine/company_address_form.rb index 3deea7726..7d759acc0 100644 --- a/app/forms/waste_carriers_engine/company_address_form.rb +++ b/app/forms/waste_carriers_engine/company_address_form.rb @@ -2,18 +2,17 @@ module WasteCarriersEngine class CompanyAddressForm < AddressLookupFormBase - delegate :temp_company_postcode, :business_type, :company_address, to: :transient_registration + delegate :temp_company_postcode, :business_type, :registered_address, to: :transient_registration - alias existing_address company_address alias postcode temp_company_postcode - validates :company_address, "waste_carriers_engine/address": true + validates :registered_address, "waste_carriers_engine/address": true def submit(params) - company_address_params = params.fetch(:company_address, {}) - company_address = create_address(company_address_params[:uprn], "REGISTERED") + address_params = params.fetch(:registered_address, {}) + registered_address = create_address(address_params[:uprn], "REGISTERED") - super(company_address: company_address) + super(registered_address:) end end end diff --git a/app/forms/waste_carriers_engine/company_address_manual_form.rb b/app/forms/waste_carriers_engine/company_address_manual_form.rb index a0842fcfc..3f1ab1c38 100644 --- a/app/forms/waste_carriers_engine/company_address_manual_form.rb +++ b/app/forms/waste_carriers_engine/company_address_manual_form.rb @@ -5,24 +5,24 @@ class CompanyAddressManualForm < ::WasteCarriersEngine::BaseForm include CanClearAddressFinderError include CanValidateManualAddress - delegate :overseas?, :company_address, :business_type, to: :transient_registration - delegate :house_number, :address_line_1, :address_line_2, to: :company_address, allow_nil: true - delegate :postcode, :town_city, :country, to: :company_address, allow_nil: true + delegate :overseas?, :registered_address, :business_type, to: :transient_registration + delegate :house_number, :address_line_1, :address_line_2, to: :registered_address, allow_nil: true + delegate :postcode, :town_city, :country, to: :registered_address, allow_nil: true after_initialize :clean_address, unless: :saved_address_still_valid? def submit(params) - address = Address.create_from_manual_entry(params[:company_address] || {}, transient_registration.overseas?) + address = Address.create_from_manual_entry(params[:registered_address] || {}, transient_registration.overseas?) address.assign_attributes(address_type: "REGISTERED") - super(company_address: address) + super(registered_address: address) end private def clean_address # Prefill the existing address unless the postcode has changed from the existing address's postcode - transient_registration.company_address = Address.new( + transient_registration.registered_address = Address.new( address_type: "REGISTERED", postcode: transient_registration.temp_company_postcode ) diff --git a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb index ba69a00d8..5d67f0529 100644 --- a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb +++ b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb @@ -22,16 +22,12 @@ module CanHaveRegistrationAttributes # rubocop:disable Layout/LineLength embeds_many :addresses, class_name: "WasteCarriersEngine::Address" - # TODO: Remove this and always use `company_address` rather than `registrered_address` - # embeds_one :address, as: :registered_address - # Define helper accessor and assignment methods for each address type - %w[contact company postal registered].each do |address_type| + %w[contact postal registered].each do |address_type| define_method(:"#{address_type}_address") do # handle synonyms address_type = "postal" if address_type == "contact" - address_type = "registered" if address_type == "company" addresses.where(address_type: address_type.upcase).first end @@ -39,7 +35,6 @@ module CanHaveRegistrationAttributes define_method(:"#{address_type}_address=") do |address| # handle synonyms address_type = "postal" if address_type == "contact" - address_type = "registered" if address_type == "company" unless address.blank? || address.address_type == address_type.upcase raise ArgumentError, "Attempted to set #{address_type} address with address of type #{address.address_type}" diff --git a/app/views/waste_carriers_engine/company_address_forms/new.html.erb b/app/views/waste_carriers_engine/company_address_forms/new.html.erb index a50ca529e..ca21c68d7 100644 --- a/app/views/waste_carriers_engine/company_address_forms/new.html.erb +++ b/app/views/waste_carriers_engine/company_address_forms/new.html.erb @@ -22,8 +22,8 @@ go_back_forms_path(@company_address_form.token)) %>

- <%= form.fields_for :company_address do |f| %> - <%= render("waste_carriers_engine/shared/select_address", form: @company_address_form, address: :company_address, f: f) %> + <%= form.fields_for :registered_address do |f| %> + <%= render("waste_carriers_engine/shared/select_address", form: @company_address_form, address: :registered_address, f: f) %> <% end %>

diff --git a/config/locales/forms/company_address_forms/en.yml b/config/locales/forms/company_address_forms/en.yml index 8decfeeab..f2ccb0cc7 100644 --- a/config/locales/forms/company_address_forms/en.yml +++ b/config/locales/forms/company_address_forms/en.yml @@ -22,6 +22,6 @@ en: models: waste_carriers_engine/company_address_form: attributes: - company_address: + registered_address: blank: "Select an address" should_be_uk: "Provide a UK contact address" diff --git a/spec/factories/forms/company_address_form.rb b/spec/factories/forms/company_address_form.rb index c1faa4b40..e28e3957e 100644 --- a/spec/factories/forms/company_address_form.rb +++ b/spec/factories/forms/company_address_form.rb @@ -9,7 +9,7 @@ :renewing_registration, :has_required_data, workflow_state: "company_address_form", - company_address: build(:address, :has_required_data, :registered), + registered_address: build(:address, :has_required_data, :registered), temp_company_postcode: "FA4 3HT" ) ) diff --git a/spec/forms/waste_carriers_engine/company_address_forms_spec.rb b/spec/forms/waste_carriers_engine/company_address_forms_spec.rb index ff6cce137..78a0bdc8b 100644 --- a/spec/forms/waste_carriers_engine/company_address_forms_spec.rb +++ b/spec/forms/waste_carriers_engine/company_address_forms_spec.rb @@ -14,7 +14,7 @@ module WasteCarriersEngine let(:valid_params) do { token: company_address_form.token, - company_address: { + registered_address: { uprn: "340116" } } @@ -63,7 +63,7 @@ module WasteCarriersEngine describe "#temp_address" do it "pre-selects the address" do - expect(company_address_form.company_address.uprn.to_s).to eq(transient_registration.addresses.where(address_type: "REGISTERED").first.uprn.to_s) + expect(company_address_form.registered_address.uprn.to_s).to eq(transient_registration.addresses.where(address_type: "REGISTERED").first.uprn.to_s) end end end diff --git a/spec/forms/waste_carriers_engine/company_address_manual_forms_spec.rb b/spec/forms/waste_carriers_engine/company_address_manual_forms_spec.rb index 2ef1e6acb..01b0bcf65 100644 --- a/spec/forms/waste_carriers_engine/company_address_manual_forms_spec.rb +++ b/spec/forms/waste_carriers_engine/company_address_manual_forms_spec.rb @@ -6,13 +6,13 @@ module WasteCarriersEngine RSpec.describe CompanyAddressManualForm do describe "#initialize" do context "when the transient registration has an address already" do - let(:company_address) { build(:address, :registered, :has_required_data) } + let(:registered_address) { build(:address, :registered, :has_required_data) } let(:transient_registration) do build( :renewing_registration, :has_required_data, workflow_state: "company_address_manual_form", - company_address: company_address + registered_address: registered_address ) end # Don't use FactoryBot for this as we need to make sure it initializes with a specific object @@ -25,7 +25,7 @@ module WasteCarriersEngine end it "prefills the form with the existing address" do - expect(company_address_manual_form.house_number).to eq(company_address.house_number) + expect(company_address_manual_form.house_number).to eq(registered_address.house_number) end end @@ -35,17 +35,17 @@ module WasteCarriersEngine end it "prefills the form with the existing address" do - expect(company_address_manual_form.house_number).to eq(company_address.house_number) + expect(company_address_manual_form.house_number).to eq(registered_address.house_number) end end context "when the temp_company_postcode matches the existing address" do before do - transient_registration.temp_company_postcode = company_address.postcode + transient_registration.temp_company_postcode = registered_address.postcode end it "prefills the form with the existing address" do - expect(company_address_manual_form.house_number).to eq(company_address.house_number) + expect(company_address_manual_form.house_number).to eq(registered_address.house_number) end end @@ -59,7 +59,7 @@ module WasteCarriersEngine end it "does not prefill the form with the existing address" do - expect(company_address_manual_form.house_number).not_to eq(company_address.address_line_1) + expect(company_address_manual_form.house_number).not_to eq(registered_address.address_line_1) end end end @@ -71,7 +71,7 @@ module WasteCarriersEngine let(:valid_params) do { token: company_address_manual_form.token, - company_address: { + registered_address: { house_number: "32", address_line_1: "My House Road", town_city: "London", @@ -97,14 +97,14 @@ module WasteCarriersEngine end context "when a valid company address exists and is still valid" do - let(:company_address) { build(:address, :company, :has_required_data) } + let(:registered_address) { build(:address, :company, :has_required_data) } let(:transient_registration) do build( :renewing_registration, :has_required_data, workflow_state: "company_address_manual_form", - company_address: company_address, - temp_company_postcode: company_address.postcode + registered_address: registered_address, + temp_company_postcode: registered_address.postcode ) end # Don't use FactoryBot for this as we need to make sure it initializes with a specific object @@ -119,7 +119,7 @@ module WasteCarriersEngine describe "#house_number" do context "when the house_number is blank" do before do - transient_registration.company_address.house_number = nil + transient_registration.registered_address.house_number = nil end it "is not valid" do @@ -129,7 +129,7 @@ module WasteCarriersEngine context "when the house_number is too long" do before do - transient_registration.company_address.house_number = "1767217672701701770041563533191862858216711534112759290091467119071305962652874388673510958015949702771617744287044629700938926040595129731732659267801150029749795917354487895716341751221579349683254601" + transient_registration.registered_address.house_number = "1767217672701701770041563533191862858216711534112759290091467119071305962652874388673510958015949702771617744287044629700938926040595129731732659267801150029749795917354487895716341751221579349683254601" end it "is not valid" do @@ -141,7 +141,7 @@ module WasteCarriersEngine describe "#address_line_1" do context "when the address_line_1 is blank" do before do - transient_registration.company_address.address_line_1 = nil + transient_registration.registered_address.address_line_1 = nil end it "is not valid" do @@ -151,7 +151,7 @@ module WasteCarriersEngine context "when the address_line_1 is too long" do before do - transient_registration.company_address.address_line_1 = "dj2mpm1gioexmhxsomk9o7oo8h5c7y7o8j2pmnwxefvoy91v9ghm7saz10r2lmdqhl3r6of58qlmlar2qeepah8c9rs8i78s2j94ws6y0gq1mxy4cw6s5myjugw62er6d2gpai0b11gsb18s2sfb9rcllye22b38o4" + transient_registration.registered_address.address_line_1 = "dj2mpm1gioexmhxsomk9o7oo8h5c7y7o8j2pmnwxefvoy91v9ghm7saz10r2lmdqhl3r6of58qlmlar2qeepah8c9rs8i78s2j94ws6y0gq1mxy4cw6s5myjugw62er6d2gpai0b11gsb18s2sfb9rcllye22b38o4" end it "is not valid" do @@ -163,7 +163,7 @@ module WasteCarriersEngine describe "#address_line_2" do context "when the address_line_2 is too long" do before do - transient_registration.company_address.address_line_2 = "gsm2lgu3q7cg5pcs02ftc1wtpx4lt5ghmyaclhe9qg9li7ibs5ldi3w3n1pt24pbfo0666bq" + transient_registration.registered_address.address_line_2 = "gsm2lgu3q7cg5pcs02ftc1wtpx4lt5ghmyaclhe9qg9li7ibs5ldi3w3n1pt24pbfo0666bq" end it "is not valid" do @@ -175,7 +175,7 @@ module WasteCarriersEngine describe "#town_city" do context "when the town_city is blank" do before do - transient_registration.company_address.town_city = nil + transient_registration.registered_address.town_city = nil end it "is not valid" do @@ -185,7 +185,7 @@ module WasteCarriersEngine context "when the town_city is too long" do before do - transient_registration.company_address.town_city = "4jhjdq46425oqers8r0b0xejkl19bapc" + transient_registration.registered_address.town_city = "4jhjdq46425oqers8r0b0xejkl19bapc" end it "is not valid" do @@ -197,7 +197,7 @@ module WasteCarriersEngine describe "#postcode" do context "when the postcode is too long" do before do - transient_registration.company_address.postcode = "4jhjdq46425oqers8r0b0xejkl19bapc" + transient_registration.registered_address.postcode = "4jhjdq46425oqers8r0b0xejkl19bapc" end it "is not valid" do @@ -209,7 +209,7 @@ module WasteCarriersEngine describe "#country" do context "when the country is blank" do before do - transient_registration.company_address.country = nil + transient_registration.registered_address.country = nil end context "when the business is not overseas" do @@ -237,7 +237,7 @@ module WasteCarriersEngine context "when the country is too long" do before do - transient_registration.company_address.country = "f8x4jhjdq46425oqers8r0b0xejkl19bapc4jhjdq46425oqers" + transient_registration.registered_address.country = "f8x4jhjdq46425oqers8r0b0xejkl19bapc4jhjdq46425oqers" end it "is not valid" do diff --git a/spec/requests/waste_carriers_engine/company_address_forms_spec.rb b/spec/requests/waste_carriers_engine/company_address_forms_spec.rb index 9e05e8b47..cb581d4e9 100644 --- a/spec/requests/waste_carriers_engine/company_address_forms_spec.rb +++ b/spec/requests/waste_carriers_engine/company_address_forms_spec.rb @@ -26,7 +26,7 @@ module WasteCarriersEngine let(:valid_params) do { token: transient_registration[:token], - company_address: { + registered_address: { uprn: "340116" } } @@ -35,7 +35,7 @@ module WasteCarriersEngine it "updates the transient registration and returns a 302 response" do post company_address_forms_path(transient_registration.token), params: { company_address_form: valid_params } - expect(transient_registration.reload.company_address.uprn.to_s).to eq("340116") + expect(transient_registration.reload.registered_address.uprn.to_s).to eq("340116") expect(response).to have_http_status(:found) end @@ -73,7 +73,7 @@ module WasteCarriersEngine post company_address_forms_path(transient_registration.token), params: { company_address_form: valid_params } expect(transient_registration.reload.addresses.count).to eq(number_of_addresses) - expect(transient_registration.reload.company_address.uprn).to eq(340_116) + expect(transient_registration.reload.registered_address.uprn).to eq(340_116) end end end @@ -102,11 +102,11 @@ module WasteCarriersEngine end it "does not update the transient registration, returns a 302 response and redirects to the correct form for the state" do - old_company_address = transient_registration.company_address + old_company_address = transient_registration.registered_address post company_address_forms_path(transient_registration.token), params: { company_address_form: valid_params } - expect(transient_registration.reload.company_address).to eq old_company_address + expect(transient_registration.reload.registered_address).to eq old_company_address expect(response).to have_http_status(:found) expect(response).to redirect_to(new_renewal_start_form_path(transient_registration[:token])) end diff --git a/spec/requests/waste_carriers_engine/company_address_manual_forms_spec.rb b/spec/requests/waste_carriers_engine/company_address_manual_forms_spec.rb index 15357c91d..71876b5ca 100644 --- a/spec/requests/waste_carriers_engine/company_address_manual_forms_spec.rb +++ b/spec/requests/waste_carriers_engine/company_address_manual_forms_spec.rb @@ -20,7 +20,7 @@ module WasteCarriersEngine context "when valid params are submitted" do let(:valid_params) do { - company_address: { + registered_address: { house_number: "41", address_line_1: "Foo Terrace", town_city: "Barton" @@ -89,7 +89,7 @@ module WasteCarriersEngine let(:valid_params) do { - company_address: { + registered_address: { house_number: "42", address_line_1: "Foo Terrace", town_city: "Barton" diff --git a/spec/services/waste_carriers_engine/registration_completion_service_spec.rb b/spec/services/waste_carriers_engine/registration_completion_service_spec.rb index f8308c4e3..4a6814d36 100644 --- a/spec/services/waste_carriers_engine/registration_completion_service_spec.rb +++ b/spec/services/waste_carriers_engine/registration_completion_service_spec.rb @@ -81,7 +81,7 @@ module WasteCarriersEngine expect { complete_registration }.to change(WasteCarriersEngine::Registration, :count).by(1) end - it { expect(complete_registration.company_address).to eq(transient_registration.company_address) } + it { expect(complete_registration.registered_address).to eq(transient_registration.registered_address) } it { expect(complete_registration.expires_on).to be_present } it { expect(complete_registration.finance_details.orders.count).to eq(1) } it { expect(complete_registration.finance_details.payments.count).to eq(1) } From 1454317c38155fd3ea39fac1f9ec9e5d6e2a87bc Mon Sep 17 00:00:00 2001 From: PaulDoyle-EA <97455399+PaulDoyle-EA@users.noreply.github.com> Date: Thu, 10 Oct 2024 16:23:44 +0100 Subject: [PATCH 4/7] Revert address area change to not update area attribute (#1575) https://eaflood.atlassian.net/browse/RUBY-3411 --- app/models/waste_carriers_engine/address.rb | 2 +- spec/models/waste_carriers_engine/address_spec.rb | 2 +- .../update_address_details_from_os_places_service_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/waste_carriers_engine/address.rb b/app/models/waste_carriers_engine/address.rb index 8b36c692e..38aa64b7e 100644 --- a/app/models/waste_carriers_engine/address.rb +++ b/app/models/waste_carriers_engine/address.rb @@ -63,7 +63,7 @@ def update_from_os_places_data(data) self[:uprn] = data["uprn"] self[:address_mode] = "address-results" self[:dependent_locality] = data["dependentLocality"] - self[:area] = data["administrativeArea"] + self[:administrative_area] = data["administrativeArea"] self[:town_city] = data["town"] self[:postcode] = data["postcode"] self[:country] = data["country"] diff --git a/spec/models/waste_carriers_engine/address_spec.rb b/spec/models/waste_carriers_engine/address_spec.rb index 0e7589503..2bc3d3c35 100644 --- a/spec/models/waste_carriers_engine/address_spec.rb +++ b/spec/models/waste_carriers_engine/address_spec.rb @@ -102,7 +102,7 @@ module WasteCarriersEngine "postcode" => os_places_data["postcode"], "country" => os_places_data["country"], "dependentLocality" => os_places_data["dependentLocality"], - "area" => os_places_data["administrativeArea"], + "administrativeArea" => os_places_data["administrativeArea"], "localAuthorityUpdateDate" => os_places_data["localAuthorityUpdateDate"], "easting" => os_places_data["easting"].to_i, "northing" => os_places_data["northing"].to_i, diff --git a/spec/services/waste_carriers_engine/update_address_details_from_os_places_service_spec.rb b/spec/services/waste_carriers_engine/update_address_details_from_os_places_service_spec.rb index 7df6403a0..b8a6d5155 100644 --- a/spec/services/waste_carriers_engine/update_address_details_from_os_places_service_spec.rb +++ b/spec/services/waste_carriers_engine/update_address_details_from_os_places_service_spec.rb @@ -59,7 +59,7 @@ module WasteCarriersEngine it { expect(address.uprn).to eq 340_116 } it { expect(address.address_mode).to eq "address-results" } it { expect(address.dependent_locality).to eq "SOUTH BRISTOL" } - it { expect(address.area).to eq("BRISTOL") } + it { expect(address.administrative_area).to eq("BRISTOL") } it { expect(address.town_city).to eq "BRISTOL" } it { expect(address.postcode).to eq "BS1 5AH" } it { expect(address.country).to eq "" } From b6ab8df95bec921d7945470aa11c93143639efe8 Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Wed, 16 Oct 2024 13:47:33 +0100 Subject: [PATCH 5/7] Remove contact address type alias "postal" --- app/forms/waste_carriers_engine/contact_address_form.rb | 2 +- .../waste_carriers_engine/contact_address_manual_form.rb | 4 ++-- .../can_have_registration_attributes.rb | 8 +------- .../contact_address_as_registered_address_service.rb | 2 +- spec/factories/address.rb | 2 +- .../contact_address_as_registered_address_service_spec.rb | 4 ++-- 6 files changed, 8 insertions(+), 14 deletions(-) diff --git a/app/forms/waste_carriers_engine/contact_address_form.rb b/app/forms/waste_carriers_engine/contact_address_form.rb index daf3cf9e6..d1ae178f3 100644 --- a/app/forms/waste_carriers_engine/contact_address_form.rb +++ b/app/forms/waste_carriers_engine/contact_address_form.rb @@ -11,7 +11,7 @@ class ContactAddressForm < AddressLookupFormBase def submit(params) contact_address_params = params.fetch(:contact_address, {}) - contact_address = create_address(contact_address_params[:uprn], "POSTAL") + contact_address = create_address(contact_address_params[:uprn], "CONTACT") super(contact_address: contact_address) end diff --git a/app/forms/waste_carriers_engine/contact_address_manual_form.rb b/app/forms/waste_carriers_engine/contact_address_manual_form.rb index b8c399a9b..1ba7cd543 100644 --- a/app/forms/waste_carriers_engine/contact_address_manual_form.rb +++ b/app/forms/waste_carriers_engine/contact_address_manual_form.rb @@ -13,7 +13,7 @@ class ContactAddressManualForm < ::WasteCarriersEngine::BaseForm def submit(params) address = Address.create_from_manual_entry(params[:contact_address] || {}, transient_registration.overseas?) - address.assign_attributes(address_type: "POSTAL") + address.assign_attributes(address_type: "CONTACT") super(contact_address: address) end @@ -23,7 +23,7 @@ def submit(params) def clean_address # Prefill the existing address unless the postcode has changed from the existing address's postcode transient_registration.contact_address = Address.new( - address_type: "POSTAL", + address_type: "CONTACT", postcode: transient_registration.temp_contact_postcode ) end diff --git a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb index 5d67f0529..0e5f2fcff 100644 --- a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb +++ b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb @@ -23,19 +23,13 @@ module CanHaveRegistrationAttributes embeds_many :addresses, class_name: "WasteCarriersEngine::Address" # Define helper accessor and assignment methods for each address type - %w[contact postal registered].each do |address_type| + %w[contact registered].each do |address_type| define_method(:"#{address_type}_address") do - # handle synonyms - address_type = "postal" if address_type == "contact" - addresses.where(address_type: address_type.upcase).first end define_method(:"#{address_type}_address=") do |address| - # handle synonyms - address_type = "postal" if address_type == "contact" - unless address.blank? || address.address_type == address_type.upcase raise ArgumentError, "Attempted to set #{address_type} address with address of type #{address.address_type}" end diff --git a/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb b/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb index 8a92ab476..18564dc5c 100644 --- a/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb +++ b/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb @@ -8,7 +8,7 @@ def run(transient_registration) return unless @transient_registration.registered_address cloned_address = @transient_registration.registered_address.clone - cloned_address.addressType = "POSTAL" + cloned_address.addressType = "CONTACT" @transient_registration.update(contact_address: cloned_address) end diff --git a/spec/factories/address.rb b/spec/factories/address.rb index 92d2a30a3..91bf031ad 100644 --- a/spec/factories/address.rb +++ b/spec/factories/address.rb @@ -11,7 +11,7 @@ end trait :contact do - address_type { "POSTAL" } + address_type { "CONTACT" } end trait :registered do diff --git a/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb b/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb index 05f948d5c..52b5cadd7 100644 --- a/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb +++ b/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb @@ -26,8 +26,8 @@ module WasteCarriersEngine expect(contact_address.postcode).to eq(registered_address.postcode) end - it "assigns the POSTAL addressType to the new contact_address" do - expect(contact_address.address_type).to eq("POSTAL") + it "assigns the CONTACTaddressType to the new contact_address" do + expect(contact_address.address_type).to eq("CONTACT") end end From 35c410a0ad9910dc548e0148be295ee423fc6277 Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Wed, 16 Oct 2024 15:32:37 +0100 Subject: [PATCH 6/7] Revert "Remove contact address type alias "postal"" This reverts commit b6ab8df95bec921d7945470aa11c93143639efe8. --- app/forms/waste_carriers_engine/contact_address_form.rb | 2 +- .../waste_carriers_engine/contact_address_manual_form.rb | 4 ++-- .../can_have_registration_attributes.rb | 8 +++++++- .../contact_address_as_registered_address_service.rb | 2 +- spec/factories/address.rb | 2 +- .../contact_address_as_registered_address_service_spec.rb | 4 ++-- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/forms/waste_carriers_engine/contact_address_form.rb b/app/forms/waste_carriers_engine/contact_address_form.rb index d1ae178f3..daf3cf9e6 100644 --- a/app/forms/waste_carriers_engine/contact_address_form.rb +++ b/app/forms/waste_carriers_engine/contact_address_form.rb @@ -11,7 +11,7 @@ class ContactAddressForm < AddressLookupFormBase def submit(params) contact_address_params = params.fetch(:contact_address, {}) - contact_address = create_address(contact_address_params[:uprn], "CONTACT") + contact_address = create_address(contact_address_params[:uprn], "POSTAL") super(contact_address: contact_address) end diff --git a/app/forms/waste_carriers_engine/contact_address_manual_form.rb b/app/forms/waste_carriers_engine/contact_address_manual_form.rb index 1ba7cd543..b8c399a9b 100644 --- a/app/forms/waste_carriers_engine/contact_address_manual_form.rb +++ b/app/forms/waste_carriers_engine/contact_address_manual_form.rb @@ -13,7 +13,7 @@ class ContactAddressManualForm < ::WasteCarriersEngine::BaseForm def submit(params) address = Address.create_from_manual_entry(params[:contact_address] || {}, transient_registration.overseas?) - address.assign_attributes(address_type: "CONTACT") + address.assign_attributes(address_type: "POSTAL") super(contact_address: address) end @@ -23,7 +23,7 @@ def submit(params) def clean_address # Prefill the existing address unless the postcode has changed from the existing address's postcode transient_registration.contact_address = Address.new( - address_type: "CONTACT", + address_type: "POSTAL", postcode: transient_registration.temp_contact_postcode ) end diff --git a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb index 0e5f2fcff..5d67f0529 100644 --- a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb +++ b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb @@ -23,13 +23,19 @@ module CanHaveRegistrationAttributes embeds_many :addresses, class_name: "WasteCarriersEngine::Address" # Define helper accessor and assignment methods for each address type - %w[contact registered].each do |address_type| + %w[contact postal registered].each do |address_type| define_method(:"#{address_type}_address") do + # handle synonyms + address_type = "postal" if address_type == "contact" + addresses.where(address_type: address_type.upcase).first end define_method(:"#{address_type}_address=") do |address| + # handle synonyms + address_type = "postal" if address_type == "contact" + unless address.blank? || address.address_type == address_type.upcase raise ArgumentError, "Attempted to set #{address_type} address with address of type #{address.address_type}" end diff --git a/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb b/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb index 18564dc5c..8a92ab476 100644 --- a/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb +++ b/app/services/waste_carriers_engine/contact_address_as_registered_address_service.rb @@ -8,7 +8,7 @@ def run(transient_registration) return unless @transient_registration.registered_address cloned_address = @transient_registration.registered_address.clone - cloned_address.addressType = "CONTACT" + cloned_address.addressType = "POSTAL" @transient_registration.update(contact_address: cloned_address) end diff --git a/spec/factories/address.rb b/spec/factories/address.rb index 91bf031ad..92d2a30a3 100644 --- a/spec/factories/address.rb +++ b/spec/factories/address.rb @@ -11,7 +11,7 @@ end trait :contact do - address_type { "CONTACT" } + address_type { "POSTAL" } end trait :registered do diff --git a/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb b/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb index 52b5cadd7..05f948d5c 100644 --- a/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb +++ b/spec/services/waste_carriers_engine/contact_address_as_registered_address_service_spec.rb @@ -26,8 +26,8 @@ module WasteCarriersEngine expect(contact_address.postcode).to eq(registered_address.postcode) end - it "assigns the CONTACTaddressType to the new contact_address" do - expect(contact_address.address_type).to eq("CONTACT") + it "assigns the POSTAL addressType to the new contact_address" do + expect(contact_address.address_type).to eq("POSTAL") end end From f08189a1eda5b9c668c4da389901d8750ce24921 Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Wed, 16 Oct 2024 17:30:51 +0100 Subject: [PATCH 7/7] Drop redundant helper for postal_address --- .../waste_carriers_engine/can_have_registration_attributes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb index 5d67f0529..4466c1b01 100644 --- a/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb +++ b/app/models/concerns/waste_carriers_engine/can_have_registration_attributes.rb @@ -23,7 +23,7 @@ module CanHaveRegistrationAttributes embeds_many :addresses, class_name: "WasteCarriersEngine::Address" # Define helper accessor and assignment methods for each address type - %w[contact postal registered].each do |address_type| + %w[contact registered].each do |address_type| define_method(:"#{address_type}_address") do # handle synonyms