Merge pull request #11192 from abdellani/bulk-invoice-printing

fix Bulk invoice printing/sending

app/controllers/spree/admin/invoices_controller.rb

@@ -30,20 +30,7 @@ module Spree
       def generate
         @order = Order.find_by(number: params[:order_id])
         authorize! :invoice, @order
-
-        @comparator = OrderInvoiceComparator.new(@order)
-        if @comparator.can_generate_new_invoice?
-          @order.invoices.create!(
-            date: Time.zone.today,
-            number: @order.invoices.count + 1,
-            data: invoice_data
-          )
-        elsif @comparator.can_update_latest_invoice?
-          @order.invoices.last.update!(
-            date: Time.zone.today,
-            data: invoice_data
-          )
-        end
+        OrderInvoiceGenerator.new(@order).generate_or_update_latest_invoice
         redirect_back(fallback_location: spree.admin_dashboard_path)
       end
 
@@ -56,12 +43,6 @@ module Spree
           render json: { created: false }, status: :unprocessable_entity
         end
       end
-
-      protected
-
-      def invoice_data
-        @invoice_data ||= InvoiceDataGenerator.new(@order).generate
-      end
     end
   end
 end

app/jobs/bulk_invoice_job.rb

@@ -5,13 +5,9 @@ class BulkInvoiceJob < ApplicationJob
   delegate :render, to: ActionController::Base
 
   def perform(order_ids, filepath, options = {})
-    pdf = CombinePDF.new
-
-    sorted_orders(order_ids).each do |order|
-      invoice = renderer.render_to_string(order)
-
-      pdf << CombinePDF.parse(invoice)
-    end
+    orders = sorted_orders(order_ids)
+    orders.filter!(&:invoiceable?) if OpenFoodNetwork::FeatureToggle.enabled?(:invoices)
+    orders.each(&method(:generate_invoice))
 
     ensure_directory_exists filepath
 
@@ -32,6 +28,17 @@ class BulkInvoiceJob < ApplicationJob
     @renderer ||= InvoiceRenderer.new
   end
 
+  def generate_invoice(order)
+    renderer_data = if OpenFoodNetwork::FeatureToggle.enabled?(:invoices)
+                      OrderInvoiceGenerator.new(order).generate_or_update_latest_invoice
+                      order.invoices.first.presenter
+                    else
+                      order
+                    end
+    invoice = renderer.render_to_string(renderer_data)
+    pdf << CombinePDF.parse(invoice)
+  end
+
   def broadcast(filepath, channel)
     file_id = filepath.split("/").last.split(".").first
 
@@ -47,4 +54,8 @@ class BulkInvoiceJob < ApplicationJob
   def ensure_directory_exists(filepath)
     FileUtils.mkdir_p(File.dirname(filepath))
   end
+
+  def pdf
+    @pdf ||= CombinePDF.new
+  end
 end

app/mailers/spree/order_mailer.rb

@@ -48,7 +48,14 @@ module Spree
 
     def invoice_email(order_or_order_id)
       @order = find_order(order_or_order_id)
-      pdf = InvoiceRenderer.new.render_to_string(@order)
+      renderer_data = if OpenFoodNetwork::FeatureToggle.enabled?(:invoices)
+                        OrderInvoiceGenerator.new(@order).generate_or_update_latest_invoice
+                        @order.invoices.first.presenter
+                      else
+                        @order
+                      end
+
+      pdf = InvoiceRenderer.new.render_to_string(renderer_data)
 
       attach_file("invoice-#{@order.number}.pdf", pdf)
       I18n.with_locale valid_locale(@order.user) do

app/models/invoice/data_presenter/address.rb

@@ -24,6 +24,10 @@ class Invoice
         render_address([address1, address2, city, zipcode, state&.name])
       end
 
+      def blank?
+        @data.nil?
+      end
+
       private
 
       def render_address(address_parts)

app/models/invoice/data_presenter/distributor.rb

@@ -3,7 +3,8 @@
 class Invoice
   class DataPresenter
     class Distributor < Invoice::DataPresenter::Base
-      attributes :name, :abn, :acn, :logo_url, :display_invoice_logo, :invoice_text, :email_address
+      attributes :name, :abn, :acn, :logo_url, :display_invoice_logo, :invoice_text, :email_address,
+                 :phone
       attributes_with_presenter :contact, :address, :business_address
 
       def display_invoice_logo?

app/models/spree/order.rb

@@ -208,6 +208,10 @@ module Spree
       completed_at.present?
     end
 
+    def invoiceable?
+      complete? || resumed?
+    end
+
     # Indicates whether or not the user is allowed to proceed to checkout.
     # Currently this is implemented as a check for whether or not there is at
     # least one LineItem in the Order.  Feel free to override this logic in your

app/reflexes/admin/orders_reflex.rb

@@ -72,7 +72,7 @@ module Admin
     def send_invoices(params)
       count = 0
       editable_orders.where(id: params[:bulk_ids]).find_each do |o|
-        next unless o.distributor.can_invoice? && (o.resumed? || o.complete?)
+        next unless o.distributor.can_invoice? && o.invoiceable?
 
         Spree::OrderMailer.invoice_email(o.id).deliver_later
         count += 1

app/serializers/invoice/enterprise_serializer.rb

@@ -2,7 +2,8 @@
 
 class Invoice
   class EnterpriseSerializer < ActiveModel::Serializer
-    attributes :name, :abn, :acn, :invoice_text, :email_address, :display_invoice_logo, :logo_url
+    attributes :name, :abn, :acn, :invoice_text, :email_address, :display_invoice_logo, :logo_url,
+               :phone
     has_one :contact, serializer: Invoice::UserSerializer
     has_one :business_address, serializer: Invoice::AddressSerializer
     has_one :address, serializer: Invoice::AddressSerializer

app/services/order_invoice_generator.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class OrderInvoiceGenerator
+  def initialize(order)
+    @order = order
+  end
+
+  def generate_or_update_latest_invoice
+    if comparator.can_generate_new_invoice?
+      order.invoices.create!(
+        date: Time.zone.today,
+        number: order.invoices.count + 1,
+        data: invoice_data
+      )
+    elsif comparator.can_update_latest_invoice?
+      order.invoices.last.update!(
+        date: Time.zone.today,
+        data: invoice_data
+      )
+    end
+  end
+
+  private
+
+  attr_reader :order
+
+  def comparator
+    @comparator ||= OrderInvoiceComparator.new(order)
+  end
+
+  def invoice_data
+    @invoice_data ||= InvoiceDataGenerator.new(order).generate
+  end
+end

config/locales/en.yml

@@ -4108,6 +4108,11 @@ See the %{link} to find out more about %{sitename}'s features and to start using
           shipping: "Shipping"
           order_number: "Order Number"
           invoice_number: "Invoice Number"
+          payments_list:
+            date_time: "Date/time"
+            payment_method: "Payment method"
+            payment_state: "Payment state"
+            amount: "Amount"
         note:
           note_label: "Note:"
           no_note_present: "No note provided."

spec/jobs/bulk_invoice_job_spec.rb

@@ -3,19 +3,51 @@
 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
+    let(:order1) { build(:order, id: 1) }
+    let(:order2) { build(:order, id: 2) }
+    let(:order3) { build(:order, id: 3) }
+    let(:order_ids) { [3, 1, 2] }
+
     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
+
+  context "when invoices are enabled" do
+    before do
+      Flipper.enable(:invoices)
+    end
+
+    describe "#perform" do
+      let!(:order1) { create(:shipped_order) }
+      let!(:order2) { create(:order_with_line_items) }
+      let!(:order3) { create(:order_ready_to_ship) }
+      let(:order_ids) { [order1.id, order2.id, order3.id] }
+      let(:path){ "/tmp/file/path.pdf" }
+      before do
+        order3.cancel
+        order3.resume
+      end
+      it "should generate invoices for invoiceable orders only" do
+        expect{
+          subject.perform(order_ids, path)
+        }.to change{ order1.invoices.count }.from(0).to(1)
+          .and change{ order2.invoices.count }.by(0)
+          .and change{ order3.invoices.count }.from(0).to(1)
+
+        File.open(path, "rb") do |io|
+          reader = PDF::Reader.new(io)
+          content = reader.pages.map(&:text).join("\n")
+          expect(content).to include(order1.number)
+          expect(content).to include(order3.number)
+          expect(content).not_to include(order2.number)
+        end
+      end
+    end
+  end
 end

spec/mailers/order_mailer_spec.rb

@@ -216,4 +216,48 @@ describe Spree::OrderMailer do
       end
     end
   end
+
+  describe "#invoice_email" do
+    subject(:email) { described_class.invoice_email(order) }
+    let(:order) { create(:completed_order_with_totals) }
+    let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) }
+    let!(:invoice){
+      create(:invoice, order:,
+                       data: invoice_data_generator.serialize_for_invoice)
+    }
+
+    let(:generator){ instance_double(OrderInvoiceGenerator) }
+    let(:renderer){ instance_double(InvoiceRenderer) }
+    let(:attachment_filename){ "invoice-#{order.number}.pdf" }
+    let(:deliveries){ ActionMailer::Base.deliveries }
+    before do
+      allow(OrderInvoiceGenerator).to receive(:new).with(order).and_return(generator)
+      allow(InvoiceRenderer).to receive(:new).and_return(renderer)
+    end
+    context "When invoices feature is not enabled" do
+      it "should call the invoice render with order as argument" do
+        expect(generator).not_to receive(:generate_or_update_latest_invoice)
+        expect(order).not_to receive(:invoices)
+        expect(renderer).to receive(:render_to_string).with(order).and_return("invoice")
+        expect {
+          email.deliver_now
+        }.to_not raise_error
+        expect(deliveries.count).to eq(1)
+        expect(deliveries.first.attachments.count).to eq(1)
+        expect(deliveries.first.attachments.first.filename).to eq(attachment_filename)
+      end
+    end
+
+    context "When invoices feature is enabled" do
+      before do
+        Flipper.enable(:invoices)
+      end
+      it "should call the invoice renderer with invoice's presenter as argument" do
+        expect(generator).to receive(:generate_or_update_latest_invoice)
+        expect(order).to receive(:invoices).and_return([invoice])
+        expect(renderer).to receive(:render_to_string).with(invoice.presenter)
+        email.deliver_now
+      end
+    end
+  end
 end

spec/models/spree/order_spec.rb

@@ -140,6 +140,23 @@ describe Spree::Order do
     end
   end
 
+  context "#invoiceable?" do
+    it "should return true if the order is completed" do
+      allow(order).to receive_messages(complete?: true)
+      expect(order.invoiceable?).to be_truthy
+    end
+
+    it "should return true if the order is resumed" do
+      allow(order).to receive_messages(resumed?: true)
+      expect(order.invoiceable?).to be_truthy
+    end
+
+    it "should return false if the order is neither completed nor resumed" do
+      allow(order).to receive_messages(complete?: false, resumed?: false)
+      expect(order.invoiceable?).to be_falsy
+    end
+  end
+
   context "checking if order is paid" do
     context "payment_state is paid" do
       before { allow(order).to receive_messages payment_state: 'paid' }

spec/services/order_invoice_generator_spec.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe OrderInvoiceGenerator do
+  let!(:order) { create(:completed_order_with_fees) }
+  let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) }
+  let!(:latest_invoice){
+    create(:invoice,
+           order:,
+           data: invoice_data_generator.serialize_for_invoice)
+  }
+
+  let(:instance) { described_class.new(order) }
+  let(:comparator){ double("OrderInvoiceComparator") }
+
+  before do
+    allow(instance).to receive(:comparator).and_return(comparator)
+  end
+
+  describe "#generate_or_update_latest_invoice" do
+    let(:subject) { instance.generate_or_update_latest_invoice }
+    context "when can generate new invoice" do
+      before do
+        expect(comparator).to receive(:can_generate_new_invoice?).and_return(true)
+      end
+
+      it "should create a new invoice" do
+        expect(instance).to receive(:invoice_data)
+        expect{ subject }.to change{ order.invoices.count }.by(1)
+        expect(order.invoices.order('created_at desc').first.number).to eq(2)
+      end
+    end
+
+    context "can update latest invoice" do
+      before do
+        allow(comparator).to receive(:can_generate_new_invoice?).and_return(false)
+        allow(comparator).to receive(:can_update_latest_invoice?).and_return(true)
+        order.update!(note: "This is an updated note")
+      end
+
+      it "should update the latest invoice" do
+        expect{ subject }.to change{ latest_invoice.reload.data }
+          .and change{ order.invoices.count }.by(0)
+      end
+    end
+
+    context "when can't generate new invoice or update latest invoice" do
+      before do
+        allow(comparator).to receive(:can_generate_new_invoice?).and_return(false)
+        allow(comparator).to receive(:can_update_latest_invoice?).and_return(false)
+      end
+
+      it "should not create or update invoices" do
+        expect(instance).not_to receive(:invoice_data)
+        expect{ subject }.to change{ order.invoices.count }.by(0)
+      end
+    end
+  end
+end