From 5d3adc0bdb33c7deea5f81667cc7a6e16c3e5487 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 20 May 2016 11:01:33 +1000 Subject: [PATCH] Fixing producer emails so that they calculate tax correctly Also listing items by full_name rather than by variant, to catch cases where line item weights/volumes have been adjusted --- app/mailers/producer_mailer.rb | 31 ++++--------- .../order_cycle_report.html.haml | 18 ++++---- .../order_cycle_report.text.haml | 4 +- spec/mailers/producer_mailer_spec.rb | 43 ++++++++++++------- 4 files changed, 46 insertions(+), 50 deletions(-) diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index 590193c4a5..30fad48f58 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -4,10 +4,11 @@ class ProducerMailer < Spree::BaseMailer @producer = producer @coordinator = order_cycle.coordinator @order_cycle = order_cycle - @line_items = aggregated_line_items_from(@order_cycle, @producer) + line_items = line_items_from(@order_cycle, @producer) + @grouped_line_items = line_items.group_by(&:product_and_full_name) @receival_instructions = @order_cycle.receival_instructions_for @producer - @total = total_from_line_items(@line_items) - @tax_total = tax_total_from_line_items(@line_items) + @total = total_from_line_items(line_items) + @tax_total = tax_total_from_line_items(line_items) subject = "[#{Spree::Config.site_name}] Order cycle report for #{producer.name}" @@ -27,10 +28,6 @@ class ProducerMailer < Spree::BaseMailer line_items_from(order_cycle, producer).any? end - def aggregated_line_items_from(order_cycle, producer) - aggregate_line_items line_items_from(order_cycle, producer) - end - def line_items_from(order_cycle, producer) Spree::LineItem. joins(:order => :order_cycle, :variant => :product). @@ -39,23 +36,11 @@ class ProducerMailer < Spree::BaseMailer merge(Spree::Order.complete) end - def aggregate_line_items(line_items) - # Arrange the items in a hash to group quantities - line_items.inject({}) do |lis, li| - if lis.key? li.variant - lis[li.variant].quantity += li.quantity - else - lis[li.variant] = li - end - lis - end + def total_from_line_items(line_items) + Spree::Money.new line_items.sum(&:total) end - def total_from_line_items(aggregated_line_items) - Spree::Money.new aggregated_line_items.values.map(&:total).sum - end - - def tax_total_from_line_items(aggregated_line_items) - Spree::Money.new aggregated_line_items.values.map(&:included_tax).sum + def tax_total_from_line_items(line_items) + Spree::Money.new line_items.sum(&:included_tax) end end diff --git a/app/views/producer_mailer/order_cycle_report.html.haml b/app/views/producer_mailer/order_cycle_report.html.haml index d74d13685f..b69dde8e66 100644 --- a/app/views/producer_mailer/order_cycle_report.html.haml +++ b/app/views/producer_mailer/order_cycle_report.html.haml @@ -28,22 +28,22 @@ %th.text-right = t :included_tax %tbody - - @line_items.each_pair do |variant, line_item| + - @grouped_line_items.each_pair do |product_and_full_name, line_items| %tr %td - #{variant.sku} + #{line_items.first.variant.sku} %td - #{raw(variant.product.supplier.name)} + #{raw(line_items.first.product.supplier.name)} %td - #{raw(variant.product_and_full_name)} + #{raw(product_and_full_name)} %td.text-right - #{line_item.quantity} + #{line_items.sum(&:quantity)} %td.text-right - #{line_item.single_money} + #{line_items.first.single_money} %td.text-right - #{line_item.display_total} - %td.text-right - #{line_item.display_included_tax_amount} + #{Spree::Money.new(line_items.sum(&:total), currency: line_items.first.currency) } + %td.tax.text-right + #{Spree::Money.new(line_items.sum(&:included_tax), currency: line_items.first.currency) } %tr.total-row %td %td diff --git a/app/views/producer_mailer/order_cycle_report.text.haml b/app/views/producer_mailer/order_cycle_report.text.haml index 31e2cf54d9..b5b644bf01 100644 --- a/app/views/producer_mailer/order_cycle_report.text.haml +++ b/app/views/producer_mailer/order_cycle_report.text.haml @@ -12,8 +12,8 @@ Orders summary \ = t :producer_mail_order_text \ -- @line_items.each_pair do |variant, line_item| - #{variant.sku} - #{raw(variant.product.supplier.name)} - #{raw(variant.product_and_full_name)} (QTY: #{line_item.quantity}) @ #{line_item.single_money} = #{line_item.display_amount} +- @grouped_line_items.each_pair do |product_and_full_name, line_items| + #{line_items.first.variant.sku} - #{raw(line_items.first.product.supplier.name)} - #{raw(product_and_full_name)} (QTY: #{line_items.sum(&:quantity)}) @ #{line_items.first.single_money} = #{Spree::Money.new(line_items.sum(&:total), currency: line_items.first.currency)} \ \ Total: #{@total} diff --git a/spec/mailers/producer_mailer_spec.rb b/spec/mailers/producer_mailer_spec.rb index 01042b56f2..b878c27895 100644 --- a/spec/mailers/producer_mailer_spec.rb +++ b/spec/mailers/producer_mailer_spec.rb @@ -2,12 +2,15 @@ require 'spec_helper' require 'yaml' describe ProducerMailer do + let!(:zone) { create(:zone_with_member) } + let!(:tax_rate) { create(:tax_rate, included_in_price: true, calculator: Spree::Calculator::DefaultTax.new, zone: zone, amount: 0.1) } + let!(:tax_category) { create(:tax_category, tax_rates: [tax_rate]) } let(:s1) { create(:supplier_enterprise) } let(:s2) { create(:supplier_enterprise) } let(:s3) { create(:supplier_enterprise) } - let(:d1) { create(:distributor_enterprise) } + let(:d1) { create(:distributor_enterprise, charges_sales_tax: true) } let(:d2) { create(:distributor_enterprise) } - let(:p1) { create(:product, price: 12.34, supplier: s1) } + let(:p1) { create(:product, price: 12.34, supplier: s1, tax_category: tax_category) } let(:p2) { create(:product, price: 23.45, supplier: s2) } let(:p3) { create(:product, price: 34.56, supplier: s1) } let(:p4) { create(:product, price: 45.67, supplier: s1) } @@ -16,10 +19,10 @@ describe ProducerMailer do let!(:order) do order = create(:order, distributor: d1, order_cycle: order_cycle, state: 'complete') - order.line_items << create(:line_item, variant: p1.variants.first) - order.line_items << create(:line_item, variant: p1.variants.first) - order.line_items << create(:line_item, variant: p2.variants.first) - order.line_items << create(:line_item, variant: p4.variants.first) + order.line_items << create(:line_item, quantity: 1, variant: p1.variants.first) + order.line_items << create(:line_item, quantity: 2, variant: p1.variants.first) + order.line_items << create(:line_item, quantity: 3, variant: p2.variants.first) + order.line_items << create(:line_item, quantity: 2, variant: p4.variants.first) order.finalize! order.save order @@ -55,12 +58,17 @@ describe ProducerMailer do it "contains an aggregated list of produce" do body_lines_including(mail, p1.name).each do |line| - line.should include 'QTY: 2' - line.should include '@ $10.00 = $20.00' + line.should include 'QTY: 3' + line.should include '@ $10.00 = $30.00' end - Capybara.string(mail.html_part.body.encoded) - .find("table.order-summary tr", text: p1.name) - .should have_selector("td", text: "$20.00") + body_as_html(mail).find("table.order-summary tr", text: p1.name) + .should have_selector("td", text: "$30.00") + end + + it "displays tax totals for each product" do + # Tax for p1 line items + body_as_html(mail).find("table.order-summary tr", text: p1.name) + .should have_selector("td", text: "$30.00") end it "does not include incomplete orders" do @@ -68,11 +76,10 @@ describe ProducerMailer do end it "includes the total" do - puts mail.text_part.body.encoded - mail.body.encoded.should include 'Total: $30.00' - Capybara.string(mail.html_part.body.encoded) - .find("tr.total-row") - .should have_selector("td", text: "$30.00") + # puts mail.text_part.body.encoded + mail.body.encoded.should include 'Total: $50.00' + body_as_html(mail).find("tr.total-row") + .should have_selector("td", text: "$50.00") end it "sends no mail when the producer has no orders" do @@ -87,4 +94,8 @@ describe ProducerMailer do def body_lines_including(mail, s) mail.body.to_s.lines.select { |line| line.include? s } end + + def body_as_html(mail) + Capybara.string(mail.html_part.body.encoded) + end end