From 330e7d71af9d8ff3145af64b37f57309c73c7308 Mon Sep 17 00:00:00 2001 From: Duende13 Date: Thu, 20 Jul 2017 13:38:20 +0100 Subject: [PATCH 1/9] Sort line items by name and unit_value for confirmation email and summary screen --- app/models/spree/line_item_decorator.rb | 7 +++++++ .../spree/order_mailer/_order_summary.html.haml | 2 +- app/views/spree/orders/_summary.html.haml | 2 +- spec/models/spree/line_item_spec.rb | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 11c9d36850..d4d5b450a4 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -32,6 +32,13 @@ Spree::LineItem.class_eval do end } + scope :sorted_by_name_and_unit_value, + # Find line items that are from order sorted by variant name and unit value + joins(:variant=> :product). + reorder('spree_products.name asc, spree_variants.unit_value asc'). + select('spree_line_items.*') + + scope :supplied_by, lambda { |enterprise| joins(:product). where('spree_products.supplier_id = ?', enterprise) diff --git a/app/views/spree/order_mailer/_order_summary.html.haml b/app/views/spree/order_mailer/_order_summary.html.haml index 5d9dbc8e1f..4a13873d56 100644 --- a/app/views/spree/order_mailer/_order_summary.html.haml +++ b/app/views/spree/order_mailer/_order_summary.html.haml @@ -11,7 +11,7 @@ %h4 = t :email_order_summary_price %tbody - - @order.line_items.each do |item| + - @order.line_items.sorted_by_name_and_unit_value.each do |item| %tr %td = render 'spree/shared/line_item_name', line_item: item diff --git a/app/views/spree/orders/_summary.html.haml b/app/views/spree/orders/_summary.html.haml index b3276faede..02bc8d9fab 100644 --- a/app/views/spree/orders/_summary.html.haml +++ b/app/views/spree/orders/_summary.html.haml @@ -11,7 +11,7 @@ %th.text-right.total %span= t(:total) %tbody{"data-hook" => ""} - - order.line_items.each do |item| + - order.line_items.sorted_by_name_and_unit_value.each do |item| %tr.line_item{"data-hook" => "order_details_line_item_row", class: "variant-#{item.variant.id}" } %td(data-hook = "order_item_description") diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index cb1755f60c..0e48f2c24f 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -14,6 +14,18 @@ module Spree let(:li1) { create(:line_item, order: o, product: p1) } let(:li2) { create(:line_item, order: o, product: p2) } + + let(:p3) {create(:product, name: 'Clear Honey') } + let(:p4) {create(:product, name: 'Apricots') } + let(:v1) {create(:variant, product: p3, unit_value: 500) } + let(:v2) {create(:variant, product: p3, unit_value: 250) } + let(:v3) {create(:variant, product: p4, unit_value: 500) } + let(:v4) {create(:variant, product: p4, unit_value: 1000) } + let(:li3) { create(:line_item, order: o, product: p3, variant: v1) } + let(:li4) { create(:line_item, order: o, product: p3, variant: v2) } + let(:li5) { create(:line_item, order: o, product: p4, variant: v3) } + let(:li6) { create(:line_item, order: o, product: p4, variant: v4) } + it "finds line items for products supplied by a particular enterprise" do LineItem.supplied_by(s1).should == [li1] LineItem.supplied_by(s2).should == [li2] @@ -40,6 +52,10 @@ module Spree LineItem.without_tax.should == [li2] end end + + it "finds line items sorted by name and unit_value" do + o.line_items.sorted_by_name_and_unit_value.should == [li5,li6,li4,li3] + end end describe "capping quantity at stock level" do From 401052be68f6644f6ea39fcf05ef6dfa96f88d8e Mon Sep 17 00:00:00 2001 From: Duende13 Date: Thu, 20 Jul 2017 14:00:44 +0100 Subject: [PATCH 2/9] Fix Robcop issues (blank spaces and indentation) --- app/models/spree/line_item_decorator.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index d4d5b450a4..be5018d80d 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -32,12 +32,10 @@ Spree::LineItem.class_eval do end } - scope :sorted_by_name_and_unit_value, - # Find line items that are from order sorted by variant name and unit value - joins(:variant=> :product). - reorder('spree_products.name asc, spree_variants.unit_value asc'). - select('spree_line_items.*') - + # Find line items that are from order sorted by variant name and unit value + scope :sorted_by_name_and_unit_value, joins(:variant => :product). + reorder('spree_products.name asc, spree_variants.unit_value asc'). + select('spree_line_items.*') scope :supplied_by, lambda { |enterprise| joins(:product). From ba37db7ccce213721b468ae169d23fe1b20f052b Mon Sep 17 00:00:00 2001 From: Duende13 Date: Thu, 20 Jul 2017 14:43:56 +0100 Subject: [PATCH 3/9] Refactoring test to adopt most Rspec syntax (expect instead of should) --- spec/models/spree/line_item_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 0e48f2c24f..e1cbb5e612 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -54,7 +54,7 @@ module Spree end it "finds line items sorted by name and unit_value" do - o.line_items.sorted_by_name_and_unit_value.should == [li5,li6,li4,li3] + expect(o.line_items.sorted_by_name_and_unit_value).to eq([li5,li6,li4,li3]) end end From f5ad950bc34244ba853ce0f87fc8961520aca02c Mon Sep 17 00:00:00 2001 From: Duende13 Date: Sun, 23 Jul 2017 19:26:37 +0100 Subject: [PATCH 4/9] Change Syntax from :variant => :product to variant: :product --- app/models/spree/line_item_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index be5018d80d..8c87d410ad 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -33,7 +33,7 @@ Spree::LineItem.class_eval do } # Find line items that are from order sorted by variant name and unit value - scope :sorted_by_name_and_unit_value, joins(:variant => :product). + scope :sorted_by_name_and_unit_value, joins(variant: :product). reorder('spree_products.name asc, spree_variants.unit_value asc'). select('spree_line_items.*') From 40506685ef6927912864c60c6b07e898be02d361 Mon Sep 17 00:00:00 2001 From: stveep Date: Sat, 10 Feb 2018 15:51:41 +0000 Subject: [PATCH 5/9] Adding extra sort step for variant name; ignore case in sorting line items by name --- app/models/spree/line_item_decorator.rb | 2 +- spec/models/spree/line_item_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 8c87d410ad..66f9504648 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -34,7 +34,7 @@ Spree::LineItem.class_eval do # Find line items that are from order sorted by variant name and unit value scope :sorted_by_name_and_unit_value, joins(variant: :product). - reorder('spree_products.name asc, spree_variants.unit_value asc'). + reorder('lower(spree_products.name) asc, lower(spree_variants.display_name) asc, spree_variants.unit_value asc'). select('spree_line_items.*') scope :supplied_by, lambda { |enterprise| diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index e1cbb5e612..ccda41c98b 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -14,13 +14,13 @@ module Spree let(:li1) { create(:line_item, order: o, product: p1) } let(:li2) { create(:line_item, order: o, product: p2) } - + let(:p3) {create(:product, name: 'Clear Honey') } let(:p4) {create(:product, name: 'Apricots') } let(:v1) {create(:variant, product: p3, unit_value: 500) } let(:v2) {create(:variant, product: p3, unit_value: 250) } - let(:v3) {create(:variant, product: p4, unit_value: 500) } - let(:v4) {create(:variant, product: p4, unit_value: 1000) } + let(:v3) {create(:variant, product: p4, unit_value: 500, display_name: "ZZ") } + let(:v4) {create(:variant, product: p4, unit_value: 500, display_name: "aa") } let(:li3) { create(:line_item, order: o, product: p3, variant: v1) } let(:li4) { create(:line_item, order: o, product: p3, variant: v2) } let(:li5) { create(:line_item, order: o, product: p4, variant: v3) } @@ -54,7 +54,7 @@ module Spree end it "finds line items sorted by name and unit_value" do - expect(o.line_items.sorted_by_name_and_unit_value).to eq([li5,li6,li4,li3]) + expect(o.line_items.sorted_by_name_and_unit_value).to eq([li6,li5,li4,li3]) end end From dc76b3922b8acbf8b4bbcd754590b0b787cdd249 Mon Sep 17 00:00:00 2001 From: stveep Date: Sat, 10 Feb 2018 16:56:03 +0000 Subject: [PATCH 6/9] Add sorting to query for producer mailer --- app/mailers/producer_mailer.rb | 1 + spec/mailers/producer_mailer_spec.rb | 14 ++++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index 0bf05b787d..045ab9f31b 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -36,6 +36,7 @@ class ProducerMailer < Spree::BaseMailer where('order_cycles.id = ?', order_cycle). merge(Spree::Product.in_supplier(producer)). merge(Spree::Order.by_state('complete')) + end def total_from_line_items(line_items) diff --git a/spec/mailers/producer_mailer_spec.rb b/spec/mailers/producer_mailer_spec.rb index 553cd571e2..3441da1a6e 100644 --- a/spec/mailers/producer_mailer_spec.rb +++ b/spec/mailers/producer_mailer_spec.rb @@ -16,11 +16,11 @@ describe ProducerMailer do let(:s3) { create(:supplier_enterprise) } let(:d1) { create(:distributor_enterprise, charges_sales_tax: true) } let(:d2) { create(:distributor_enterprise) } - 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) } - let(:p5) { create(:product, price: 56.78, supplier: s1) } + let(:p1) { create(:product, name: "Zebra", price: 12.34, supplier: s1, tax_category: tax_category) } + let(:p2) { create(:product, name: "Aardvark", price: 23.45, supplier: s2) } + let(:p3) { create(:product, name: "Banana", price: 34.56, supplier: s1) } + let(:p4) { create(:product, name: "Coffee", price: 45.67, supplier: s1) } + let(:p5) { create(:product, name: "Daffodil", price: 56.78, supplier: s1) } let(:order_cycle) { create(:simple_order_cycle) } let!(:incoming_exchange) { order_cycle.exchanges.create! sender: s1, receiver: d1, incoming: true, receival_instructions: 'Outside shed.' } @@ -71,7 +71,8 @@ describe ProducerMailer do expect(mail.cc).to eq [order_cycle.coordinator.contact.email] end - it "contains an aggregated list of produce" do + it "contains an aggregated list of produce in alphabetical order" do + mail.body.encoded.should match(/Coffee.+\n.+Zebra/) body_lines_including(mail, p1.name).each do |line| line.should include 'QTY: 3' line.should include '@ $10.00 = $30.00' @@ -80,6 +81,7 @@ describe ProducerMailer do .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) From 9375318c4bf1cb6d7f5aa79922eb135d007e92dc Mon Sep 17 00:00:00 2001 From: stveep Date: Mon, 12 Feb 2018 21:46:31 +0000 Subject: [PATCH 7/9] Codeclimate: remove empty line --- app/mailers/producer_mailer.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index 045ab9f31b..0bf05b787d 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -36,7 +36,6 @@ class ProducerMailer < Spree::BaseMailer where('order_cycles.id = ?', order_cycle). merge(Spree::Product.in_supplier(producer)). merge(Spree::Order.by_state('complete')) - end def total_from_line_items(line_items) From 8a783bbb7dd885c0c0451278af357e981c63bb66 Mon Sep 17 00:00:00 2001 From: stveep Date: Sun, 4 Mar 2018 10:14:39 +0000 Subject: [PATCH 8/9] PR changes: extract SQL in methods to scopes, add case insensitivity to test for sorting --- app/mailers/producer_mailer.rb | 4 ++-- app/models/spree/line_item_decorator.rb | 8 ++++++-- spec/mailers/producer_mailer_spec.rb | 4 ++-- spec/models/spree/line_item_spec.rb | 6 ++++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/mailers/producer_mailer.rb b/app/mailers/producer_mailer.rb index 0bf05b787d..ad7569dad2 100644 --- a/app/mailers/producer_mailer.rb +++ b/app/mailers/producer_mailer.rb @@ -32,8 +32,8 @@ class ProducerMailer < Spree::BaseMailer def line_items_from(order_cycle, producer) Spree::LineItem. - joins(order: :order_cycle, variant: :product). - where('order_cycles.id = ?', order_cycle). + from_order_cycle(order_cycle). + sorted_by_name_and_unit_value. merge(Spree::Product.in_supplier(producer)). merge(Spree::Order.by_state('complete')) end diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 66f9504648..7ba0735a88 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -34,8 +34,12 @@ Spree::LineItem.class_eval do # Find line items that are from order sorted by variant name and unit value scope :sorted_by_name_and_unit_value, joins(variant: :product). - reorder('lower(spree_products.name) asc, lower(spree_variants.display_name) asc, spree_variants.unit_value asc'). - select('spree_line_items.*') + reorder('lower(spree_products.name) asc, lower(spree_variants.display_name) asc, spree_variants.unit_value asc') + + scope :from_order_cycle, lambda {|order_cycle| + joins(order: :order_cycle). + where('order_cycles.id = ?', order_cycle) + } scope :supplied_by, lambda { |enterprise| joins(:product). diff --git a/spec/mailers/producer_mailer_spec.rb b/spec/mailers/producer_mailer_spec.rb index 3441da1a6e..5fd1fc172d 100644 --- a/spec/mailers/producer_mailer_spec.rb +++ b/spec/mailers/producer_mailer_spec.rb @@ -19,7 +19,7 @@ describe ProducerMailer do let(:p1) { create(:product, name: "Zebra", price: 12.34, supplier: s1, tax_category: tax_category) } let(:p2) { create(:product, name: "Aardvark", price: 23.45, supplier: s2) } let(:p3) { create(:product, name: "Banana", price: 34.56, supplier: s1) } - let(:p4) { create(:product, name: "Coffee", price: 45.67, supplier: s1) } + let(:p4) { create(:product, name: "coffee", price: 45.67, supplier: s1) } let(:p5) { create(:product, name: "Daffodil", price: 56.78, supplier: s1) } let(:order_cycle) { create(:simple_order_cycle) } let!(:incoming_exchange) { order_cycle.exchanges.create! sender: s1, receiver: d1, incoming: true, receival_instructions: 'Outside shed.' } @@ -72,7 +72,7 @@ describe ProducerMailer do end it "contains an aggregated list of produce in alphabetical order" do - mail.body.encoded.should match(/Coffee.+\n.+Zebra/) + expect(mail.body.encoded).to match(/coffee.+\n.+Zebra/) body_lines_including(mail, p1.name).each do |line| line.should include 'QTY: 3' line.should include '@ $10.00 = $30.00' diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index ccda41c98b..7109322b1f 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -26,6 +26,8 @@ module Spree let(:li5) { create(:line_item, order: o, product: p4, variant: v3) } let(:li6) { create(:line_item, order: o, product: p4, variant: v4) } + let(:oc_order) { create :order_with_totals_and_distribution } + it "finds line items for products supplied by a particular enterprise" do LineItem.supplied_by(s1).should == [li1] LineItem.supplied_by(s2).should == [li2] @@ -56,6 +58,10 @@ module Spree it "finds line items sorted by name and unit_value" do expect(o.line_items.sorted_by_name_and_unit_value).to eq([li6,li5,li4,li3]) end + + it "finds line items from a given order cycle" do + expect(LineItem.from_order_cycle(oc_order.order_cycle).first.id).to eq oc_order.line_items.first.id + end end describe "capping quantity at stock level" do From 231dd7b6a26d5c20dbcbe6487249b0d3c7ce254b Mon Sep 17 00:00:00 2001 From: stveep Date: Sun, 4 Mar 2018 10:18:30 +0000 Subject: [PATCH 9/9] Add some rubocop spaces --- app/models/spree/line_item_decorator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index 7ba0735a88..6f74e19530 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -36,9 +36,9 @@ Spree::LineItem.class_eval do scope :sorted_by_name_and_unit_value, joins(variant: :product). reorder('lower(spree_products.name) asc, lower(spree_variants.display_name) asc, spree_variants.unit_value asc') - scope :from_order_cycle, lambda {|order_cycle| + scope :from_order_cycle, lambda { |order_cycle| joins(order: :order_cycle). - where('order_cycles.id = ?', order_cycle) + where('order_cycles.id = ?', order_cycle) } scope :supplied_by, lambda { |enterprise|