diff --git a/app/controllers/spree/admin/invoices_controller.rb b/app/controllers/spree/admin/invoices_controller.rb index b2035b9826..61ab99eb44 100644 --- a/app/controllers/spree/admin/invoices_controller.rb +++ b/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 diff --git a/app/jobs/bulk_invoice_job.rb b/app/jobs/bulk_invoice_job.rb index fba3dde99f..b7f0012018 100644 --- a/app/jobs/bulk_invoice_job.rb +++ b/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 diff --git a/app/mailers/spree/order_mailer.rb b/app/mailers/spree/order_mailer.rb index a2fda5d94f..9495566974 100644 --- a/app/mailers/spree/order_mailer.rb +++ b/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 diff --git a/app/models/invoice/data_presenter/address.rb b/app/models/invoice/data_presenter/address.rb index c7a08add40..29122dceba 100644 --- a/app/models/invoice/data_presenter/address.rb +++ b/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) diff --git a/app/models/invoice/data_presenter/distributor.rb b/app/models/invoice/data_presenter/distributor.rb index a0cc2299d3..5772ef31dd 100644 --- a/app/models/invoice/data_presenter/distributor.rb +++ b/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? diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 153143af66..707449af4a 100644 --- a/app/models/spree/order.rb +++ b/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 diff --git a/app/reflexes/admin/orders_reflex.rb b/app/reflexes/admin/orders_reflex.rb index 7e722a2705..311cab117f 100644 --- a/app/reflexes/admin/orders_reflex.rb +++ b/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 diff --git a/app/serializers/invoice/enterprise_serializer.rb b/app/serializers/invoice/enterprise_serializer.rb index 54b064fa7d..77611a75ef 100644 --- a/app/serializers/invoice/enterprise_serializer.rb +++ b/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 diff --git a/app/services/order_invoice_generator.rb b/app/services/order_invoice_generator.rb new file mode 100644 index 0000000000..4e02ccf487 --- /dev/null +++ b/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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 9a5333c7d3..1af585fb03 100644 --- a/config/locales/en.yml +++ b/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." diff --git a/spec/jobs/bulk_invoice_job_spec.rb b/spec/jobs/bulk_invoice_job_spec.rb index d783e88321..63c8bf34b0 100644 --- a/spec/jobs/bulk_invoice_job_spec.rb +++ b/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 diff --git a/spec/mailers/order_mailer_spec.rb b/spec/mailers/order_mailer_spec.rb index 9a2d3c4066..ef19ec4ee9 100644 --- a/spec/mailers/order_mailer_spec.rb +++ b/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 diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index e73de877a0..ca7bccfdc1 100644 --- a/spec/models/spree/order_spec.rb +++ b/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' } diff --git a/spec/services/order_invoice_generator_spec.rb b/spec/services/order_invoice_generator_spec.rb new file mode 100644 index 0000000000..852de2028a --- /dev/null +++ b/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