Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add columns to "pay your supplier" report #13037

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3330,6 +3330,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using
report_header_item_fees_price: "Item + Fees (%{currency})"
report_header_admin_handling_fees: "Admin & Handling (%{currency})"
report_header_ship_price: "Ship (%{currency})"
report_header_producer_charges_gst: Producer charges GST?
report_header_total_tax_on_product: "Total tax on product (%{currency})"
report_header_pay_fee_price: "Pay fee (%{currency})"
report_header_total_price: "Total (%{currency})"
report_header_product_total_price: "Product Total (%{currency})"
Expand Down
2 changes: 2 additions & 0 deletions lib/reporting/reports/suppliers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def columns
producer:,
producer_address:,
producer_abn_acn:,
producer_charges_gst:,
email:,
hub:,
hub_address:,
Expand All @@ -43,6 +44,7 @@ def columns
total_excl_fees_and_tax:,
total_excl_vat:,
total_fees_excl_tax:,
total_tax_on_product:,
total_tax_on_fees:,
total_tax:,
total:,
Expand Down
30 changes: 18 additions & 12 deletions lib/reporting/reports/suppliers/helpers/columns_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ def producer_abn_acn
end
end

def producer_charges_gst
proc do |line_items|
supplier(line_items).charges_sales_tax
end
end

def email
proc { |line_item| supplier(line_item).email_address }
end
Expand Down Expand Up @@ -80,8 +86,7 @@ def total_excl_fees_and_tax

def total_excl_vat
proc do |line_item|
total_fees = adjustments_by_type(line_item, :fees)
total_excl_fees_and_tax.call(line_item) + total_fees
total_excl_fees_and_tax.call(line_item) + total_fees_excl_tax.call(line_item)
end
end

Expand All @@ -92,27 +97,28 @@ def total_fees_excl_tax
end
end

def total_tax_on_product
proc do |line_item|
excluded_tax = adjustments_by_type(line_item, :tax)
included_tax = adjustments_by_type(line_item, :tax, included: true)

excluded_tax + included_tax
end
end

def total_tax_on_fees
proc { |line_item| tax_on_fees(line_item) + tax_on_fees(line_item, included: true) }
end

def total_tax
proc do |line_item|
excluded_tax = adjustments_by_type(line_item, :tax)
included_tax = adjustments_by_type(line_item, :tax, included: true)

excluded_tax + included_tax
total_tax_on_product.call(line_item) + total_tax_on_fees.call(line_item)
end
end

def total
proc do |line_item|
total_price = total_excl_fees_and_tax.call(line_item)
total_fees = total_fees_excl_tax.call(line_item)
total_fees_tax = total_tax_on_fees.call(line_item)
tax = total_tax.call(line_item)

total_price + total_fees + total_fees_tax + tax
total_excl_vat.call(line_item) + total_tax.call(line_item)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see some of the logic ends up simpler! 😎

end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,11 @@ def suppliers_adjustments(line_item, adjustment_type = 'EnterpriseFee')
end

def adjustments_by_type(line_item, type, included: false)
is_tax = type == :tax
return 0.0 if is_tax && !supplier(line_item).charges_sales_tax
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems strange that the report needs to check if charges_sales_tax. I thought that the application logic should be able to work this out. I see that Spree::TaxRate checks for this in match, but I don't think that helps here.

Oh well, it turns out it's quite common to refer to this directly in other reports, all good 👍


total_amount = 0.0
adjustment_type = type == :tax ? 'Spree::TaxRate' : 'EnterpriseFee'
adjustment_type = is_tax ? 'Spree::TaxRate' : 'EnterpriseFee'
suppliers_adjustments(line_item, adjustment_type).each do |adjustment|
amount = included == adjustment.included ? adjustment.amount : 0.0
total_amount += amount
Expand All @@ -49,6 +52,8 @@ def adjustments_by_type(line_item, type, included: false)
end

def tax_on_fees(line_item, included: false)
return 0.0 unless supplier(line_item).charges_sales_tax

total_amount = 0.0
suppliers_adjustments(line_item).each do |adjustment|
adjustment.adjustments.tax.each do |fee_adjustment|
Expand Down
6 changes: 5 additions & 1 deletion spec/lib/reports/suppliers/pay_your_suppliers_report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
expect(table_row.producer).to eq(supplier.name)
expect(table_row.producer_address).to eq(supplier.address.full_address)
expect(table_row.producer_abn_acn).to eq("none")
expect(table_row.producer_charges_gst).to eq("No")
expect(table_row.email).to eq("none")
expect(table_row.hub).to eq(hub.name)
expect(table_row.hub_address).to eq(hub.address.full_address)
Expand All @@ -42,6 +43,7 @@
expect(table_row.total_excl_vat.to_f).to eq(10.0)
expect(table_row.total_fees_excl_tax.to_f).to eq(0.0)
expect(table_row.total_tax_on_fees.to_f).to eq(0.0)
expect(table_row.total_tax_on_product.to_f).to eq(0.0)
expect(table_row.total_tax.to_f).to eq(0.0)
expect(table_row.total.to_f).to eq(10.0)
end
Expand Down Expand Up @@ -94,11 +96,13 @@
expect(report_table_rows.length).to eq(1)
table_row = report_table_rows.first

expect(table_row.producer_charges_gst).to eq('Yes')
expect(table_row.total_excl_fees_and_tax.to_f).to eq(10.0)
expect(table_row.total_excl_vat.to_f).to eq(10.1)
expect(table_row.total_fees_excl_tax.to_f).to eq(0.1)
expect(table_row.total_tax_on_fees.to_f).to eq(0.01)
expect(table_row.total_tax.to_f).to eq(1.0)
expect(table_row.total_tax_on_product.to_f).to eq(1.0)
expect(table_row.total_tax.to_f).to eq(1.01)
expect(table_row.total.to_f).to eq(11.11)
end
end
Expand Down
6 changes: 6 additions & 0 deletions spec/system/admin/reports/pay_your_suppliers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"Producer",
"Producer Address",
"Producer ABN/ACN",
"Producer charges GST?",
"Email",
"Hub",
"Hub Address",
Expand All @@ -63,6 +64,7 @@
"Total excl. fees and tax ($)",
"Total excl. tax ($)",
"Total fees excl. tax ($)",
"Total tax on product ($)",
"Total tax on fees ($)",
"Total Tax ($)",
"Total ($)"
Expand All @@ -83,6 +85,7 @@
supplier.name,
supplier.address.full_address,
"none",
"No",
"none",
hub1.name,
hub1.address.full_address,
Expand All @@ -100,6 +103,7 @@
0.0,
0.0,
0.0,
0.0,
10.0,
].compact.join(" "))
end
Expand All @@ -115,6 +119,7 @@
supplier.name,
supplier.address.full_address,
"none",
"No",
"none",
hub2.name,
hub2.address.full_address,
Expand All @@ -132,6 +137,7 @@
0.0,
0.0,
0.0,
0.0,
10.0,
].compact.join(" "))
end
Expand Down
Loading