[RUBY-3387] Fix duplicate storage of govpay status by refactoring `Sa…

…feCopyAttributesService`

- Implement `safe_copy_attributes` method to handle attribute copying more efficiently.
- Introduce private helper methods `get_attributes`, `filter_attributes`, and `find_embedded_relation` to modularize logic.
- Update specs to cover new functionality, including handling of Hash and BSON::Document sources.
- Ensure only valid attributes are copied, and embedded documents are processed correctly.

[RUBY-3387] Refactor `SafeCopyAttributesService` to streamline attribute copying

- Removed unused parameters `embedded_documents` and `attributes_to_exclude` from the `SafeCopyAttributesService`.
- Simplified the logic for copying attributes, focusing on filtering and processing embedded documents directly within the `copy_attributes` method.
- Updated related specs to reflect changes in the service, ensuring correct attributes are copied and non-existent attributes are excluded.

[RUBY-3387] Implement exclusion of attributes in SafeCopyAttributesService

- Updated `SafeCopyAttributesService` to allow exclusion of specified attributes during the copy process.
- Modified `can_copy_data_from_registration` to pass `attributes_to_exclude` option.
- Enhanced `filter_attributes` method to exclude specified attributes.
- Adjusted tests in `safe_copy_attributes_service_spec` to cover new exclusion functionality.
- Removed redundant embedded document copying logic from `past_registration` and `registration_completion_service`.

[RUBY-3387] Fix duplicate storage of govpay status in `SafeCopyAttributesService`

- Updated the service to correctly handle source relation data with both camelCase and snake_case keys.
- Adjusted the logic to prevent storing attributes not present on the model, specifically `govpayStatus`.
- Enhanced tests to verify that `govpayStatus` is not included in the result while valid attributes like `currency` are correctly processed.

[RUBY-3387] Rubocop changes

- Corrected the logic to prevent `govpayStatus` from being stored twice by refining the attribute matching process.
- Improved code readability by adding parentheses for variable assignments within conditional statements.
- Removed redundant code and fixed indentation issues.
- Updated tests to reflect changes in attribute presence checks, ensuring `govpayStatus` is not included if not present in the order model.

[RUBY-3387] Refactor attribute copying in `SafeCopyAttributesService`

- Simplified the `copy_attributes` method to improve readability and maintainability.
- Introduced `extract_attributes`, `filter_attributes`, and `process_embedded_relations` helper methods for better structure.
- Enhanced handling of embedded relations by adding `find_relation_data` to support different naming conventions.
- Improved error handling for unsupported source types.

[RUBY-3387] Fix duplicate storage of `govpayStatus` in SafeCopyAttributesService

- Updated `SafeCopyAttributesService` to exclude relations specified in `attributes_to_exclude` when copying attributes.
- Refactored method to find matching attribute keys based on naming conventions.
- Added tests to ensure excluded relations are not copied and that copyable attributes are correctly included.

[RUBY-3387] Fix attribute comparison in specs to ignore `_id` field

Updated tests in `past_registration_spec.rb` and `registration_completion_service_spec.rb` to compare attributes excluding the `_id` field as new implementation of SafeCopyAttributes does not copy the id field of the embedded documents like the old one did

Chore/ruby 3409 mongoid reference one (#1574)

https://eaflood.atlassian.net/browse/RUBY-3409

[RUBY-3387] Update spec to reference registered address as company address was removed

[RUBY-3387] Add documentation to `SafeCopyAttributesService` in `safe_copy_attributes_service.rb`

Enhanced the `SafeCopyAttributesService` with detailed comments explaining its functionality, including how it safely copies attributes between instances and handles embedded relations.

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

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
 

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)

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

app/forms/waste_carriers_engine/company_address_manual_form.rb

@@ -5,24 +5,25 @@ 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
       )
     end

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

app/models/concerns/waste_carriers_engine/can_copy_data_from_registration.rb

@@ -21,7 +21,6 @@ def copy_attributes_from_registration
       attributes = SafeCopyAttributesService.run(
         source_instance: registration,
         target_class: self.class,
-        embedded_documents: %w[metaData],
         attributes_to_exclude: options[:ignorable_attributes]
       )
       assign_attributes(strip_whitespace(attributes))

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,30 @@ 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" }
+      # Define helper accessor and assignment methods for each 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
+
+          # 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"

app/models/concerns/waste_carriers_engine/can_strip_whitespace.rb

@@ -34,7 +34,7 @@ def strip_hash(hash)
 
     def strip_array(array)
       array.each do |nested_object|
-        return nested_object if nested_object.is_a? BSON::Document
+        return nested_object if nested_object.is_a?(BSON::Document) || nested_object.is_a?(Hash)
 
         strip_whitespace(nested_object.attributes)
       end

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
 

app/models/waste_carriers_engine/past_registration.rb

@@ -22,6 +22,7 @@ class PastRegistration
       deregistration_token_created_at
       view_certificate_token
       view_certificate_token_created_at
+      conviction_sign_offs
     ].freeze
 
     def self.build_past_registration(registration, cause = nil)
@@ -35,7 +36,6 @@ def self.build_past_registration(registration, cause = nil)
       attributes = SafeCopyAttributesService.run(
         source_instance: registration,
         target_class: self,
-        embedded_documents: %w[addresses metaData financeDetails key_people conviction_search_result],
         attributes_to_exclude: NON_COPYABLE_ATTRIBUTES
       )
 

app/services/waste_carriers_engine/registration_completion_service.rb

@@ -148,7 +148,6 @@ def copyable_attributes(do_not_copy_attributes)
       SafeCopyAttributesService.run(
         source_instance: transient_registration,
         target_class: Registration,
-        embedded_documents: %w[addresses metaData financeDetails],
         attributes_to_exclude: do_not_copy_attributes
       )
     end

app/services/waste_carriers_engine/renewal_completion_service.rb

@@ -154,7 +154,6 @@ def copy_data_from_transient_registration
       renewal_attributes = SafeCopyAttributesService.run(
         source_instance: transient_registration,
         target_class: Registration,
-        embedded_documents: %w[addresses metaData financeDetails key_people conviction_search_result],
         attributes_to_exclude: do_not_copy_attributes
       )
 

app/services/waste_carriers_engine/safe_copy_attributes_service.rb

@@ -1,32 +1,94 @@
 # frozen_string_literal: true
 
 module WasteCarriersEngine
-  class SafeCopyAttributesService < BaseService
+  # This responsible for safely copying attributes from a source instance of
+  # one class to a new instance of the targeted class. It ensures that only attributes
+  # that are defined in the target class are copied, excluding any specified attributes.
+  # The service also handles embedded relations recursively, processing nested
+  # attributes according to the target class definitions.
 
-    attr_accessor :source_instance, :target_class, :embedded_documents, :attributes_to_exclude
+  class SafeCopyAttributesService
+    def self.run(source_instance:, target_class:, attributes_to_exclude: [])
+      new(source_instance, target_class, attributes_to_exclude).run
+    end
 
-    def run(source_instance:, target_class:, embedded_documents: [], attributes_to_exclude: [])
+    def initialize(source_instance, target_class, attributes_to_exclude = [])
       @source_instance = source_instance
       @target_class = target_class
-      @embedded_documents = embedded_documents
       @attributes_to_exclude = attributes_to_exclude
+    end
 
-      source_attributes.except(*unsupported_attribute_keys)
+    def run
+      copy_attributes(@source_instance, @target_class)
     end
 
-    def source_attributes
-      attributes = source_instance.is_a?(BSON::Document) ? source_instance : source_instance.attributes
+    private
+
+    # Recursively copies attributes from the source to match the target class
+    def copy_attributes(source, target_class)
+      attributes = extract_attributes(source)
+      valid_attributes = filter_attributes(attributes, target_class)
+      embedded_attributes = process_embedded_relations(attributes, target_class)
+      valid_attributes.merge(embedded_attributes)
+    end
 
-      attributes.except(*attributes_to_exclude)
+    # Extracts attributes from the source instance based on its type
+    def extract_attributes(source)
+      case source
+      when Hash, BSON::Document
+        source.to_h.stringify_keys
+      when ->(obj) { obj.respond_to?(:attributes) }
+        source.attributes
+      else
+        raise ArgumentError, "Unsupported source_instance type: #{source.class}"
+      end
     end
 
-    def target_fields
-      # Include both camelCase (DB) and snake_case (model) attribute names:
-      (target_class.fields.keys + target_class.fields.keys.map(&:underscore)).uniq
+    # Filters attributes to include only those defined in the target class, excluding specified attributes
+    def filter_attributes(attributes, target_class)
+      target_fields = target_class.fields.keys.map(&:to_s)
+      attributes.slice(*target_fields).except("_id", *@attributes_to_exclude)
+    end
+
+    # Processes embedded relations defined in the target class
+    def process_embedded_relations(attributes, target_class)
+      embedded_attributes = {}
+
+      target_class.embedded_relations.each do |relation_name, relation_metadata|
+        # Skip if the relation is in the attributes_to_exclude list
+        next if @attributes_to_exclude.map(&:underscore).include?(relation_name.underscore)
+
+        # Find the corresponding key in attributes (handles snake_case and camelCase)
+        key = matching_attribute_key(attributes, relation_name)
+        next unless key
+
+        source_data = attributes[key]
+        embedded_class = relation_metadata.class_name.constantize
+        embedded_attributes[key] = process_embedded_data(source_data, embedded_class)
+      end
+
+      embedded_attributes
+    end
+
+    # Finds the attribute key in attributes that corresponds to the relation name
+    def matching_attribute_key(attributes, relation_name)
+      snake_case_name = relation_name.underscore
+      camel_case_name = relation_name.camelize(:lower)
+
+      if attributes.key?(snake_case_name)
+        snake_case_name
+      elsif attributes.key?(camel_case_name)
+        camel_case_name
+      end
     end
 
-    def unsupported_attribute_keys
-      source_attributes.except(*target_fields).excluding(embedded_documents).keys
+    # Recursively processes embedded data
+    def process_embedded_data(data, embedded_class)
+      if data.is_a?(Array)
+        data.map { |item| copy_attributes(item, embedded_class) }
+      elsif data.is_a?(Hash) || data.is_a?(BSON::Document)
+        copy_attributes(data, embedded_class)
+      end
     end
   end
 end

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

@@ -22,8 +22,8 @@
             go_back_forms_path(@company_address_form.token))  %>
       </p>
 
-      <%= 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 %>
 
       <p class="govuk-body">

config/locales/forms/company_address_forms/en.yml

@@ -22,5 +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"

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"