From 5d74b2a2cc7717504d114d3afe86864aa34e8aae Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Mon, 12 Feb 2024 19:33:57 +0000 Subject: [PATCH 1/5] Remove conflict between reference_number method and reference_number association --- app/models/flood_risk_engine/enrollment.rb | 7 ++----- .../notify/enrollment_submitted_email_service.rb | 2 +- .../flood_risk_engine/enrollment_mailer/submitted.html.erb | 2 +- .../flood_risk_engine/enrollment_mailer/submitted.text.erb | 2 +- .../registration_complete_forms/new.html.erb | 2 +- spec/models/flood_risk_engine/enrollment_spec.rb | 3 +-- .../notify/enrollment_submitted_email_service_spec.rb | 2 +- .../registration_completion_service_spec.rb | 4 ++-- 8 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/models/flood_risk_engine/enrollment.rb b/app/models/flood_risk_engine/enrollment.rb index 699f9034..c60bff94 100644 --- a/app/models/flood_risk_engine/enrollment.rb +++ b/app/models/flood_risk_engine/enrollment.rb @@ -4,9 +4,6 @@ module FloodRiskEngine class Enrollment < ApplicationRecord has_secure_token - # We don't define the inverse relationship of applicant_contact as, in WEX at least, - # we query never from contact to its enrollment - belongs_to :applicant_contact, class_name: "Contact" belongs_to :organisation delegate :org_type, :partners, to: :organisation, allow_nil: true @@ -22,8 +19,8 @@ class Enrollment < ApplicationRecord belongs_to :secondary_contact, class_name: "Contact" belongs_to :reference_number - def reference_number - super.try(:number) + def ref_number + reference_number.number end has_one( diff --git a/app/services/flood_risk_engine/notify/enrollment_submitted_email_service.rb b/app/services/flood_risk_engine/notify/enrollment_submitted_email_service.rb index 22989681..de0a0d2c 100644 --- a/app/services/flood_risk_engine/notify/enrollment_submitted_email_service.rb +++ b/app/services/flood_risk_engine/notify/enrollment_submitted_email_service.rb @@ -10,7 +10,7 @@ def notify_options email_address: @recipient_address, template_id:, personalisation: { - registration_number: @enrollment.reference_number, + registration_number: @enrollment.ref_number, exemption_description: enrollment_description } } diff --git a/app/views/flood_risk_engine/enrollment_mailer/submitted.html.erb b/app/views/flood_risk_engine/enrollment_mailer/submitted.html.erb index 7adb482a..1a1d44fd 100644 --- a/app/views/flood_risk_engine/enrollment_mailer/submitted.html.erb +++ b/app/views/flood_risk_engine/enrollment_mailer/submitted.html.erb @@ -5,7 +5,7 @@ <% end %>

- <%= t('.reference_number_heading', reference_number: @enrollment.reference_number) %> + <%= t('.ref_number_heading', reference_number: @enrollment.ref_number) %>

<%= t('.exemption_heading') %>

diff --git a/app/views/flood_risk_engine/enrollment_mailer/submitted.text.erb b/app/views/flood_risk_engine/enrollment_mailer/submitted.text.erb index 50acd607..56ee5101 100644 --- a/app/views/flood_risk_engine/enrollment_mailer/submitted.text.erb +++ b/app/views/flood_risk_engine/enrollment_mailer/submitted.text.erb @@ -1,7 +1,7 @@ <%= t('.registration_submitted') %> <%= t('.please_wait') %> -------------------------------------------------------- <%= "\r\n" %> -<%= t('.text_reference_number', reference_number: @enrollment.reference_number) %> +<%= t('.text_reference_number', reference_number: @enrollment.ref_number) %> <%= "\r\n" %> <%= t(".exemption_heading") %> <%= @exemption.summary %> <%= @exemption.code %>. diff --git a/app/views/flood_risk_engine/registration_complete_forms/new.html.erb b/app/views/flood_risk_engine/registration_complete_forms/new.html.erb index 9d6e9fa3..ed8652d0 100644 --- a/app/views/flood_risk_engine/registration_complete_forms/new.html.erb +++ b/app/views/flood_risk_engine/registration_complete_forms/new.html.erb @@ -13,7 +13,7 @@
<%= t(".reference") %>
- <%= @enrollment.reference_number %> + <%= @enrollment.ref_number %>
diff --git a/spec/models/flood_risk_engine/enrollment_spec.rb b/spec/models/flood_risk_engine/enrollment_spec.rb index 8898dac6..34b0c03a 100644 --- a/spec/models/flood_risk_engine/enrollment_spec.rb +++ b/spec/models/flood_risk_engine/enrollment_spec.rb @@ -6,7 +6,6 @@ module FloodRiskEngine RSpec.describe Enrollment do let(:enrollment) { create(:enrollment) } - it { is_expected.to belong_to(:applicant_contact) } it { is_expected.to belong_to(:organisation) } it { is_expected.to have_one(:exemption_location).dependent(:restrict_with_exception) } @@ -16,7 +15,7 @@ module FloodRiskEngine end it "is of the right format" do - expect(enrollment.reference_number).to match(/EXFRA\d{6}/) + expect(enrollment.ref_number).to match(/EXFRA\d{6}/) end end diff --git a/spec/services/flood_risk_engine/notify/enrollment_submitted_email_service_spec.rb b/spec/services/flood_risk_engine/notify/enrollment_submitted_email_service_spec.rb index 812ade2a..32570a9b 100644 --- a/spec/services/flood_risk_engine/notify/enrollment_submitted_email_service_spec.rb +++ b/spec/services/flood_risk_engine/notify/enrollment_submitted_email_service_spec.rb @@ -23,7 +23,7 @@ module Notify email_address: recipient_address, template_id:, personalisation: { - registration_number: enrollment.reference_number, + registration_number: enrollment.ref_number, exemption_description: } } diff --git a/spec/services/flood_risk_engine/registration_completion_service_spec.rb b/spec/services/flood_risk_engine/registration_completion_service_spec.rb index ebe6bed3..35967401 100644 --- a/spec/services/flood_risk_engine/registration_completion_service_spec.rb +++ b/spec/services/flood_risk_engine/registration_completion_service_spec.rb @@ -31,7 +31,7 @@ module FloodRiskEngine expect(enrollment.submitted_at).to eq(stub_time) end - it "assigns the correct correspondance contact to the new enrollment" do + it "assigns the correct correspondence contact to the new enrollment" do correspondence_contact_attributes = { "full_name" => new_registration.contact_name, "email_address" => new_registration.contact_email, @@ -117,7 +117,7 @@ module FloodRiskEngine it "assigns the correct reference number" do run_service - expect(enrollment.reference_number).to eq(ReferenceNumber.last.number) + expect(enrollment.ref_number).to eq(ReferenceNumber.last.number) end it "assigns the status" do From 1f01f1ee0ecf9ffdd68e610713a60e7647bf8eed Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Mon, 12 Feb 2024 19:34:55 +0000 Subject: [PATCH 2/5] Instantiate dependent entities during registration creation to avoid ActiveRecord validation failures --- .../registration_completion_service.rb | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/app/services/flood_risk_engine/registration_completion_service.rb b/app/services/flood_risk_engine/registration_completion_service.rb index 10a3b7df..a54daac1 100644 --- a/app/services/flood_risk_engine/registration_completion_service.rb +++ b/app/services/flood_risk_engine/registration_completion_service.rb @@ -8,8 +8,6 @@ def run(transient_registration:) @registration = nil @transient_registration.with_lock do - @transient_registration.update(workflow_state: :creating_registration) - @registration = Enrollment.new transfer_data @registration.save! @@ -22,8 +20,8 @@ def run(transient_registration:) @registration rescue StandardError => e - Airbrake.notify(e, reference: @registration&.reference_number) if defined?(Airbrake) - Rails.logger.error "Completing registration error: #{e}" + Airbrake.notify(e, reference: @registration&.ref_number) if defined?(Airbrake) + Rails.logger.error "Completing registration error: #{e}\n#{@registration.errors.inspect}\n#{e.backtrace}" raise e end @@ -39,7 +37,7 @@ def transfer_data if @transient_registration.partnership? add_partnership_organisation else - add_organisation + add_organisation(@registration.correspondence_contact) end assign_exemption @@ -55,7 +53,7 @@ def add_core_data end def add_correspondence_contact - @registration.correspondence_contact = Contact.new( + @registration.correspondence_contact = Contact.create!( full_name: @transient_registration.contact_name, email_address: @transient_registration.contact_email, telephone_number: @transient_registration.contact_phone, @@ -64,13 +62,13 @@ def add_correspondence_contact end def add_secondary_contact - @registration.secondary_contact = Contact.new( + @registration.secondary_contact = Contact.create!( email_address: @transient_registration.additional_contact_email ) end def add_partnership_organisation - @registration.organisation = Organisation.new( + @registration.organisation = Organisation.create!( org_type: ) @@ -79,12 +77,12 @@ def add_partnership_organisation def add_partners @transient_registration.transient_people.each do |partner| - contact = Contact.new( + contact = Contact.create!( full_name: partner.full_name, address: build_partner_address(partner) ) - @registration.organisation.partners << Partner.new(contact:) + @registration.organisation.partners << Partner.create!(contact:) end end @@ -92,13 +90,14 @@ def build_partner_address(partner) attributes = transferable_address_attributes(partner.transient_address) attributes["organisation"] ||= "" - Address.new(attributes) + Address.create!(attributes) end - def add_organisation - @registration.organisation = Organisation.new( + def add_organisation(contact) + @registration.organisation = Organisation.create!( name: @transient_registration.company_name, - org_type: + org_type:, + contact: ) add_address @@ -108,7 +107,7 @@ def add_address attributes = transferable_address_attributes(@transient_registration.company_address) attributes["organisation"] ||= "" - @registration.organisation.primary_address = Address.new(attributes) + @registration.organisation.primary_address = Address.create!(attributes) end def assign_exemption @@ -118,7 +117,7 @@ def assign_exemption end def add_exemption_location - @registration.exemption_location = Location.new( + @registration.exemption_location = Location.create!( grid_reference: @transient_registration.temp_grid_reference, description: @transient_registration.temp_site_description, dredging_length: @transient_registration.dredging_length From 5c66ab8de98331de9fb42740142a2c8dfaf659f1 Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Mon, 12 Feb 2024 19:36:02 +0000 Subject: [PATCH 3/5] Make some belongs_to associations optional to allow persisting during registration creation --- app/models/flood_risk_engine/address.rb | 2 +- app/models/flood_risk_engine/contact.rb | 4 ++-- app/models/flood_risk_engine/location.rb | 4 ++-- app/models/flood_risk_engine/organisation.rb | 2 +- app/models/flood_risk_engine/partner.rb | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/flood_risk_engine/address.rb b/app/models/flood_risk_engine/address.rb index 9acdab23..0bca40b7 100644 --- a/app/models/flood_risk_engine/address.rb +++ b/app/models/flood_risk_engine/address.rb @@ -5,7 +5,7 @@ class Address < ApplicationRecord has_secure_token - belongs_to :addressable, polymorphic: true + belongs_to :addressable, polymorphic: true, optional: true has_one :location, as: :locatable, dependent: :restrict_with_exception after_create :clean_up_duplicate_addresses diff --git a/app/models/flood_risk_engine/contact.rb b/app/models/flood_risk_engine/contact.rb index e884bce5..659127d0 100644 --- a/app/models/flood_risk_engine/contact.rb +++ b/app/models/flood_risk_engine/contact.rb @@ -4,8 +4,8 @@ module FloodRiskEngine class Contact < ApplicationRecord - has_one :organisation, dependent: :restrict_with_exception - has_one :address, as: :addressable, dependent: :restrict_with_exception + has_one :organisation, dependent: :restrict_with_exception, required: false + has_one :address, as: :addressable, dependent: :restrict_with_exception, required: false enum contact_type: { individual: 0, diff --git a/app/models/flood_risk_engine/location.rb b/app/models/flood_risk_engine/location.rb index 154a8b19..861c80b0 100644 --- a/app/models/flood_risk_engine/location.rb +++ b/app/models/flood_risk_engine/location.rb @@ -4,8 +4,8 @@ module FloodRiskEngine class Location < ApplicationRecord - belongs_to :locatable, polymorphic: true - belongs_to :water_management_area + belongs_to :locatable, polymorphic: true, optional: true + belongs_to :water_management_area, optional: true before_save :process_grid_reference diff --git a/app/models/flood_risk_engine/organisation.rb b/app/models/flood_risk_engine/organisation.rb index 7415f3c7..812e31b9 100644 --- a/app/models/flood_risk_engine/organisation.rb +++ b/app/models/flood_risk_engine/organisation.rb @@ -2,7 +2,7 @@ module FloodRiskEngine class Organisation < ApplicationRecord - belongs_to :contact + belongs_to :contact, optional: true has_one :enrollment, dependent: :restrict_with_exception has_many :partners # Only needed for Partnerships has_one :primary_address, -> { where(address_type: Address.address_types["primary"]) }, diff --git a/app/models/flood_risk_engine/partner.rb b/app/models/flood_risk_engine/partner.rb index 0b2d4647..199b1c58 100644 --- a/app/models/flood_risk_engine/partner.rb +++ b/app/models/flood_risk_engine/partner.rb @@ -2,8 +2,8 @@ module FloodRiskEngine class Partner < ApplicationRecord - belongs_to :organisation - belongs_to :contact + belongs_to :organisation, optional: true + belongs_to :contact, optional: true delegate :full_name, :address, to: :contact From 2ecbcbb806c7abbc914fbdb2e46b75315ca0fa4e Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Mon, 12 Feb 2024 19:36:43 +0000 Subject: [PATCH 4/5] Fix translation labels for address finder errors --- Gemfile.lock | 13 +++++++------ .../company_address_manual_forms.en.yml | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 342024da..d6cfe519 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -166,7 +166,8 @@ GEM console (1.17.0) fiber-annotation fiber-local - crack (0.4.5) + crack (1.0.0) + bigdecimal rexml crass (1.0.6) date (3.3.4) @@ -241,7 +242,7 @@ GEM activemodel (>= 6.1) activesupport (>= 6.1) html-attributes-utils (~> 1) - hashdiff (1.0.1) + hashdiff (1.1.0) high_voltage (3.1.2) html-attributes-utils (1.0.2) activesupport (>= 6.1.4.4) @@ -280,7 +281,7 @@ GEM minitest (5.21.2) multi_json (1.15.0) mutex_m (0.2.0) - net-imap (0.4.9.1) + net-imap (0.4.10) date net-protocol net-pop (0.1.2) @@ -291,10 +292,10 @@ GEM net-protocol netrc (0.11.0) nio4r (2.7.0) - nokogiri (1.16.0) + nokogiri (1.16.2) mini_portile2 (~> 2.8.2) racc (~> 1.4) - nokogiri (1.16.0-x86_64-linux) + nokogiri (1.16.2-x86_64-linux) racc (~> 1.4) notifications-ruby-client (6.0.0) jwt (>= 1.5, < 3) @@ -481,7 +482,7 @@ GEM validates_email_format_of (1.7.2) i18n vcr (6.2.0) - webmock (3.19.1) + webmock (3.20.0) addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) diff --git a/config/locales/flood_risk_engine/company_address_manual_forms.en.yml b/config/locales/flood_risk_engine/company_address_manual_forms.en.yml index a626c72a..450b8052 100644 --- a/config/locales/flood_risk_engine/company_address_manual_forms.en.yml +++ b/config/locales/flood_risk_engine/company_address_manual_forms.en.yml @@ -10,8 +10,8 @@ en: soleTrader: What’s the address of the business? charity: What’s the address of the charity or trust? title: *heading - os_places_error_heading: Our address finder is not working - os_places_error_text: Please enter the address below. + address_finder_error_heading: Our address finder is not working + address_finder_error_text: Please enter the address below. preset_postcode_label: Postcode postcode_change_link: Change postcode error_heading: Something is wrong From 4b4cf5c2689b8b2c0da1c9414d71c95db00012d5 Mon Sep 17 00:00:00 2001 From: PaulDoyle-DEFRA <97455399+PaulDoyle-DEFRA@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:03:36 +0000 Subject: [PATCH 5/5] Revert optional: true on some associations --- app/models/flood_risk_engine/address.rb | 2 +- app/models/flood_risk_engine/contact.rb | 4 +-- app/models/flood_risk_engine/location.rb | 2 +- app/models/flood_risk_engine/organisation.rb | 2 +- app/models/flood_risk_engine/partner.rb | 4 +-- .../registration_completion_service.rb | 28 ++++++++++--------- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/app/models/flood_risk_engine/address.rb b/app/models/flood_risk_engine/address.rb index 0bca40b7..9acdab23 100644 --- a/app/models/flood_risk_engine/address.rb +++ b/app/models/flood_risk_engine/address.rb @@ -5,7 +5,7 @@ class Address < ApplicationRecord has_secure_token - belongs_to :addressable, polymorphic: true, optional: true + belongs_to :addressable, polymorphic: true has_one :location, as: :locatable, dependent: :restrict_with_exception after_create :clean_up_duplicate_addresses diff --git a/app/models/flood_risk_engine/contact.rb b/app/models/flood_risk_engine/contact.rb index 659127d0..e884bce5 100644 --- a/app/models/flood_risk_engine/contact.rb +++ b/app/models/flood_risk_engine/contact.rb @@ -4,8 +4,8 @@ module FloodRiskEngine class Contact < ApplicationRecord - has_one :organisation, dependent: :restrict_with_exception, required: false - has_one :address, as: :addressable, dependent: :restrict_with_exception, required: false + has_one :organisation, dependent: :restrict_with_exception + has_one :address, as: :addressable, dependent: :restrict_with_exception enum contact_type: { individual: 0, diff --git a/app/models/flood_risk_engine/location.rb b/app/models/flood_risk_engine/location.rb index 861c80b0..99ffec69 100644 --- a/app/models/flood_risk_engine/location.rb +++ b/app/models/flood_risk_engine/location.rb @@ -4,7 +4,7 @@ module FloodRiskEngine class Location < ApplicationRecord - belongs_to :locatable, polymorphic: true, optional: true + belongs_to :locatable, polymorphic: true belongs_to :water_management_area, optional: true before_save :process_grid_reference diff --git a/app/models/flood_risk_engine/organisation.rb b/app/models/flood_risk_engine/organisation.rb index 812e31b9..7415f3c7 100644 --- a/app/models/flood_risk_engine/organisation.rb +++ b/app/models/flood_risk_engine/organisation.rb @@ -2,7 +2,7 @@ module FloodRiskEngine class Organisation < ApplicationRecord - belongs_to :contact, optional: true + belongs_to :contact has_one :enrollment, dependent: :restrict_with_exception has_many :partners # Only needed for Partnerships has_one :primary_address, -> { where(address_type: Address.address_types["primary"]) }, diff --git a/app/models/flood_risk_engine/partner.rb b/app/models/flood_risk_engine/partner.rb index 199b1c58..0b2d4647 100644 --- a/app/models/flood_risk_engine/partner.rb +++ b/app/models/flood_risk_engine/partner.rb @@ -2,8 +2,8 @@ module FloodRiskEngine class Partner < ApplicationRecord - belongs_to :organisation, optional: true - belongs_to :contact, optional: true + belongs_to :organisation + belongs_to :contact delegate :full_name, :address, to: :contact diff --git a/app/services/flood_risk_engine/registration_completion_service.rb b/app/services/flood_risk_engine/registration_completion_service.rb index a54daac1..6c578862 100644 --- a/app/services/flood_risk_engine/registration_completion_service.rb +++ b/app/services/flood_risk_engine/registration_completion_service.rb @@ -21,7 +21,7 @@ def run(transient_registration:) @registration rescue StandardError => e Airbrake.notify(e, reference: @registration&.ref_number) if defined?(Airbrake) - Rails.logger.error "Completing registration error: #{e}\n#{@registration.errors.inspect}\n#{e.backtrace}" + Rails.logger.error "Completing registration error: #{e}\n#{@registration.errors.inspect}" raise e end @@ -37,7 +37,7 @@ def transfer_data if @transient_registration.partnership? add_partnership_organisation else - add_organisation(@registration.correspondence_contact) + add_organisation end assign_exemption @@ -53,7 +53,7 @@ def add_core_data end def add_correspondence_contact - @registration.correspondence_contact = Contact.create!( + @registration.correspondence_contact = Contact.new( full_name: @transient_registration.contact_name, email_address: @transient_registration.contact_email, telephone_number: @transient_registration.contact_phone, @@ -62,7 +62,7 @@ def add_correspondence_contact end def add_secondary_contact - @registration.secondary_contact = Contact.create!( + @registration.secondary_contact = Contact.new( email_address: @transient_registration.additional_contact_email ) end @@ -77,12 +77,12 @@ def add_partnership_organisation def add_partners @transient_registration.transient_people.each do |partner| - contact = Contact.create!( + contact = Contact.new( full_name: partner.full_name, address: build_partner_address(partner) ) - @registration.organisation.partners << Partner.create!(contact:) + @registration.organisation.partners << Partner.new(contact:) end end @@ -90,14 +90,13 @@ def build_partner_address(partner) attributes = transferable_address_attributes(partner.transient_address) attributes["organisation"] ||= "" - Address.create!(attributes) + Address.new(attributes) end - def add_organisation(contact) - @registration.organisation = Organisation.create!( + def add_organisation + @registration.organisation = Organisation.new( name: @transient_registration.company_name, - org_type:, - contact: + org_type: ) add_address @@ -107,7 +106,7 @@ def add_address attributes = transferable_address_attributes(@transient_registration.company_address) attributes["organisation"] ||= "" - @registration.organisation.primary_address = Address.create!(attributes) + @registration.organisation.primary_address = Address.new(attributes) end def assign_exemption @@ -117,7 +116,7 @@ def assign_exemption end def add_exemption_location - @registration.exemption_location = Location.create!( + @registration.exemption_location = Location.new( grid_reference: @transient_registration.temp_grid_reference, description: @transient_registration.temp_site_description, dredging_length: @transient_registration.dredging_length @@ -136,6 +135,9 @@ def update_status_and_assistance_mode end def update_water_management_area + # location.id must be present for ActiveJob serialization: + @registration.exemption_location.save! + UpdateWaterManagementAreaJob.perform_later(@registration.exemption_location) end