diff --git a/app/models/concerns/waste_carriers_engine/can_copy_data_from_registration.rb b/app/models/concerns/waste_carriers_engine/can_copy_data_from_registration.rb index 5eb534375..ea236b472 100644 --- a/app/models/concerns/waste_carriers_engine/can_copy_data_from_registration.rb +++ b/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)) diff --git a/app/models/concerns/waste_carriers_engine/can_strip_whitespace.rb b/app/models/concerns/waste_carriers_engine/can_strip_whitespace.rb index addb1f5f4..5d323ae31 100644 --- a/app/models/concerns/waste_carriers_engine/can_strip_whitespace.rb +++ b/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 diff --git a/app/models/waste_carriers_engine/past_registration.rb b/app/models/waste_carriers_engine/past_registration.rb index c07216db6..b965f75ec 100644 --- a/app/models/waste_carriers_engine/past_registration.rb +++ b/app/models/waste_carriers_engine/past_registration.rb @@ -12,7 +12,6 @@ class PastRegistration embedded_in :registration, class_name: "WasteCarriersEngine::Registration" NON_COPYABLE_ATTRIBUTES = %w[ - _id accountEmail past_registrations locking_name @@ -22,6 +21,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 +35,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 ) diff --git a/app/models/waste_carriers_engine/renewing_registration.rb b/app/models/waste_carriers_engine/renewing_registration.rb index 66d01b532..0fc7be8c7 100644 --- a/app/models/waste_carriers_engine/renewing_registration.rb +++ b/app/models/waste_carriers_engine/renewing_registration.rb @@ -14,8 +14,7 @@ class RenewingRegistration < TransientRegistration field :from_magic_link, type: Boolean COPY_DATA_OPTIONS = { - ignorable_attributes: %w[_id - accountEmail + ignorable_attributes: %w[accountEmail constructionWaste contactEmail conviction_search_result diff --git a/app/services/waste_carriers_engine/registration_completion_service.rb b/app/services/waste_carriers_engine/registration_completion_service.rb index 039d7d415..abd17b372 100644 --- a/app/services/waste_carriers_engine/registration_completion_service.rb +++ b/app/services/waste_carriers_engine/registration_completion_service.rb @@ -127,7 +127,6 @@ def copy_data_from_transient_registration transient_registration.reload do_not_copy_attributes = %w[ - _id conviction_search_result conviction_sign_offs created_at @@ -148,7 +147,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 diff --git a/app/services/waste_carriers_engine/renewal_completion_service.rb b/app/services/waste_carriers_engine/renewal_completion_service.rb index 5bb6e2356..ae99171a9 100644 --- a/app/services/waste_carriers_engine/renewal_completion_service.rb +++ b/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 ) diff --git a/app/services/waste_carriers_engine/safe_copy_attributes_service.rb b/app/services/waste_carriers_engine/safe_copy_attributes_service.rb index bfa4cf1f2..c3905f99a 100644 --- a/app/services/waste_carriers_engine/safe_copy_attributes_service.rb +++ b/app/services/waste_carriers_engine/safe_copy_attributes_service.rb @@ -1,32 +1,95 @@ # frozen_string_literal: true module WasteCarriersEngine - class SafeCopyAttributesService < BaseService + # This responsible for safely copying attributes and embedded relations from a source instance of + # one class to a new instance of the targeted class. By default it will copy every attribute except _id + # and every embedded relation that are defined in the target class. However, both attributes and embedded + # relations can be excluded by passing them into the attributes_to_exclude argument + # Embedded relations are processed recursively, and attributes to exclude + # will be applied to the embedded relations as well. - 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 diff --git a/spec/models/waste_carriers_engine/past_registration_spec.rb b/spec/models/waste_carriers_engine/past_registration_spec.rb index 4b2e1b71a..5972c6b2b 100644 --- a/spec/models/waste_carriers_engine/past_registration_spec.rb +++ b/spec/models/waste_carriers_engine/past_registration_spec.rb @@ -24,7 +24,8 @@ module WasteCarriersEngine end it "copies nested objects from the registration" do - expect(past_registration.registered_address).to eq(registration.registered_address) + expect(past_registration.registered_address.attributes.except("_id")) + .to eq(registration.registered_address.attributes.except("_id")) end context "with non-copyable attribute values" do 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 4a6814d36..63b29dec9 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,11 @@ module WasteCarriersEngine expect { complete_registration }.to change(WasteCarriersEngine::Registration, :count).by(1) end - it { expect(complete_registration.registered_address).to eq(transient_registration.registered_address) } + it do + expect(complete_registration.registered_address.attributes.except("_id")) + .to eq(transient_registration.registered_address.attributes.except("_id")) + end + 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) } diff --git a/spec/services/waste_carriers_engine/safe_copy_attributes_service_spec.rb b/spec/services/waste_carriers_engine/safe_copy_attributes_service_spec.rb index ba1bbed34..1c164e148 100644 --- a/spec/services/waste_carriers_engine/safe_copy_attributes_service_spec.rb +++ b/spec/services/waste_carriers_engine/safe_copy_attributes_service_spec.rb @@ -5,26 +5,27 @@ module WasteCarriersEngine RSpec.describe SafeCopyAttributesService do describe "#run" do + subject(:run_service) do + described_class.run( + source_instance: source_instance, + target_class: target_class, + attributes_to_exclude: exclusion_list + ) + end + let(:exclusion_list) { [] } - subject(:run_service) { described_class.run(source_instance:, target_class:, embedded_documents:, attributes_to_exclude:) } - - let(:embedded_documents) { nil } - let(:exclusion_list) { nil } - let(:attributes_to_exclude) { exclusion_list } - - # ensure all available attributes are populated on the source + # Ensure all available attributes are populated on the source before do unless source_instance.is_a?(BSON::Document) source_instance.class.fields.keys.excluding("_id").each do |attr| next unless source_instance.send(attr).blank? && source_instance.respond_to?("#{attr}=") - source_instance.send "#{attr}=", 0 + source_instance.send("#{attr}=", 0) end end end shared_examples "returns the correct attributes" do - it { expect { run_service }.not_to raise_error } it "returns copyable attributes" do @@ -48,11 +49,9 @@ module WasteCarriersEngine context "when the target is a Registration" do let(:target_class) { Registration } - # include all embeds_many relationships - let(:embedded_documents) { %w[addresses financeDetails metaData] } let(:copyable_attributes) { %w[location contactEmail] } - let(:non_copyable_attributes) { %w[workflow_state temp_contact_postcode not_even_an_attribute] } - let(:exclusion_list) { %w[_id email_history] } + let(:non_copyable_attributes) { %w[workflow_state temp_contact_postcode not_even_an_attribute _id email_history] } + let(:exclusion_list) { %w[email_history] } context "when the source is a NewRegistration" do let(:source_instance) { build(:new_registration, :has_required_data) } @@ -73,7 +72,14 @@ module WasteCarriersEngine end context "when the source is a BSON::Document" do - let(:source_instance) { BSON::Document.new(build(:new_registration, :has_required_data).attributes) } + let(:source_instance) do + attributes = build(:new_registration, :has_required_data).attributes + attributes["non_existent_attribute"] = "some value" + BSON::Document.new(attributes) + end + + # Include the non-existent attribute in non_copyable_attributes + let(:non_copyable_attributes) { super() + ["non_existent_attribute"] } it_behaves_like "returns the correct attributes" end @@ -93,11 +99,85 @@ module WasteCarriersEngine let(:source_instance) { build(:key_person, :has_required_data) } let(:target_class) { KeyPerson } let(:copyable_attributes) { %w[first_name dob] } - let(:non_copyable_attributes) { %w[not_an_attribute neitherIsThis] } + let(:non_copyable_attributes) { %w[not_an_attribute neitherIsThis _id position] } let(:exclusion_list) { %w[_id position] } it_behaves_like "returns the correct attributes" end + + context "when the source is a BSON::Document with a nested attribute not present on the model" do + let(:target_class) { Registration } + let(:source_instance) do + attributes = { + "location" => "uk", + "contactEmail" => "test@example.com", + "non_existent_attribute" => "some value", + "financeDetails" => { + "orders" => [ + { + "govpayStatus" => "success", + "currency" => "GBP" + } + ] + + } + } + BSON::Document.new(attributes) + end + + let(:copyable_attributes) { %w[location contactEmail] } + let(:non_copyable_attributes) { %w[non_existent_attribute _id] } + let(:result) { run_service } + let(:order_attributes) { result["financeDetails"]["orders"].first } + + it "does not have the govpayStatus attribute as it is not present on the order model" do + expect(order_attributes.keys).not_to include("govpayStatus") + end + + it "has the currency attribute as it is present on the order model" do + expect(order_attributes.keys).to include("currency") + end + end + + context "when an embedded relation is included in attributes_to_exclude" do + let(:target_class) { Registration } + let(:source_instance) do + attributes = { + "location" => "uk", + "contactEmail" => "test@example.com", + "financeDetails" => { + "orders" => [ + { + "govpayStatus" => "success", + "currency" => "GBP" + } + ] + } + } + BSON::Document.new(attributes) + end + + let(:copyable_attributes) { %w[location contactEmail] } + let(:non_copyable_attributes) { %w[financeDetails _id] } + let(:exclusion_list) { %w[financeDetails] } + let(:result) { run_service } + + it "does not include the excluded embedded relation" do + expect(result.keys).not_to include("financeDetails") + end + + it "includes other copyable attributes" do + copyable_attributes.each do |attr| + expect(result[attr]).not_to be_nil + end + end + + it "does not include non-copyable attributes" do + non_copyable_attributes.each do |attr| + expect(result[attr]).to be_nil + end + end + end end end end