raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-02-26 01:33:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #12357 from mkllnk/fix-invoice-order

Browse Source 
Fix: preserve order of invoices in bulk print
This commit is contained in:
Konrad
2024-04-11 12:47:14 +02:00
committed by GitHub
parent dc7c5b6748 34fc6283b8
commit 27a4202fa4
6 changed files with 54 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
app/jobs/bulk_invoice_job.rb
View File

@@ -7,9 +7,10 @@ class BulkInvoiceJob < ApplicationJob
def perform(order_ids, filepath, options = {})
@options = options
orders = sorted_orders(order_ids)
orders.filter!(&:invoiceable?) if OpenFoodNetwork::FeatureToggle.enabled?(:invoices,
current_user)
# The `find` method returns records in the same order as the given ids.
orders = Spree::Order.find(order_ids)
orders.each(&method(:generate_invoice))
ensure_directory_exists filepath
@@ -21,12 +22,6 @@ class BulkInvoiceJob < ApplicationJob
private
# Ensures the records are returned in the same order the ids were originally given in
def sorted_orders(order_ids)
orders_by_id = Spree::Order.where(id: order_ids).to_a.index_by(&:id)
order_ids.map { |id| orders_by_id[id.to_i] }
end
def renderer
@renderer ||= InvoiceRenderer.new
end

5
app/models/spree/order.rb
View File

@@ -165,6 +165,7 @@ module Spree
scope :finalized, -> { where(state: FINALIZED_STATES) }
scope :complete, -> { where.not(completed_at: nil) }
scope :incomplete, -> { where(completed_at: nil) }
scope :invoiceable, -> { where(state: [:complete, :resumed]) }
scope :by_state, lambda { |state| where(state:) }
scope :not_state, lambda { |state| where.not(state:) }
@@ -213,10 +214,6 @@ 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

38
app/reflexes/admin/orders_reflex.rb
View File

@@ -32,11 +32,18 @@ module Admin
end
def bulk_invoice(params)
visible_orders = editable_orders.where(id: params[:bulk_ids]).filter(&:invoiceable?)
if Spree::Config.enterprise_number_required_on_invoices? &&
!all_distributors_can_invoice?(visible_orders)
render_business_number_required_error(visible_orders)
return
visible_orders = editable_orders.invoiceable.where(id: params[:bulk_ids])
if Spree::Config.enterprise_number_required_on_invoices?
distributors_without_abn = Enterprise.where(
id: visible_orders.select(:distributor_id),
abn: nil,
)
if distributors_without_abn.exists?
render_business_number_required_error(distributors_without_abn)
return
end
end
cable_ready.append(
@@ -44,8 +51,13 @@ module Admin
html: render(partial: "spree/admin/orders/bulk/invoice_modal")
).broadcast
# Preserve order of bulk_ids.
# The ids are supplied in the sequence of the orders screen and may be
# sorted, for example by last name of the customer.
visible_order_ids = params[:bulk_ids].map(&:to_i) & visible_orders.pluck(:id)
BulkInvoiceJob.perform_later(
visible_orders.pluck(:id),
visible_order_ids,
"tmp/invoices/#{Time.zone.now.to_i}-#{SecureRandom.hex(2)}.pdf",
channel: SessionChannel.for_request(request),
current_user_id: current_user.id
@@ -82,8 +94,8 @@ 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.invoiceable?
editable_orders.invoiceable.where(id: params[:bulk_ids]).find_each do |o|
next unless o.distributor.can_invoice?
Spree::OrderMailer.invoice_email(o.id, current_user_id: current_user.id).deliver_later
count += 1
@@ -114,14 +126,8 @@ module Admin
params[:id] = @order.number
end
def all_distributors_can_invoice?(orders)
distributor_ids = orders.map(&:distributor_id)
Enterprise.where(id: distributor_ids, abn: nil).empty?
end
def render_business_number_required_error(orders)
distributor_ids = orders.map(&:distributor_id)
distributor_names = Enterprise.where(id: distributor_ids, abn: nil).pluck(:name)
def render_business_number_required_error(distributors)
distributor_names = distributors.pluck(:name)
flash[:error] = I18n.t(:must_have_valid_business_number,
enterprise_name: distributor_names.join(", "))

35
spec/jobs/bulk_invoice_job_spec.rb
View File

@@ -5,45 +5,36 @@ require 'spec_helper'
describe BulkInvoiceJob do
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", feature: :invoices do
describe "#perform" do
let!(:order1) { create(:shipped_order) }
let!(:order2) { create(:order_with_line_items) }
let!(:order2) { create(:shipped_order) }
let!(:order3) { create(:order_ready_to_ship) }
let(:order_ids) { [order1.id, order2.id, order3.id] }
let(:order_ids) { [order3.id, order1.id, order2.id] }
let(:path){ "/tmp/file/path.pdf" }
before do
allow(TermsOfServiceFile).to receive(:current_url).and_return("http://example.com/terms.pdf")
order3.cancel
order3.resume
end
it "should generate invoices for invoiceable orders only" do
it "should generate invoices for given order ids" 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{ order2.invoices.count }.from(0).to(1)
.and change{ order3.invoices.count }.from(0).to(1)
File.open(path, "rb") do |io|
pages = 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)
reader.pages.map(&:text)
end
# Pages should be in the order of order ids given:
expect(pages[0]).to include(order3.number)
expect(pages[1]).to include(order1.number)
expect(pages[2]).to include(order2.number)
end
end
end

30
spec/models/spree/order_spec.rb
View File

@@ -140,23 +140,6 @@ 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 '#changes_allowed?' do
let(:order) { create(:order_ready_for_details) }
let(:complete) { true }
@@ -997,6 +980,19 @@ describe Spree::Order do
end
describe "scopes" do
describe "invoiceable" do
it "finds only active orders" do
order_complete = create(:order, state: :complete)
order_canceled = create(:order, state: :canceled)
order_resumed = create(:order, state: :resumed)
expect(Spree::Order.invoiceable).to match_array [
order_complete,
order_resumed,
]
end
end
describe "not_state" do
it "finds only orders not in specified state" do
o = FactoryBot.create(:completed_order_with_totals,

4
spec/system/admin/orders_spec.rb
View File

@@ -686,7 +686,7 @@ describe '
invoice_content = extract_pdf_content
expect(
invoice_content
invoice_content.join
).to match(/#{surnames[0]}.*#{surnames[1]}.*#{surnames[2]}.*#{surnames[3]}/m)
end
end
@@ -710,7 +710,6 @@ describe '
order4.name.gsub(/.* /, ""), order5.name.gsub(/.* /, "")].sort
}
before do
pending("#12340")
page.find('a', text: "NAME").click # orders alphabetically (asc)
sleep(0.5) # waits for column sorting
page.find('#selectAll').click
@@ -723,7 +722,6 @@ describe '
order4.name.gsub(/.* /, ""), order5.name.gsub(/.* /, "")].sort.reverse
}
before do
pending("#12340")
page.find('a', text: "NAME").click # orders alphabetically (asc)
sleep(0.5) # waits for column sorting
page.find('a', text: "NAME").click # orders alphabetically (desc)