From 45beefc5336fc9895c467dec7a8bfd1bae0cfdec Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 15 Dec 2021 20:29:12 +0000
Subject: [PATCH] Keep the order of passed argument

Do not retrieve the orders list sorted by completed_at but by the order_ids array itself

Add BulkInvoiceJob spec and cover sorting behaviour
---
 app/jobs/bulk_invoice_job.rb               |  7 +++---
 spec/jobs/bulk_invoice_job_spec.rb         | 21 +++++++++++++++++
 spec/services/bulk_invoice_service_spec.rb | 26 ----------------------
 3 files changed, 25 insertions(+), 29 deletions(-)
 create mode 100644 spec/jobs/bulk_invoice_job_spec.rb

diff --git a/app/jobs/bulk_invoice_job.rb b/app/jobs/bulk_invoice_job.rb
index 7335c552d6..3a1969c49a 100644
--- a/app/jobs/bulk_invoice_job.rb
+++ b/app/jobs/bulk_invoice_job.rb
@@ -4,7 +4,7 @@ class BulkInvoiceJob < ActiveJob::Base
   def perform(order_ids, filepath)
     pdf = CombinePDF.new
 
-    orders_from(order_ids).each do |order|
+    sorted_orders(order_ids).each do |order|
       invoice = renderer.render_to_string(order)
 
       pdf << CombinePDF.parse(invoice)
@@ -15,8 +15,9 @@ class BulkInvoiceJob < ActiveJob::Base
 
   private
 
-  def orders_from(order_ids)
-    Spree::Order.where(id: order_ids).order("completed_at DESC")
+  # Ensures the records are returned in the same order the ids were originally given in
+  def sorted_orders(order_ids)
+    Spree::Order.where(id: order_ids).to_a.sort_by{ |order| order_ids.index(order.id) }
   end
 
   def renderer
diff --git a/spec/jobs/bulk_invoice_job_spec.rb b/spec/jobs/bulk_invoice_job_spec.rb
new file mode 100644
index 0000000000..d783e88321
--- /dev/null
+++ b/spec/jobs/bulk_invoice_job_spec.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe BulkInvoiceJob do
+  let(:order1) { build(:order, id: 1) }
+  let(:order2) { build(:order, id: 2) }
+  let(:order3) { build(:order, id: 3) }
+
+  let(:order_ids) { [3, 1, 2] }
+
+  subject { BulkInvoiceJob.new(order_ids, "/tmp/file/path") }
+
+  describe "#sorted_orders" do
+    it "returns results in their original order" do
+      expect(Spree::Order).to receive(:where).and_return([order1, order2, order3])
+
+      expect(subject.__send__(:sorted_orders, order_ids)).to eq [order3, order1, order2]
+    end
+  end
+end
diff --git a/spec/services/bulk_invoice_service_spec.rb b/spec/services/bulk_invoice_service_spec.rb
index 62aaa977cb..c2142d15b6 100644
--- a/spec/services/bulk_invoice_service_spec.rb
+++ b/spec/services/bulk_invoice_service_spec.rb
@@ -53,30 +53,4 @@ describe BulkInvoiceService do
       expect(filepath).to eq 'tmp/invoices/1234567.pdf'
     end
   end
-
-  describe "#orders_from" do
-    let(:renderer) { InvoiceRenderer.new }
-
-    before do
-      allow(InvoiceRenderer).to receive(:new).and_return(renderer)
-    end
-
-    it "orders with completed desc" do
-      order_old = create(:order_with_distributor, :with_line_item, :completed,
-                         completed_at: 2.minutes.ago)
-      order_oldest = create(:order_with_distributor, :with_line_item, :completed,
-                            completed_at: 4.minutes.ago)
-      order_older = create(:order_with_distributor, :with_line_item, :completed,
-                           completed_at: 3.minutes.ago)
-
-      expect(renderer).to receive(:render_to_string).with(order_old).ordered.and_return("")
-      expect(renderer).to receive(:render_to_string).with(order_older).ordered.and_return("")
-      expect(renderer).to receive(:render_to_string).with(order_oldest).ordered.and_return("")
-
-      order_ids = [order_oldest, order_old, order_older].map(&:id)
-      perform_enqueued_jobs do
-        service.start_pdf_job(order_ids)
-      end
-    end
-  end
 end