[RUBY-2477] work to fix issue with creating order logs and issue with

iteration when merging finance details

- Return order item log to being created from a transient registration
  rather than a registration
- Allow getting to registration from finance details of a transient
  registration
- reload finance details so orders are present
- Go back to using interim array rather than cloning for correcting
  issue with embedded document parent not being correct
- Add interim array for duplication for orders that was present for
  payments
- Add extra order to renewal_completion_service_spec to ensure it can
  handle multiple orders as well as multiple payments

app/models/waste_carriers_engine/finance_details.rb

@@ -45,5 +45,11 @@ def update_balance
       payment_balance = payments.except_online_not_authorised.sum { |item| item[:amount] }
       self.balance = order_balance - payment_balance
     end
+
+    def find_registration
+      return registration unless registration.nil?
+
+      WasteCarriersEngine::Registration.find_by(reg_identifier: transient_registration.reg_identifier)
+    end
   end
 end

app/models/waste_carriers_engine/order_item_log.rb

@@ -22,7 +22,7 @@ def active_registration?
     end
 
     def self.create_from_registration(registration, activation_time = nil)
-      registration.finance_details.orders.each do |order|
+      registration.finance_details.reload.orders.each do |order|
         order.order_items.each do |order_item|
           create_from_order_item(order_item, activation_time)
         end
@@ -31,7 +31,7 @@ def self.create_from_registration(registration, activation_time = nil)
 
     def self.create_from_order_item(order_item, activation_time = nil)
       order = order_item.order
-      registration = order.finance_details.registration
+      registration = order.finance_details.find_registration
       OrderItemLog.find_or_create_by(order_item_id: order_item.id) do |order_item_log|
         order_item_log.type            = order_item.type
         order_item_log.quantity        = order_item.quantity

app/services/concerns/waste_carriers_engine/can_merge_finance_details.rb

@@ -8,15 +8,29 @@ def merge_finance_details
       initialize_finance_details(registration)
       initialize_finance_details(transient_registration)
 
+      # To avoid issues which arose during the Rails 7 upgrade, where direct iteration over
+      # `transient_registration.finance_details.payments` and `transient_registration.finance_details.orders`
+      # didn't iterate over every payment or order respectively, we first collect all payments and orders
+      # into temporary arrays. This ensures that each payment and each order are iterated over without interference,
+      # as a payment can belong to only one payments collection at a time, and similarly for orders.
+      # This cannot be done using a clone as that changes the ids.
+
+      transient_orders_array = []
       transient_registration.finance_details.orders.each do |order|
+         transient_orders_array << order
+      end
+
+      transient_orders_array.each do |order|
         registration.finance_details.orders << order
       end
 
+      transient_payments_array = []
       transient_registration.finance_details.payments.each do |payment|
-        # To avoid issues which came up during the rails 7 upgrade with direct iteration
-        # over `transient_registration.finance_details.payments` we need to clone the
-        # payment as a payment can belong to only one payments collection at a time.
-          registration.finance_details.payments << payment.clone
+        transient_payments_array << payment
+      end
+
+      transient_payments_array.each do |payment|
+        registration.finance_details.payments << payment
       end
 
       registration.finance_details.update_balance

app/services/waste_carriers_engine/renewal_completion_service.rb

@@ -37,8 +37,8 @@ def complete_renewal
         transient_registration.with_lock do
           copy_names_to_contact_address
           create_past_registration
-          create_order_item_logs
           update_registration
+          create_order_item_logs
           delete_transient_registration
           send_confirmation_messages
         end
@@ -97,7 +97,7 @@ def extend_expiry_date
     end
 
     def create_order_item_logs
-      OrderItemLog.create_from_registration(registration)
+      OrderItemLog.create_from_registration(transient_registration)
     end
 
     def delete_transient_registration

spec/factories/finance_details.rb

@@ -50,6 +50,23 @@
       after(:build, :create, &:update_balance)
     end
 
+    trait :has_paid_orders_and_payments do
+      orders do
+        [
+          build(:order, :has_required_data),
+          build(:order, :has_copy_cards_item)
+        ]
+      end
+      payments do
+        [
+          build(:payment, :bank_transfer, amount: 10_500),
+          build(:payment, :bank_transfer, amount: 500),
+          build(:payment, :bank_transfer, amount: 500)
+        ]
+      end
+      after(:build, :create, &:update_balance)
+    end
+
     trait :has_outstanding_copy_card do
       orders { [build(:order, :has_required_data)] }
       payments { [build(:payment, :bank_transfer, amount: 10_500)] }

spec/factories/renewing_registration.rb

@@ -64,6 +64,10 @@
       finance_details { association(:finance_details, :has_paid_order_and_payment, strategy: :build) }
     end
 
+    trait :has_paid_order_with_two_orders do
+      finance_details { association(:finance_details, :has_paid_orders_and_payments, strategy: :build) }
+    end
+
     trait :has_conviction_search_result do
       conviction_search_result { association(:conviction_search_result, :match_result_no, strategy: :build) }
     end

spec/services/waste_carriers_engine/renewal_completion_service_spec.rb

@@ -10,7 +10,7 @@ module WasteCarriersEngine
         :has_required_data,
         :has_addresses,
         :has_key_people,
-        :has_paid_order,
+        :has_paid_order_with_two_orders,
         company_name: "FooBiz",
         workflow_state: "renewal_complete_form"
       )