From 7c5b430a3788bfa4ac229574b1e4c7d513bd7958 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 6 Feb 2019 11:26:22 +1100 Subject: [PATCH 1/7] Test execution of BulkInvoiceService#start_pdf_job The spec just tested if a job was enqueued, but not if the job can actually be executed. Unfortunately, this test is quite slow. --- spec/services/bulk_invoice_service_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/spec/services/bulk_invoice_service_spec.rb b/spec/services/bulk_invoice_service_spec.rb index 4611a46846..d310dacdfb 100644 --- a/spec/services/bulk_invoice_service_spec.rb +++ b/spec/services/bulk_invoice_service_spec.rb @@ -11,6 +11,17 @@ describe BulkInvoiceService do expect(Delayed::Job.last.payload_object.method_name).to eq :start_pdf_job_without_delay end + + it "creates a PDF invoice" do + order = create(:completed_order_with_fees) + order.bill_address = order.ship_address + order.save! + + service.start_pdf_job([order.id]) + Delayed::Job.last.invoke_job + + expect(service.invoice_created?(service.id)).to be_truthy + end end describe "#invoice_created?" do From d97fa60c31939ec96ec6dcaf30e4d3a00b65382b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 6 Feb 2019 14:07:13 +1100 Subject: [PATCH 2/7] Remove code duplication and test PDF creation --- .../admin/orders_controller_decorator.rb | 10 ++----- app/services/bulk_invoice_service.rb | 14 +-------- app/services/invoice_renderer.rb | 29 +++++++++++++++++++ spec/services/invoice_renderer_spec.rb | 15 ++++++++++ 4 files changed, 47 insertions(+), 21 deletions(-) create mode 100644 app/services/invoice_renderer.rb create mode 100644 spec/services/invoice_renderer_spec.rb diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 84e66451ba..1bac86035b 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -37,9 +37,7 @@ Spree::Admin::OrdersController.class_eval do end def invoice - pdf = render_to_string pdf: "invoice-#{@order.number}.pdf", - template: invoice_template, - formats: [:html], encoding: "UTF-8" + pdf = InvoiceRenderer.new.render(@order) Spree::OrderMailer.invoice_email(@order.id, pdf).deliver flash[:success] = t('admin.orders.invoice_email_sent') @@ -48,7 +46,7 @@ Spree::Admin::OrdersController.class_eval do end def print - render pdf: "invoice-#{@order.number}", template: invoice_template, encoding: "UTF-8" + render InvoiceRenderer.new.args(@order) end def print_ticket @@ -61,10 +59,6 @@ Spree::Admin::OrdersController.class_eval do private - def invoice_template - Spree::Config.invoice_style2? ? "spree/admin/orders/invoice2" : "spree/admin/orders/invoice" - end - def require_distributor_abn unless @order.distributor.abn.present? flash[:error] = t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) diff --git a/app/services/bulk_invoice_service.rb b/app/services/bulk_invoice_service.rb index 397020635e..4d2b9f1950 100644 --- a/app/services/bulk_invoice_service.rb +++ b/app/services/bulk_invoice_service.rb @@ -1,5 +1,4 @@ class BulkInvoiceService - include WickedPdf::PdfHelper attr_reader :id def initialize @@ -11,10 +10,7 @@ class BulkInvoiceService orders = Spree::Order.where(id: order_ids) orders.each do |order| - invoice = renderer.render_to_string pdf: "invoice-#{order.number}.pdf", - template: invoice_template, - formats: [:html], encoding: "UTF-8", - locals: { :@order => order } + invoice = InvoiceRenderer.new.render_to_string(order) pdf << CombinePDF.parse(invoice) end @@ -41,14 +37,6 @@ class BulkInvoiceService 'tmp/invoices' end - def renderer - ApplicationController.new - end - - def invoice_template - Spree::Config.invoice_style2? ? "spree/admin/orders/invoice2" : "spree/admin/orders/invoice" - end - def file_directory Dir.mkdir(directory) unless File.exist?(directory) directory diff --git a/app/services/invoice_renderer.rb b/app/services/invoice_renderer.rb new file mode 100644 index 0000000000..e3613dccbe --- /dev/null +++ b/app/services/invoice_renderer.rb @@ -0,0 +1,29 @@ +class InvoiceRenderer + def render_to_string(order) + renderer.render_to_string(args(order)) + end + + def args(order) + { + pdf: "invoice-#{order.number}.pdf", + template: invoice_template, + formats: [:html], + encoding: "UTF-8", + locals: { :@order => order } + } + end + + private + + def renderer + ApplicationController.new + end + + def invoice_template + if Spree::Config.invoice_style2? + "spree/admin/orders/invoice2" + else + "spree/admin/orders/invoice" + end + end +end diff --git a/spec/services/invoice_renderer_spec.rb b/spec/services/invoice_renderer_spec.rb new file mode 100644 index 0000000000..e136443a40 --- /dev/null +++ b/spec/services/invoice_renderer_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe InvoiceRenderer do + let(:service) { described_class.new } + + it "creates a PDF invoice" do + order = create(:completed_order_with_fees) + order.bill_address = order.ship_address + order.save! + + result = service.render_to_string(order) + + expect(result).to match /^%PDF/ + end +end From 3aea16e9ba194e422e3694ede7d2e0190b049a22 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 6 Feb 2019 15:06:44 +1100 Subject: [PATCH 3/7] Cover alternative invoice template with specs --- spec/services/invoice_renderer_spec.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/spec/services/invoice_renderer_spec.rb b/spec/services/invoice_renderer_spec.rb index e136443a40..7516b86492 100644 --- a/spec/services/invoice_renderer_spec.rb +++ b/spec/services/invoice_renderer_spec.rb @@ -3,13 +3,18 @@ require 'spec_helper' describe InvoiceRenderer do let(:service) { described_class.new } - it "creates a PDF invoice" do + it "creates a PDF invoice with two different templates" do order = create(:completed_order_with_fees) order.bill_address = order.ship_address order.save! result = service.render_to_string(order) - expect(result).to match /^%PDF/ + + allow(Spree::Config).to receive(:invoice_style2?).and_return true + + alternative = service.render_to_string(order) + expect(alternative).to match /^%PDF/ + expect(alternative).to_not eq result end end From 4a3e5f1d0a0a60efdc74dee66887103cf188b2d7 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 6 Feb 2019 15:07:05 +1100 Subject: [PATCH 4/7] Print supplier name on invoice --- app/views/spree/admin/orders/_invoice_table.html.haml | 3 +++ app/views/spree/admin/orders/_invoice_table2.html.haml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/app/views/spree/admin/orders/_invoice_table.html.haml b/app/views/spree/admin/orders/_invoice_table.html.haml index 9375f9bb68..c96099edb6 100644 --- a/app/views/spree/admin/orders/_invoice_table.html.haml +++ b/app/views/spree/admin/orders/_invoice_table.html.haml @@ -14,6 +14,9 @@ %tr %td = render 'spree/shared/line_item_name', line_item: item + %br + %small + %em= raw(item.variant.product.supplier.name) %td{:align => "right"} = item.quantity %td{:align => "right"} diff --git a/app/views/spree/admin/orders/_invoice_table2.html.haml b/app/views/spree/admin/orders/_invoice_table2.html.haml index 0941904d8b..2d42591ddc 100644 --- a/app/views/spree/admin/orders/_invoice_table2.html.haml +++ b/app/views/spree/admin/orders/_invoice_table2.html.haml @@ -17,6 +17,9 @@ %tr %td = render 'spree/shared/line_item_name', line_item: item + %br + %small + %em= raw(item.variant.product.supplier.name) %td{:align => "right"} = item.quantity %td{:align => "right"} From 2d0df7ffdfe11b7f83c85d6d7e83e7c5f438e829 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 7 Feb 2019 09:19:19 +1100 Subject: [PATCH 5/7] Update app/controllers/spree/admin/orders_controller_decorator.rb Co-Authored-By: mkllnk --- app/controllers/spree/admin/orders_controller_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 1bac86035b..20daae29c0 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -37,7 +37,7 @@ Spree::Admin::OrdersController.class_eval do end def invoice - pdf = InvoiceRenderer.new.render(@order) + pdf = InvoiceRenderer.new.render_to_string(@order) Spree::OrderMailer.invoice_email(@order.id, pdf).deliver flash[:success] = t('admin.orders.invoice_email_sent') From 5ee3dbf9f22920c5196343cb8d9b7365e3011f7e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 7 Feb 2019 10:12:06 +1100 Subject: [PATCH 6/7] Re-use renderer object --- app/services/bulk_invoice_service.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/services/bulk_invoice_service.rb b/app/services/bulk_invoice_service.rb index 4d2b9f1950..18921bd413 100644 --- a/app/services/bulk_invoice_service.rb +++ b/app/services/bulk_invoice_service.rb @@ -10,7 +10,7 @@ class BulkInvoiceService orders = Spree::Order.where(id: order_ids) orders.each do |order| - invoice = InvoiceRenderer.new.render_to_string(order) + invoice = renderer.render_to_string(order) pdf << CombinePDF.parse(invoice) end @@ -37,6 +37,10 @@ class BulkInvoiceService 'tmp/invoices' end + def renderer + @renderer ||= InvoiceRenderer.new + end + def file_directory Dir.mkdir(directory) unless File.exist?(directory) directory From 612ea4c7811657f99bc2e55db0c4d1011742ef5c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 13 Feb 2019 17:55:48 +1100 Subject: [PATCH 7/7] Simplify spec of invoice creation --- spec/services/bulk_invoice_service_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/services/bulk_invoice_service_spec.rb b/spec/services/bulk_invoice_service_spec.rb index d310dacdfb..4056a1b88f 100644 --- a/spec/services/bulk_invoice_service_spec.rb +++ b/spec/services/bulk_invoice_service_spec.rb @@ -17,8 +17,7 @@ describe BulkInvoiceService do order.bill_address = order.ship_address order.save! - service.start_pdf_job([order.id]) - Delayed::Job.last.invoke_job + service.start_pdf_job_without_delay([order.id]) expect(service.invoice_created?(service.id)).to be_truthy end