From 4414a3f28755ca8cff89c44f13eb8212218cfdca Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Sat, 9 Apr 2016 10:05:45 +0100 Subject: [PATCH 01/24] Fix spelling mistake --- app/models/spree/user_decorator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 17473fd521..10a1c28972 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -55,7 +55,7 @@ Spree.user_class.class_eval do end # Returns orders and their associated payments for all distributors that have been ordered from - def compelete_orders_by_distributor + def complete_orders_by_distributor Enterprise .includes(distributed_orders: { payments: :payment_method }) .where(enterprises: { id: enterprises_ordered_from }, @@ -65,7 +65,7 @@ Spree.user_class.class_eval do def orders_by_distributor # Remove uncompleted payments as these will not be reflected in order balance - data_array = compelete_orders_by_distributor.to_a + data_array = complete_orders_by_distributor.to_a remove_uncompleted_payments(data_array) data_array.sort! { |a, b| b.distributed_orders.length <=> a.distributed_orders.length } end From bc048a943c776f910ff6cabe44df773d65b59e53 Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Sun, 10 Apr 2016 22:23:39 +0100 Subject: [PATCH 02/24] Show all payments, format unsuccessful payments grey, add 'invalid' translation. --- .../stylesheets/darkswarm/account.css.sass | 7 +++++ app/models/spree/user_decorator.rb | 6 ++-- app/serializers/api/payment_serializer.rb | 2 +- app/views/spree/users/_fat.html.haml | 6 ++-- config/locales/en-GB.yml | 31 +++++++++++++++++++ config/locales/en.yml | 1 + 6 files changed, 46 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/darkswarm/account.css.sass b/app/assets/stylesheets/darkswarm/account.css.sass index f582545b74..0f3de5af57 100644 --- a/app/assets/stylesheets/darkswarm/account.css.sass +++ b/app/assets/stylesheets/darkswarm/account.css.sass @@ -32,6 +32,13 @@ .debit color: $clr-brick + .invalid + color: $ofn-grey + .credit + color: $ofn-grey + .debit + color: $ofn-grey + .distributor-balance.paid visibility: hidden diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 10a1c28972..274330d1da 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -66,7 +66,7 @@ Spree.user_class.class_eval do def orders_by_distributor # Remove uncompleted payments as these will not be reflected in order balance data_array = complete_orders_by_distributor.to_a - remove_uncompleted_payments(data_array) + remove_payments_in_checkout(data_array) data_array.sort! { |a, b| b.distributed_orders.length <=> a.distributed_orders.length } end @@ -78,10 +78,10 @@ Spree.user_class.class_eval do end end - def remove_uncompleted_payments(enterprises) + def remove_payments_in_checkout(enterprises) enterprises.each do |enterprise| enterprise.distributed_orders.each do |order| - order.payments.keep_if { |payment| payment.state == "completed" } + order.payments.keep_if { |payment| payment.state != "checkout" } end end end diff --git a/app/serializers/api/payment_serializer.rb b/app/serializers/api/payment_serializer.rb index d48f75a07f..8465998db6 100644 --- a/app/serializers/api/payment_serializer.rb +++ b/app/serializers/api/payment_serializer.rb @@ -1,6 +1,6 @@ module Api class PaymentSerializer < ActiveModel::Serializer - attributes :amount, :updated_at, :payment_method + attributes :amount, :updated_at, :payment_method, :state def payment_method object.payment_method.name end diff --git a/app/views/spree/users/_fat.html.haml b/app/views/spree/users/_fat.html.haml index c10cf8254a..9f9ad67b10 100644 --- a/app/views/spree/users/_fat.html.haml +++ b/app/views/spree/users/_fat.html.haml @@ -19,10 +19,10 @@ %td.order5.text-right{"ng-class" => "{'credit' : order.total < 0, 'debit' : order.total > 0, 'paid' : order.total == 0}","bo-text" => "order.total | localizeCurrency"} %td.order6.text-right.show-for-large-up{"ng-class" => "{'credit' : order.outstanding_balance < 0, 'debit' : order.outstanding_balance > 0, 'paid' : order.outstanding_balance == 0}", "bo-text" => "order.outstanding_balance | localizeCurrency"} %td.order7.text-right{"ng-class" => "{'credit' : order.running_balance < 0, 'debit' : order.running_balance > 0, 'paid' : order.running_balance == 0}", "bo-text" => "order.running_balance | localizeCurrency"} - %tr.payment-row{"ng-repeat" => "payment in order.payments"} - %td.order1= t :payment + %tr.payment-row{"ng-repeat" => "payment in order.payments", "ng-class" => "{'invalid': payment.state != 'completed'}"} + %td.order1{"bo-text" => "payment.payment_method"} %td.order2{"bo-text" => "payment.updated_at"} - %td.order3.show-for-large-up{"bo-text" => "payment.payment_method"} + %td.order3.show-for-large-up{"bo-text" => "'spree.payment_states.' + payment.state | t | capitalize"} %td.order4.show-for-large-up %td.order5.text-right{"ng-class" => "{'credit' : payment.amount > 0, 'debit' : payment.amount < 0, 'paid' : payment.amount == 0}","bo-text" => "payment.amount | localizeCurrency"} %td.order6.show-for-large-up diff --git a/config/locales/en-GB.yml b/config/locales/en-GB.yml index 7301ef4f2c..3e793f88a4 100644 --- a/config/locales/en-GB.yml +++ b/config/locales/en-GB.yml @@ -635,3 +635,34 @@ Please follow the instructions there to make your enterprise visible on the Open price_graph: "Price graph" included_tax: "Included tax" remove_tax: "Remove tax" + spree: + shipment_states: + backorder: backorder + partial: partial + pending: pending + ready: ready + shipped: shipped + payment_states: + balance_due: balance due + completed: completed + checkout: checkout + credit_owed: credit owed + failed: failed + paid: paid + pending: pending + processing: processing + void: void + invalid: invalid + order_state: + address: address + adjustments: adjustments + awaiting_return: awaiting return + canceled: canceled + cart: cart + complete: complete + confirm: confirm + delivery: delivery + payment: payment + resumed: resumed + returned: returned + skrill: skrill diff --git a/config/locales/en.yml b/config/locales/en.yml index 5bf35d1aa5..020011ea8d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -996,6 +996,7 @@ Please follow the instructions there to make your enterprise visible on the Open pending: pending processing: processing void: void + invalid: invalid order_state: address: address adjustments: adjustments From 14830237692bb005c409a83fde4df9caace58a0d Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Sun, 10 Apr 2016 22:48:38 +0100 Subject: [PATCH 03/24] Add a sweet warning sign --- app/views/spree/users/_fat.html.haml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/spree/users/_fat.html.haml b/app/views/spree/users/_fat.html.haml index 9f9ad67b10..df5bd71c5d 100644 --- a/app/views/spree/users/_fat.html.haml +++ b/app/views/spree/users/_fat.html.haml @@ -22,7 +22,9 @@ %tr.payment-row{"ng-repeat" => "payment in order.payments", "ng-class" => "{'invalid': payment.state != 'completed'}"} %td.order1{"bo-text" => "payment.payment_method"} %td.order2{"bo-text" => "payment.updated_at"} - %td.order3.show-for-large-up{"bo-text" => "'spree.payment_states.' + payment.state | t | capitalize"} + %td.order3.show-for-large-up + %i{"ng-class" => "{'ofn-i_012-warning': payment.state == 'invalid' || payment.state == 'void'}"} + %span{"bo-text" => "'spree.payment_states.' + payment.state | t | capitalize"} %td.order4.show-for-large-up %td.order5.text-right{"ng-class" => "{'credit' : payment.amount > 0, 'debit' : payment.amount < 0, 'paid' : payment.amount == 0}","bo-text" => "payment.amount | localizeCurrency"} %td.order6.show-for-large-up From fc719230a3ea29a4b9fd16c5bb4a075c323aa0e0 Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Fri, 15 Apr 2016 08:26:12 +0100 Subject: [PATCH 04/24] Add failed payments, update spec --- app/views/spree/users/_fat.html.haml | 2 +- spec/models/spree/user_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/spree/users/_fat.html.haml b/app/views/spree/users/_fat.html.haml index df5bd71c5d..5c87077b4c 100644 --- a/app/views/spree/users/_fat.html.haml +++ b/app/views/spree/users/_fat.html.haml @@ -23,7 +23,7 @@ %td.order1{"bo-text" => "payment.payment_method"} %td.order2{"bo-text" => "payment.updated_at"} %td.order3.show-for-large-up - %i{"ng-class" => "{'ofn-i_012-warning': payment.state == 'invalid' || payment.state == 'void'}"} + %i{"ng-class" => "{'ofn-i_012-warning': payment.state == 'invalid' || payment.state == 'void' || payment.state == 'failed'}"} %span{"bo-text" => "'spree.payment_states.' + payment.state | t | capitalize"} %td.order4.show-for-large-up %td.order5.text-right{"ng-class" => "{'credit' : payment.amount > 0, 'debit' : payment.amount < 0, 'paid' : payment.amount == 0}","bo-text" => "payment.amount | localizeCurrency"} diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index ee01326316..9911c16a51 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -92,7 +92,7 @@ describe Spree.user_class do let!(:d2o1) { create(:completed_order_with_totals, distributor: distributor2, user_id: u2.id)} let!(:completed_payment) { create(:payment, order: d1o1, state: 'completed')} - let!(:payment) { create(:payment, order: d1o2, state: 'invalid')} + let!(:payment) { create(:payment, order: d1o2, state: 'checkout')} it "returns enterprises that the user has ordered from" do expect(u1.enterprises_ordered_from).to eq [distributor1.id] @@ -114,7 +114,7 @@ describe Spree.user_class do expect(u1.orders_by_distributor.first.distributed_orders).not_to include d1o3 end - it "doesn't return uncompleted payments" do + it "doesn't return payments that are still at checkout stage" do expect(u1.orders_by_distributor.first.distributed_orders.map{|o| o.payments}.flatten).not_to include payment end end From 7f38f1dd1c63f7b4b804fa05a519fcfb3bfa61e9 Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Tue, 26 Apr 2016 03:14:13 +0100 Subject: [PATCH 05/24] Exclude Accounts & Billing distributor --- app/models/spree/user_decorator.rb | 5 ++++- spec/features/consumer/account_spec.rb | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 9a992ba910..7e3a38afb2 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -56,7 +56,10 @@ Spree.user_class.class_eval do # Returns Enterprise IDs for distributors that the user has shopped at def enterprises_ordered_from - orders.where(state: :complete).map(&:distributor_id).uniq + orders.where(state: :complete) + .where('distributor_id != ?', Spree::Config.accounts_distributor_id) + .map(&:distributor_id) + .uniq end # Returns orders and their associated payments for all distributors that have been ordered from diff --git a/spec/features/consumer/account_spec.rb b/spec/features/consumer/account_spec.rb index b072a24357..1203d459de 100644 --- a/spec/features/consumer/account_spec.rb +++ b/spec/features/consumer/account_spec.rb @@ -13,6 +13,8 @@ feature %q{ let!(:distributor2) { create(:distributor_enterprise) } let!(:distributor_credit) { create(:distributor_enterprise) } let!(:distributor_without_orders) { create(:distributor_enterprise) } + let!(:accounts_distributor) {create :distributor_enterprise} + let!(:order_account_invoice) { create(:order, distributor: accounts_distributor, user: user) } let!(:d1o1) { create(:completed_order_with_totals, distributor_id: distributor1.id, user_id: user.id, total: 10000)} let!(:d1o2) { create(:order_without_full_payment, distributor_id: distributor1.id, user_id: user.id, total: 5000)} let!(:d2o1) { create(:completed_order_with_totals, distributor_id: distributor2.id, user_id: user.id)} @@ -21,19 +23,24 @@ feature %q{ before do + Spree::Config.accounts_distributor_id = accounts_distributor.id credit_order.update! login_as user visit "/account" end it "shows all hubs that have been ordered from with balance or credit" do + # Single test to avoid re-rendering page expect(page).to have_content distributor1.name expect(page).to have_content distributor2.name expect(page).not_to have_content distributor_without_orders.name + # Exclude the special Accounts & Billing distributor + expect(page).not_to have_content accounts_distributor.name expect(page).to have_content distributor1.name + " " + "Balance due" expect(page).to have_content distributor_credit.name + " Credit" end + it "reveals table of orders for distributors when clicked" do expand_active_table_node distributor1.name expect(page).to have_link "Order " + d1o1.number, href:"/orders/#{d1o1.number}" From 43d6e49c3a372dba3021af7c834d4a7740b8af84 Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Tue, 26 Apr 2016 03:31:56 +0100 Subject: [PATCH 06/24] Fix spec --- spec/features/consumer/account_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/consumer/account_spec.rb b/spec/features/consumer/account_spec.rb index 1203d459de..58d4e1acc6 100644 --- a/spec/features/consumer/account_spec.rb +++ b/spec/features/consumer/account_spec.rb @@ -14,7 +14,7 @@ feature %q{ let!(:distributor_credit) { create(:distributor_enterprise) } let!(:distributor_without_orders) { create(:distributor_enterprise) } let!(:accounts_distributor) {create :distributor_enterprise} - let!(:order_account_invoice) { create(:order, distributor: accounts_distributor, user: user) } + let!(:order_account_invoice) { create(:order, distributor: accounts_distributor, state: 'complete', user: user) } let!(:d1o1) { create(:completed_order_with_totals, distributor_id: distributor1.id, user_id: user.id, total: 10000)} let!(:d1o2) { create(:order_without_full_payment, distributor_id: distributor1.id, user_id: user.id, total: 5000)} let!(:d2o1) { create(:completed_order_with_totals, distributor_id: distributor2.id, user_id: user.id)} From ad6037ac63193a921ed9bb18c118b79acbd19290 Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Tue, 26 Apr 2016 04:21:50 +0100 Subject: [PATCH 07/24] Fix user spec, check config is set --- app/models/spree/user_decorator.rb | 11 +++++++---- spec/models/spree/user_spec.rb | 8 +++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index ac62fee8d1..011bffc5dd 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -51,10 +51,13 @@ Spree.user_class.class_eval do # Returns Enterprise IDs for distributors that the user has shopped at def enterprises_ordered_from - orders.where(state: :complete) - .where('distributor_id != ?', Spree::Config.accounts_distributor_id) - .map(&:distributor_id) - .uniq + enterprise_ids = orders.where(state: :complete).map(&:distributor_id).uniq + # Exclude the accounts distributor + if Spree::Config.accounts_distributor_id + enterprise_ids = enterprise_ids.keep_if{|a| a != Spree::Config.accounts_distributor_id} + end + enterprise_ids + end # Returns orders and their associated payments for all distributors that have been ordered from diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index 0c15da5378..441757bf38 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -90,11 +90,17 @@ describe Spree.user_class do let!(:d1_order_for_u2) { create(:completed_order_with_totals, distributor: distributor1, user_id: u2.id) } let!(:d1o3) { create(:order, state: 'cart', distributor: distributor1, user_id: u1.id) } let!(:d2o1) { create(:completed_order_with_totals, distributor: distributor2, user_id: u2.id) } + let!(:accounts_distributor) {create :distributor_enterprise} + let!(:order_account_invoice) { create(:order, distributor: accounts_distributor, state: 'complete', user: u1) } let!(:completed_payment) { create(:payment, order: d1o1, state: 'completed') } let!(:payment) { create(:payment, order: d1o2, state: 'checkout') } - it "returns enterprises that the user has ordered from" do + before do + Spree::Config.accounts_distributor_id = accounts_distributor.id + end + + it "returns enterprises that the user has ordered from, excluding accounts distributor" do expect(u1.enterprises_ordered_from).to eq [distributor1.id] end From 2b921542a527bc0bb21f59c9281dbe5a366c1e5d Mon Sep 17 00:00:00 2001 From: Steve Pettitt Date: Tue, 26 Apr 2016 04:24:44 +0100 Subject: [PATCH 08/24] Code styling --- app/models/spree/user_decorator.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 011bffc5dd..afd32bd294 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -54,10 +54,9 @@ Spree.user_class.class_eval do enterprise_ids = orders.where(state: :complete).map(&:distributor_id).uniq # Exclude the accounts distributor if Spree::Config.accounts_distributor_id - enterprise_ids = enterprise_ids.keep_if{|a| a != Spree::Config.accounts_distributor_id} + enterprise_ids = enterprise_ids.keep_if { |a| a != Spree::Config.accounts_distributor_id } end enterprise_ids - end # Returns orders and their associated payments for all distributors that have been ordered from From 8f8a1191cb2866d209985ddc7922c039773468a9 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 Apr 2016 11:44:04 +1000 Subject: [PATCH 09/24] Remove stock cap on max_quantity --- .../darkswarm/services/cart.js.coffee | 2 +- .../shop_variant_with_group_buy.html.haml | 2 +- app/models/spree/line_item_decorator.rb | 7 +------ app/models/spree/order_populator_decorator.rb | 2 +- spec/features/consumer/shopping/shopping_spec.rb | 16 +++++++++------- .../unit/darkswarm/services/cart_spec.js.coffee | 4 ++-- spec/models/spree/line_item_spec.rb | 4 ++-- spec/models/spree/order_populator_spec.rb | 4 ++-- 8 files changed, 19 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/darkswarm/services/cart.js.coffee b/app/assets/javascripts/darkswarm/services/cart.js.coffee index d75ef7a628..9ee03e26b3 100644 --- a/app/assets/javascripts/darkswarm/services/cart.js.coffee +++ b/app/assets/javascripts/darkswarm/services/cart.js.coffee @@ -54,7 +54,7 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo if li.quantity > li.variant.count_on_hand li.quantity = li.variant.count_on_hand scope.variants.push li.variant - if li.max_quantity > li.variant.count_on_hand + if li.variant.count_on_hand == 0 && li.max_quantity > li.variant.count_on_hand li.max_quantity = li.variant.count_on_hand scope.variants.push(li.variant) unless li.variant in scope.variants diff --git a/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml b/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml index c44a71948b..f2b0b95837 100644 --- a/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml +++ b/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml @@ -18,6 +18,6 @@ "ng-model" => "variant.line_item.max_quantity", placeholder: "{{'shop_variant_quantity_max' | t}}", "ofn-disable-scroll" => true, - max: "{{variant.on_demand && 9999 || variant.count_on_hand }}", + min: "{{variant.line_item.quantity}}", name: "variant_attributes[{{variant.id}}][max_quantity]", id: "variants_{{variant.id}}_max"} diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index b9f8af577f..8a89d8fdef 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -44,12 +44,7 @@ Spree::LineItem.class_eval do def cap_quantity_at_stock! - attrs = {} - - attrs[:quantity] = variant.on_hand if quantity > variant.on_hand - attrs[:max_quantity] = variant.on_hand if (max_quantity || 0) > variant.on_hand - - update_attributes!(attrs) if attrs.any? + update_attributes!(quantity: variant.on_hand) if quantity > variant.on_hand end diff --git a/app/models/spree/order_populator_decorator.rb b/app/models/spree/order_populator_decorator.rb index a106de6936..db03f1f2e2 100644 --- a/app/models/spree/order_populator_decorator.rb +++ b/app/models/spree/order_populator_decorator.rb @@ -77,7 +77,7 @@ Spree::OrderPopulator.class_eval do on_hand = variant.on_hand on_hand = [quantity, max_quantity].compact.max if Spree::Config.allow_backorders quantity_to_add = [quantity, on_hand].min - max_quantity_to_add = [max_quantity, on_hand].min if max_quantity + max_quantity_to_add = max_quantity # max_quantity is not capped [quantity_to_add, max_quantity_to_add] end diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb index 9dc0a39dac..5b617e7db6 100644 --- a/spec/features/consumer/shopping/shopping_spec.rb +++ b/spec/features/consumer/shopping/shopping_spec.rb @@ -288,33 +288,32 @@ feature "As a consumer I want to shop with a distributor", js: true do # Update amount available in product list # If amount falls to zero, variant should be greyed out and input disabled page.should have_selector "#variant-#{variant.id}.out-of-stock" - page.should have_selector "#variants_#{variant.id}_max[max='0']" page.should have_selector "#variants_#{variant.id}_max[disabled='disabled']" end end context "when the update is for another product" do it "updates quantity" do - fill_in "variants[#{variant.id}]", with: '1' + fill_in "variants[#{variant.id}]", with: '2' wait_until { !cart_dirty } - variant.update_attributes! on_hand: 0 + variant.update_attributes! on_hand: 1 fill_in "variants[#{variant2.id}]", with: '1' wait_until { !cart_dirty } within(".out-of-stock-modal") do page.should have_content "stock levels for one or more of the products in your cart have reduced" - page.should have_content "#{product.name} - #{variant.unit_to_display} is now out of stock." + page.should have_content "#{product.name} - #{variant.unit_to_display} now only has 1 remaining" end end context "group buy products" do let(:product) { create(:simple_product, group_buy: true) } - it "updates max_quantity" do - fill_in "variants[#{variant.id}]", with: '1' - fill_in "variant_attributes[#{variant.id}][max_quantity]", with: '2' + it "does not update max_quantity" do + fill_in "variants[#{variant.id}]", with: '2' + fill_in "variant_attributes[#{variant.id}][max_quantity]", with: '3' wait_until { !cart_dirty } variant.update_attributes! on_hand: 1 @@ -325,6 +324,9 @@ feature "As a consumer I want to shop with a distributor", js: true do page.should have_content "stock levels for one or more of the products in your cart have reduced" page.should have_content "#{product.name} - #{variant.unit_to_display} now only has 1 remaining" end + + page.should have_field "variants[#{variant.id}]", with: '1' + page.should have_field "variant_attributes[#{variant.id}][max_quantity]", with: '3' end end end diff --git a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee index 6df74e3ede..d985904f81 100644 --- a/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee @@ -159,12 +159,12 @@ describe 'Cart service', -> expect(li.quantity).toEqual 5 expect(li.max_quantity).toBeUndefined() - it "reduces the max_quantity in the cart", -> + it "does not reduce the max_quantity in the cart", -> li = {variant: {id: 1}, quantity: 6, max_quantity: 7} stockLevels = {1: {quantity: 5, max_quantity: 5, on_hand: 5}} spyOn(Cart, 'line_items_present').andReturn [li] Cart.compareAndNotifyStockLevels stockLevels - expect(li.max_quantity).toEqual 5 + expect(li.max_quantity).toEqual 7 it "resets the count on hand available", -> li = {variant: {id: 1}, quantity: 6} diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index 2b5362e94f..111108f7b4 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -55,9 +55,9 @@ module Spree li.reload.quantity.should == 5 end - it "caps max_quantity" do + it "does not cap max_quantity" do li.cap_quantity_at_stock! - li.reload.max_quantity.should == 5 + li.reload.max_quantity.should == 10 end it "works for products without max_quantity" do diff --git a/spec/models/spree/order_populator_spec.rb b/spec/models/spree/order_populator_spec.rb index 4a50e59fa8..f0bbc81f55 100644 --- a/spec/models/spree/order_populator_spec.rb +++ b/spec/models/spree/order_populator_spec.rb @@ -213,8 +213,8 @@ module Spree op.quantities_to_add(v, 5, 6).should == [5, 6] end - it "returns a limited amount when not entirely available" do - op.quantities_to_add(v, 15, 16).should == [10, 10] + it "also returns the full amount when not entirely available" do + op.quantities_to_add(v, 15, 16).should == [10, 16] end end end From cf40e0432abde6c54c72fbd9c5a437492deaffa2 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 Apr 2016 12:22:51 +1000 Subject: [PATCH 10/24] When cart is updated with insufficient stock, show amount in cart, not amount entered --- .../spree/orders_controller_decorator.rb | 37 ++++++++++++++ app/views/spree/orders/_line_item.html.haml | 2 +- spec/features/consumer/shopping/cart_spec.rb | 50 +++++++++++++------ 3 files changed, 73 insertions(+), 16 deletions(-) diff --git a/app/controllers/spree/orders_controller_decorator.rb b/app/controllers/spree/orders_controller_decorator.rb index e675ce7e94..2c0ff8562c 100644 --- a/app/controllers/spree/orders_controller_decorator.rb +++ b/app/controllers/spree/orders_controller_decorator.rb @@ -16,6 +16,7 @@ Spree::OrdersController.class_eval do # Patching to redirect to shop if order is empty def edit @order = current_order(true) + @insufficient_stock_lines = @order.insufficient_stock_lines if @order.line_items.empty? redirect_to main_app.shop_path @@ -28,6 +29,41 @@ Spree::OrdersController.class_eval do end end + + def update + @insufficient_stock_lines = [] + @order = current_order + unless @order + flash[:error] = t(:order_not_found) + redirect_to root_path and return + end + + if @order.update_attributes(params[:order]) + @order.line_items = @order.line_items.select {|li| li.quantity > 0 } + @order.restart_checkout_flow + + render :edit and return unless apply_coupon_code + + fire_event('spree.order.contents_changed') + respond_with(@order) do |format| + format.html do + if params.has_key?(:checkout) + @order.next_transition.run_callbacks if @order.cart? + redirect_to checkout_state_path(@order.checkout_steps.first) + else + redirect_to cart_path + end + end + end + else + # Show order with original values, not newly entered ones + @insufficient_stock_lines = @order.insufficient_stock_lines + @order.line_items(true) + respond_with(@order) + end + end + + def populate # Without intervention, the Spree::Adjustment#update_adjustable callback is called many times # during cart population, for both taxation and enterprise fees. This operation triggers a @@ -55,6 +91,7 @@ Spree::OrdersController.class_eval do end end + # Report the stock levels in the order for all variant ids requested def stock_levels(order, variant_ids) stock_levels = li_stock_levels(order) diff --git a/app/views/spree/orders/_line_item.html.haml b/app/views/spree/orders/_line_item.html.haml index 6014b617a9..f4415199a4 100644 --- a/app/views/spree/orders/_line_item.html.haml +++ b/app/views/spree/orders/_line_item.html.haml @@ -17,7 +17,7 @@ = render 'spree/shared/line_item_name', line_item: line_item - - if @order.insufficient_stock_lines.include? line_item + - if @insufficient_stock_lines.include? line_item %span.out-of-stock = variant.in_stock? ? t(:insufficient_stock, :on_hand => variant.on_hand) : t(:out_of_stock) %br/ diff --git a/spec/features/consumer/shopping/cart_spec.rb b/spec/features/consumer/shopping/cart_spec.rb index a80fd908ff..5097cb50b8 100644 --- a/spec/features/consumer/shopping/cart_spec.rb +++ b/spec/features/consumer/shopping/cart_spec.rb @@ -7,25 +7,45 @@ feature "full-page cart", js: true do include UIComponentHelper describe "viewing the cart" do + let!(:zone) { create(:zone_with_member) } + let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true, charges_sales_tax: true) } + let(:supplier) { create(:supplier_enterprise) } + let!(:order_cycle) { create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], coordinator: create(:distributor_enterprise), variants: [product.variants.first]) } + let(:enterprise_fee) { create(:enterprise_fee, amount: 11.00, tax_category: product.tax_category) } + let(:product) { create(:taxed_product, supplier: supplier, zone: zone, price: 110.00, tax_rate_amount: 0.1) } + let(:variant) { product.variants.first } + let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor) } + + before do + add_enterprise_fee enterprise_fee + set_order order + add_product_to_cart + visit spree.cart_path + end + describe "tax" do - let!(:zone) { create(:zone_with_member) } - let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true, charges_sales_tax: true) } - let(:supplier) { create(:supplier_enterprise) } - let!(:order_cycle) { create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], coordinator: create(:distributor_enterprise), variants: [product.variants.first]) } - let(:enterprise_fee) { create(:enterprise_fee, amount: 11.00, tax_category: product.tax_category) } - let(:product) { create(:taxed_product, supplier: supplier, zone: zone, price: 110.00, tax_rate_amount: 0.1) } - let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor) } - - before do - add_enterprise_fee enterprise_fee - set_order order - add_product_to_cart - visit spree.cart_path - end - it "shows the total tax for the order, including product tax and tax on fees" do page.should have_selector '.tax-total', text: '11.00' # 10 + 1 end end + + describe "updating quantities with insufficient stock available" do + let(:li) { order.line_items(true).last } + + use_short_wait 10 + + before do + variant.update_attributes! on_hand: 2 + end + + it "shows the quantities saved, not those submitted" do + fill_in "order_line_items_attributes_0_quantity", with: '4' + + click_button 'Update' + + page.should have_field "order[line_items_attributes][0][quantity]", with: '1' + page.should have_content "Insufficient stock available, only 2 remaining" + end + end end end From 36f4df29315f9bfd3338a4449f2704ddb4b3308a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 Apr 2016 12:27:20 +1000 Subject: [PATCH 11/24] Allow max value in cart of what's on hand --- app/views/spree/orders/_line_item.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/orders/_line_item.html.haml b/app/views/spree/orders/_line_item.html.haml index f4415199a4..018cd9fa55 100644 --- a/app/views/spree/orders/_line_item.html.haml +++ b/app/views/spree/orders/_line_item.html.haml @@ -30,7 +30,7 @@ -# "price-breakdown-placement" => "left", -# "price-breakdown-animation" => true} %td.text-center.cart-item-quantity{"data-hook" => "cart_item_quantity"} - = item_form.number_field :quantity, :min => 0, :class => "line_item_quantity", :size => 5 + = item_form.number_field :quantity, :min => 0, :max => variant.on_hand, :class => "line_item_quantity", :size => 5 %td.cart-item-total.text-right{"data-hook" => "cart_item_total"} = line_item.display_amount_with_adjustments.to_html unless line_item.quantity.nil? From daa5b00a2aa2851bc0a782bea854b3893087af63 Mon Sep 17 00:00:00 2001 From: Bing Xie Date: Fri, 15 Apr 2016 12:22:17 +1000 Subject: [PATCH 12/24] Uses openstreetmap tiles --- .../darkswarm/directives/map_search.js.coffee | 15 +++++++++++++++ .../services/map_configuration.js.coffee | 9 ++++++--- app/assets/stylesheets/darkswarm/map.css.sass | 13 +++++++++++++ app/views/map/index.html.haml | 5 +++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/darkswarm/directives/map_search.js.coffee b/app/assets/javascripts/darkswarm/directives/map_search.js.coffee index f67407ffb3..20a2cba1d4 100644 --- a/app/assets/javascripts/darkswarm/directives/map_search.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/map_search.js.coffee @@ -7,6 +7,21 @@ Darkswarm.directive 'mapSearch', ($timeout)-> link: (scope, elem, attrs, ctrl)-> $timeout => map = ctrl.getMap() + + map.mapTypes.set 'OSM', new (google.maps.ImageMapType)( + getTileUrl: (coord, zoom) -> + # "Wrap" x (logitude) at 180th meridian properly + # NB: Don't touch coord.x because coord param is by reference, and changing its x property breakes something in Google's lib + tilesPerGlobe = 1 << zoom + x = coord.x % tilesPerGlobe + if x < 0 + x = tilesPerGlobe + x + # Wrap y (latitude) in a like manner if you want to enable vertical infinite scroll + 'http://tile.openstreetmap.org/' + zoom + '/' + x + '/' + coord.y + '.png' + tileSize: new (google.maps.Size)(256, 256) + name: 'OpenStreetMap' + maxZoom: 18) + input = (document.getElementById("pac-input")) map.controls[google.maps.ControlPosition.TOP_LEFT].push input searchBox = new google.maps.places.SearchBox((input)) diff --git a/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee b/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee index 9c5a375d8c..992872e2da 100644 --- a/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee @@ -1,11 +1,14 @@ Darkswarm.factory "MapConfiguration", -> new class MapConfiguration options: - center: + center: latitude: -37.4713077 longitude: 144.7851531 zoom: 12 - additional_options: {} - #mapTypeId: 'satellite' + additional_options: + #mapTypeId: 'satellite' + mapTypeId: 'OSM' + mapTypeControl: false + streetViewControl: false styles: [{"featureType":"landscape","stylers":[{"saturation":-100},{"lightness":65},{"visibility":"on"}]},{"featureType":"poi","stylers":[{"saturation":-100},{"lightness":51},{"visibility":"simplified"}]},{"featureType":"road.highway","stylers":[{"saturation":-100},{"visibility":"simplified"}]},{"featureType":"road.arterial","stylers":[{"saturation":-100},{"lightness":30},{"visibility":"on"}]},{"featureType":"road.local","stylers":[{"saturation":-100},{"lightness":40},{"visibility":"on"}]},{"featureType":"transit","stylers":[{"saturation":-100},{"visibility":"simplified"}]},{"featureType":"administrative.province","stylers":[{"visibility":"off"}]},{"featureType":"water","elementType":"labels","stylers":[{"visibility":"on"},{"lightness":-25},{"saturation":-100}]},{"featureType":"water","elementType":"geometry","stylers":[{"hue":"#ffff00"},{"lightness":-25},{"saturation":-97}]},{"featureType":"road","elementType": "labels.icon","stylers":[{"visibility":"off"}]}] diff --git a/app/assets/stylesheets/darkswarm/map.css.sass b/app/assets/stylesheets/darkswarm/map.css.sass index a831e9448c..8a63803816 100644 --- a/app/assets/stylesheets/darkswarm/map.css.sass +++ b/app/assets/stylesheets/darkswarm/map.css.sass @@ -26,3 +26,16 @@ width: 80% &:active, &:focus, &.active background: rgba(255,255,255, 1) + +.map-footer + position: fixed + font-size: x-small + left: 0 + right: 0 + bottom: 0 + width: 100% + height: 23px + margin: 0 + padding: 6px + z-index: 2 + background: WHITE diff --git a/app/views/map/index.html.haml b/app/views/map/index.html.haml index a4d012282b..30948b8724 100644 --- a/app/views/map/index.html.haml +++ b/app/views/map/index.html.haml @@ -9,3 +9,8 @@ %map-search %markers{models: "OfnMap.enterprises", fit: "true", coords: "'self'", icon: "'icon'", click: "'reveal'"} + +.map-footer + \© + %a{:href => "http://www.openstreetmap.org/copyright"} OpenStreetMap + contributors From 824a29624fbba3bf270e8fa5f17a6d4a2c50e146 Mon Sep 17 00:00:00 2001 From: Bing Xie Date: Wed, 27 Apr 2016 14:17:29 +1000 Subject: [PATCH 13/24] Tweak the map UI --- .../javascripts/darkswarm/directives/map_search.js.coffee | 3 ++- .../javascripts/darkswarm/services/map_configuration.js.coffee | 2 +- app/assets/stylesheets/darkswarm/map.css.sass | 3 ++- app/views/map/index.html.haml | 3 +-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/darkswarm/directives/map_search.js.coffee b/app/assets/javascripts/darkswarm/directives/map_search.js.coffee index 20a2cba1d4..e67ce529a5 100644 --- a/app/assets/javascripts/darkswarm/directives/map_search.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/map_search.js.coffee @@ -8,6 +8,7 @@ Darkswarm.directive 'mapSearch', ($timeout)-> $timeout => map = ctrl.getMap() + # Use OSM tiles server map.mapTypes.set 'OSM', new (google.maps.ImageMapType)( getTileUrl: (coord, zoom) -> # "Wrap" x (logitude) at 180th meridian properly @@ -36,7 +37,7 @@ Darkswarm.directive 'mapSearch', ($timeout)-> #map.setCenter place.geometry.location map.fitBounds place.geometry.viewport #map.fitBounds bounds - + # Bias the SearchBox results towards places that are within the bounds of the # current map's viewport. google.maps.event.addListener map, "bounds_changed", -> diff --git a/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee b/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee index 992872e2da..1979affeab 100644 --- a/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map_configuration.js.coffee @@ -6,7 +6,7 @@ Darkswarm.factory "MapConfiguration", -> longitude: 144.7851531 zoom: 12 additional_options: - #mapTypeId: 'satellite' + # mapTypeId: 'satellite' mapTypeId: 'OSM' mapTypeControl: false streetViewControl: false diff --git a/app/assets/stylesheets/darkswarm/map.css.sass b/app/assets/stylesheets/darkswarm/map.css.sass index 8a63803816..4692bdc1b4 100644 --- a/app/assets/stylesheets/darkswarm/map.css.sass +++ b/app/assets/stylesheets/darkswarm/map.css.sass @@ -22,6 +22,7 @@ background: rgba(255,255,255,0.85) width: 50% margin-top: 1.2rem + margin-left: 1rem @media all and (max-width: 768px) width: 80% &:active, &:focus, &.active @@ -38,4 +39,4 @@ margin: 0 padding: 6px z-index: 2 - background: WHITE + background: #fff diff --git a/app/views/map/index.html.haml b/app/views/map/index.html.haml index 30948b8724..97f4eb602d 100644 --- a/app/views/map/index.html.haml +++ b/app/views/map/index.html.haml @@ -12,5 +12,4 @@ .map-footer \© - %a{:href => "http://www.openstreetmap.org/copyright"} OpenStreetMap - contributors + %a{:href => "http://www.openstreetmap.org/copyright"} OpenStreetMap contributors From f691636c757b452f6a173d08e38e5375ec1edcd2 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 Apr 2016 14:47:45 +1000 Subject: [PATCH 14/24] Fix spec --- spec/controllers/spree/orders_controller_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/controllers/spree/orders_controller_spec.rb b/spec/controllers/spree/orders_controller_spec.rb index 286bb9217a..74e9efaaf9 100644 --- a/spec/controllers/spree/orders_controller_spec.rb +++ b/spec/controllers/spree/orders_controller_spec.rb @@ -15,6 +15,7 @@ describe Spree::OrdersController do controller.stub(:current_order_cycle).and_return(order_cycle) controller.stub(:current_order).and_return order order.stub_chain(:line_items, :empty?).and_return true + order.stub(:insufficient_stock_lines).and_return [] session[:access_token] = order.token spree_get :edit response.should redirect_to shop_path From 1220ff8a068ff21fce960f06bf04d755c3fd0986 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 Apr 2016 15:05:44 +1000 Subject: [PATCH 15/24] Notify when stock limit reached on shopfront rather than silently capping --- .../darkswarm/directives/on_hand.js.coffee | 9 +++++++++ .../partials/shop_variant_no_group_buy.html.haml | 2 +- .../partials/shop_variant_with_group_buy.html.haml | 2 +- spec/features/consumer/shopping/shopping_spec.rb | 11 +++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/darkswarm/directives/on_hand.js.coffee diff --git a/app/assets/javascripts/darkswarm/directives/on_hand.js.coffee b/app/assets/javascripts/darkswarm/directives/on_hand.js.coffee new file mode 100644 index 0000000000..0e3a96c608 --- /dev/null +++ b/app/assets/javascripts/darkswarm/directives/on_hand.js.coffee @@ -0,0 +1,9 @@ +Darkswarm.directive "ofnOnHand", -> + restrict: 'A' + link: (scope, elem, attr) -> + on_hand = parseInt(attr.ofnOnHand) + elem.bind 'change', (e) -> + if parseInt(elem.val()) > on_hand + scope.$apply -> + alert t('insufficient_stock', {on_hand: on_hand}) + elem.val(on_hand) diff --git a/app/assets/javascripts/templates/partials/shop_variant_no_group_buy.html.haml b/app/assets/javascripts/templates/partials/shop_variant_no_group_buy.html.haml index 36a4e44ade..1153d93b0f 100644 --- a/app/assets/javascripts/templates/partials/shop_variant_no_group_buy.html.haml +++ b/app/assets/javascripts/templates/partials/shop_variant_no_group_buy.html.haml @@ -7,6 +7,6 @@ placeholder: "0", "ofn-disable-scroll" => true, "ng-model" => "variant.line_item.quantity", - max: "{{variant.on_demand && 9999 || variant.count_on_hand }}", + "ofn-on-hand" => "{{variant.on_demand && 9999 || variant.count_on_hand }}", "ng-disabled" => "!variant.on_demand && variant.count_on_hand == 0", name: "variants[{{variant.id}}]", id: "variants_{{variant.id}}"} diff --git a/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml b/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml index f2b0b95837..6601f0c019 100644 --- a/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml +++ b/app/assets/javascripts/templates/partials/shop_variant_with_group_buy.html.haml @@ -8,7 +8,7 @@ "ng-model" => "variant.line_item.quantity", placeholder: "{{'shop_variant_quantity_min' | t}}", "ofn-disable-scroll" => true, - max: "{{variant.on_demand && 9999 || variant.count_on_hand }}", + "ofn-on-hand" => "{{variant.on_demand && 9999 || variant.count_on_hand }}", name: "variants[{{variant.id}}]", id: "variants_{{variant.id}}"} %span.bulk-input %input.bulk.second{type: :number, diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb index 5b617e7db6..2ca2d63341 100644 --- a/spec/features/consumer/shopping/shopping_spec.rb +++ b/spec/features/consumer/shopping/shopping_spec.rb @@ -238,6 +238,17 @@ feature "As a consumer I want to shop with a distributor", js: true do Spree::LineItem.where(id: li).should be_empty end + it "alerts us when we enter a quantity greater than the stock available" do + variant.update_attributes on_hand: 5 + visit shop_path + + accept_alert 'Insufficient stock available, only 5 remaining' do + fill_in "variants[#{variant.id}]", with: '10' + end + + page.should have_field "variants[#{variant.id}]", with: '5' + end + describe "when a product goes out of stock just before it's added to the cart" do it "stops the attempt, shows an error message and refreshes the products asynchronously" do variant.update_attributes! on_hand: 0 From 1384140e41b4266caa8e933d3af4d895357fa4e4 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 Apr 2016 15:10:54 +1000 Subject: [PATCH 16/24] Notify when stock limit reached on cart rather than silently capping --- app/views/spree/orders/_line_item.html.haml | 2 +- spec/features/consumer/shopping/cart_spec.rb | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/app/views/spree/orders/_line_item.html.haml b/app/views/spree/orders/_line_item.html.haml index 018cd9fa55..3897654467 100644 --- a/app/views/spree/orders/_line_item.html.haml +++ b/app/views/spree/orders/_line_item.html.haml @@ -30,7 +30,7 @@ -# "price-breakdown-placement" => "left", -# "price-breakdown-animation" => true} %td.text-center.cart-item-quantity{"data-hook" => "cart_item_quantity"} - = item_form.number_field :quantity, :min => 0, :max => variant.on_hand, :class => "line_item_quantity", :size => 5 + = item_form.number_field :quantity, :min => 0, "ofn-on-hand" => variant.on_hand, :class => "line_item_quantity", :size => 5 %td.cart-item-total.text-right{"data-hook" => "cart_item_total"} = line_item.display_amount_with_adjustments.to_html unless line_item.quantity.nil? diff --git a/spec/features/consumer/shopping/cart_spec.rb b/spec/features/consumer/shopping/cart_spec.rb index 5097cb50b8..ed573d0460 100644 --- a/spec/features/consumer/shopping/cart_spec.rb +++ b/spec/features/consumer/shopping/cart_spec.rb @@ -32,12 +32,20 @@ feature "full-page cart", js: true do describe "updating quantities with insufficient stock available" do let(:li) { order.line_items(true).last } - use_short_wait 10 - before do variant.update_attributes! on_hand: 2 end + it "prevents me from entering an invalid value" do + visit spree.cart_path + + accept_alert 'Insufficient stock available, only 2 remaining' do + fill_in "order_line_items_attributes_0_quantity", with: '4' + end + + page.should have_field "order_line_items_attributes_0_quantity", with: '2' + end + it "shows the quantities saved, not those submitted" do fill_in "order_line_items_attributes_0_quantity", with: '4' From 8996acf314adc698aa36faa9c946126541813a0d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 Apr 2016 15:25:05 +1000 Subject: [PATCH 17/24] Fix spec --- spec/features/consumer/shopping/shopping_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb index 2ca2d63341..dceecd43f8 100644 --- a/spec/features/consumer/shopping/shopping_spec.rb +++ b/spec/features/consumer/shopping/shopping_spec.rb @@ -270,7 +270,7 @@ feature "As a consumer I want to shop with a distributor", js: true do # Update amount available in product list # If amount falls to zero, variant should be greyed out and input disabled page.should have_selector "#variant-#{variant.id}.out-of-stock" - page.should have_selector "#variants_#{variant.id}[max='0']" + page.should have_selector "#variants_#{variant.id}[ofn-on-hand='0']" page.should have_selector "#variants_#{variant.id}[disabled='disabled']" end From 65895752dacf1541e25f969867a713067e1ee5c0 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 Apr 2016 11:49:30 +1000 Subject: [PATCH 18/24] Remove cruft --- app/assets/javascripts/admin/directives/track_master.js.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/directives/track_master.js.coffee b/app/assets/javascripts/admin/directives/track_master.js.coffee index ce26a4aa32..be84e0b204 100644 --- a/app/assets/javascripts/admin/directives/track_master.js.coffee +++ b/app/assets/javascripts/admin/directives/track_master.js.coffee @@ -1,4 +1,4 @@ -angular.module("ofn.admin").directive "ofnTrackMaster", ["DirtyProducts", (DirtyProducts) -> +angular.module("ofn.admin").directive "ofnTrackMaster", (DirtyProducts) -> require: "ngModel" link: (scope, element, attrs, ngModel) -> ngModel.$parsers.push (viewValue) -> @@ -6,4 +6,3 @@ angular.module("ofn.admin").directive "ofnTrackMaster", ["DirtyProducts", (Dirty DirtyProducts.addMasterProperty scope.product.id, scope.product.master.id, attrs.ofnTrackMaster, viewValue scope.displayDirtyProducts() viewValue - ] \ No newline at end of file From 88e9eb59cf5de3453965bca310bd72dddcaf8a40 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 Apr 2016 11:56:48 +1000 Subject: [PATCH 19/24] Do not allow invalid quantity to reach model, triggering server update --- .../darkswarm/directives/on_hand.js.coffee | 24 +++++++++++++------ app/views/spree/orders/_line_item.html.haml | 2 +- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/darkswarm/directives/on_hand.js.coffee b/app/assets/javascripts/darkswarm/directives/on_hand.js.coffee index 0e3a96c608..610b11d3ca 100644 --- a/app/assets/javascripts/darkswarm/directives/on_hand.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/on_hand.js.coffee @@ -1,9 +1,19 @@ Darkswarm.directive "ofnOnHand", -> restrict: 'A' - link: (scope, elem, attr) -> - on_hand = parseInt(attr.ofnOnHand) - elem.bind 'change', (e) -> - if parseInt(elem.val()) > on_hand - scope.$apply -> - alert t('insufficient_stock', {on_hand: on_hand}) - elem.val(on_hand) + require: "ngModel" + + link: (scope, elem, attr, ngModel) -> + # In cases where this field gets its value from the HTML element rather than the model, + # initialise the model with the HTML value. + if scope.$eval(attr.ngModel) == undefined + ngModel.$setViewValue elem.val() + + ngModel.$parsers.push (viewValue) -> + on_hand = parseInt(attr.ofnOnHand) + if parseInt(viewValue) > on_hand + alert t('insufficient_stock', {on_hand: on_hand}) + viewValue = on_hand + ngModel.$setViewValue viewValue + ngModel.$render() + + viewValue diff --git a/app/views/spree/orders/_line_item.html.haml b/app/views/spree/orders/_line_item.html.haml index 3897654467..5976332a18 100644 --- a/app/views/spree/orders/_line_item.html.haml +++ b/app/views/spree/orders/_line_item.html.haml @@ -30,7 +30,7 @@ -# "price-breakdown-placement" => "left", -# "price-breakdown-animation" => true} %td.text-center.cart-item-quantity{"data-hook" => "cart_item_quantity"} - = item_form.number_field :quantity, :min => 0, "ofn-on-hand" => variant.on_hand, :class => "line_item_quantity", :size => 5 + = item_form.number_field :quantity, :min => 0, "ofn-on-hand" => variant.on_hand, "ng-model" => "line_item_#{line_item.id}", :class => "line_item_quantity", :size => 5 %td.cart-item-total.text-right{"data-hook" => "cart_item_total"} = line_item.display_amount_with_adjustments.to_html unless line_item.quantity.nil? From 36a4aab020edc8c968e2f3754bc07854df9b01b8 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 Apr 2016 15:07:55 +1000 Subject: [PATCH 20/24] Adjust styling to blend --- app/assets/stylesheets/darkswarm/map.css.sass | 20 +++++++++++++------ app/views/map/index.html.haml | 3 +-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/app/assets/stylesheets/darkswarm/map.css.sass b/app/assets/stylesheets/darkswarm/map.css.sass index 4692bdc1b4..e2a25d61c9 100644 --- a/app/assets/stylesheets/darkswarm/map.css.sass +++ b/app/assets/stylesheets/darkswarm/map.css.sass @@ -30,13 +30,21 @@ .map-footer position: fixed - font-size: x-small - left: 0 - right: 0 - bottom: 0 + z-index: 2 width: 100% height: 23px + left: 80px + right: 0 + bottom: 6px margin: 0 padding: 6px - z-index: 2 - background: #fff + font-size: 14px + font-weight: bold + text-shadow: 2px 2px #aaa + color: #fff + + a, a:hover, a:active, a:focus + color: #fff + + @media all and (max-width: 1025px) + left: 0px \ No newline at end of file diff --git a/app/views/map/index.html.haml b/app/views/map/index.html.haml index 97f4eb602d..e4a22e540c 100644 --- a/app/views/map/index.html.haml +++ b/app/views/map/index.html.haml @@ -11,5 +11,4 @@ coords: "'self'", icon: "'icon'", click: "'reveal'"} .map-footer - \© - %a{:href => "http://www.openstreetmap.org/copyright"} OpenStreetMap contributors + %a{:href => "http://www.openstreetmap.org/copyright"} © OpenStreetMap contributors From fa5fa9e2286554617996182424056f9f29569edd Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 29 Apr 2016 12:33:33 +1000 Subject: [PATCH 21/24] Auto-complete tags on customers page - new controller serving tags for an enterprise as JSON - customers page suggesting these tags - emphasising tags that have rules --- .../customers_controller.js.coffee | 12 +++++++- .../services/tags_resource.js.coffee | 9 ++++++ .../admin/filters/translate.js.coffee | 7 ----- .../shipping_methods.js.coffee | 2 +- .../tags_with_translation.js.coffee | 3 +- .../admin/utils/filters/translate.js.coffee | 7 +++++ .../javascripts/templates/admin/tag.html.haml | 8 +++++ .../admin/tag_autocomplete.html.haml | 11 +++++++ .../templates/admin/tags_input.html.haml | 7 +++++ .../stylesheets/admin/customers.css.scss | 3 ++ app/controllers/admin/tags_controller.rb | 28 ++++++++++++++++++ app/models/enterprise.rb | 14 +++++++++ app/models/spree/ability_decorator.rb | 9 ++---- .../api/admin/customer_serializer.rb | 6 +++- app/views/admin/customers/index.html.haml | 2 +- config/locales/en.yml | 4 +++ config/routes.rb | 2 ++ .../customers_controller_spec.js.coffee | 29 +++++++++++++++++++ .../admin/customer_serializer_spec.rb | 14 +++++++++ 19 files changed, 159 insertions(+), 18 deletions(-) create mode 100644 app/assets/javascripts/admin/customers/services/tags_resource.js.coffee delete mode 100644 app/assets/javascripts/admin/filters/translate.js.coffee create mode 100644 app/assets/javascripts/admin/utils/filters/translate.js.coffee create mode 100644 app/assets/javascripts/templates/admin/tag.html.haml create mode 100644 app/assets/javascripts/templates/admin/tag_autocomplete.html.haml create mode 100644 app/assets/javascripts/templates/admin/tags_input.html.haml create mode 100644 app/assets/stylesheets/admin/customers.css.scss create mode 100644 app/controllers/admin/tags_controller.rb create mode 100644 spec/serializers/admin/customer_serializer_spec.rb diff --git a/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee b/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee index d2e6d58562..bfccfd3f4b 100644 --- a/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee +++ b/app/assets/javascripts/admin/customers/controllers/customers_controller.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.customers").controller "customersCtrl", ($scope, CustomerResource, Columns, pendingChanges, shops) -> +angular.module("admin.customers").controller "customersCtrl", ($scope, CustomerResource, TagsResource, $q, Columns, pendingChanges, shops) -> $scope.shop = {} $scope.shops = shops $scope.submitAll = pendingChanges.submitAll @@ -12,6 +12,16 @@ angular.module("admin.customers").controller "customersCtrl", ($scope, CustomerR if $scope.shop.id? $scope.customers = index {enterprise_id: $scope.shop.id} + $scope.findTags = (query) -> + defer = $q.defer() + params = + enterprise_id: $scope.shop.id + TagsResource.index params, (data) => + filtered = data.filter (tag) -> + tag.text.toLowerCase().indexOf(query.toLowerCase()) != -1 + defer.resolve filtered + defer.promise + $scope.add = (email) -> params = enterprise_id: $scope.shop.id diff --git a/app/assets/javascripts/admin/customers/services/tags_resource.js.coffee b/app/assets/javascripts/admin/customers/services/tags_resource.js.coffee new file mode 100644 index 0000000000..ad89511e32 --- /dev/null +++ b/app/assets/javascripts/admin/customers/services/tags_resource.js.coffee @@ -0,0 +1,9 @@ +angular.module("admin.customers").factory 'TagsResource', ($resource) -> + $resource('/admin/tags.json', {}, { + 'index': + method: 'GET' + isArray: true + cache: true + params: + enterprise_id: '@enterprise_id' + }) diff --git a/app/assets/javascripts/admin/filters/translate.js.coffee b/app/assets/javascripts/admin/filters/translate.js.coffee deleted file mode 100644 index 20becc147a..0000000000 --- a/app/assets/javascripts/admin/filters/translate.js.coffee +++ /dev/null @@ -1,7 +0,0 @@ -angular.module('ofn.admin').filter "translate", -> - (key, options) -> - t(key, options) - -angular.module('ofn.admin').filter "t", -> - (key, options) -> - t(key, options) diff --git a/app/assets/javascripts/admin/shipping_methods/shipping_methods.js.coffee b/app/assets/javascripts/admin/shipping_methods/shipping_methods.js.coffee index 232eee7045..863163a9ef 100644 --- a/app/assets/javascripts/admin/shipping_methods/shipping_methods.js.coffee +++ b/app/assets/javascripts/admin/shipping_methods/shipping_methods.js.coffee @@ -1 +1 @@ -angular.module("admin.shippingMethods", ["ngTagsInput", 'admin.utils']) +angular.module("admin.shippingMethods", ["ngTagsInput", 'admin.utils', 'templates']) diff --git a/app/assets/javascripts/admin/utils/directives/tags_with_translation.js.coffee b/app/assets/javascripts/admin/utils/directives/tags_with_translation.js.coffee index 6ce7953608..7a6ae9afff 100644 --- a/app/assets/javascripts/admin/utils/directives/tags_with_translation.js.coffee +++ b/app/assets/javascripts/admin/utils/directives/tags_with_translation.js.coffee @@ -1,10 +1,11 @@ angular.module("admin.utils").directive "tagsWithTranslation", ($timeout) -> restrict: "E" - template: "" + templateUrl: "admin/tags_input.html" scope: object: "=" tagsAttr: "@?" tagListAttr: "@?" + findTags: "&" link: (scope, element, attrs) -> $timeout -> scope.tagsAttr ||= "tags" diff --git a/app/assets/javascripts/admin/utils/filters/translate.js.coffee b/app/assets/javascripts/admin/utils/filters/translate.js.coffee new file mode 100644 index 0000000000..1a0602a59d --- /dev/null +++ b/app/assets/javascripts/admin/utils/filters/translate.js.coffee @@ -0,0 +1,7 @@ +angular.module("admin.utils").filter "translate", -> + (key, options) -> + t(key, options) + +angular.module("admin.utils").filter "t", -> + (key, options) -> + t(key, options) diff --git a/app/assets/javascripts/templates/admin/tag.html.haml b/app/assets/javascripts/templates/admin/tag.html.haml new file mode 100644 index 0000000000..c4afcfa5ad --- /dev/null +++ b/app/assets/javascripts/templates/admin/tag.html.haml @@ -0,0 +1,8 @@ +.tag-template + %div + %span.tag-with-rules{ ng: { if: "data.rules" }, "ofn-with-tip" => "{{ 'admin.tag_has_rules' | t:{num: data.rules} }}" } + {{$getDisplayText()}} + %span{ ng: { if: "!data.rules" } } + {{$getDisplayText()}} + %a.remove-button{ ng: {click: "$removeTag()"} } + ✖ diff --git a/app/assets/javascripts/templates/admin/tag_autocomplete.html.haml b/app/assets/javascripts/templates/admin/tag_autocomplete.html.haml new file mode 100644 index 0000000000..49ff00ff27 --- /dev/null +++ b/app/assets/javascripts/templates/admin/tag_autocomplete.html.haml @@ -0,0 +1,11 @@ +.autocomplete-template + %span.tag-with-rules{ ng: { if: "data.rules" } } + {{$getDisplayText()}} + %span.tag-with-rules{ ng: { if: "data.rules == 1" } } + — + = t 'admin.has_one_rule' + %span.tag-with-rules{ ng: { if: "data.rules > 1" } } + — + = t 'admin.has_n_rules', { num: '{{data.rules}}' } + %span{ ng: { if: "!data.rules" } } + {{$getDisplayText()}} diff --git a/app/assets/javascripts/templates/admin/tags_input.html.haml b/app/assets/javascripts/templates/admin/tags_input.html.haml new file mode 100644 index 0000000000..2dd75ce5ac --- /dev/null +++ b/app/assets/javascripts/templates/admin/tags_input.html.haml @@ -0,0 +1,7 @@ +%tags-input{ template: 'admin/tag.html', ng: { model: 'object[tagsAttr]' } } + %auto-complete{source: "findTags({query: $query})", + template: "admin/tag_autocomplete.html", + "min-length" => "0", + "load-on-focus" => "true", + "load-on-empty" => "true", + "max-results-to-show" => "32"} diff --git a/app/assets/stylesheets/admin/customers.css.scss b/app/assets/stylesheets/admin/customers.css.scss new file mode 100644 index 0000000000..e3c427649c --- /dev/null +++ b/app/assets/stylesheets/admin/customers.css.scss @@ -0,0 +1,3 @@ +.tag-with-rules { + color: black; +} diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb new file mode 100644 index 0000000000..3ab5685ffe --- /dev/null +++ b/app/controllers/admin/tags_controller.rb @@ -0,0 +1,28 @@ +module Admin + class TagsController < Spree::Admin::BaseController + respond_to :json + + def index + respond_to do |format| + format.json do + serialiser = ActiveModel::ArraySerializer.new(tags_of_enterprise) + render json: serialiser.to_json + end + end + end + + private + + def enterprise + Enterprise.managed_by(spree_current_user).find_by_id(params[:enterprise_id]) + end + + def tags_of_enterprise + return [] unless enterprise + tag_rule_map = enterprise.rules_per_tag + tag_rule_map.keys.map do |tag| + { text: tag, rules: tag_rule_map[tag] } + end + end + end +end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index dfc5050a38..d3e86ff3ca 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -352,6 +352,20 @@ class Enterprise < ActiveRecord::Base end end + def rules_per_tag + tag_rule_map = {} + tag_rules.each do |rule| + rule.preferred_customer_tags.split(",").each do |tag| + if tag_rule_map[tag] + tag_rule_map[tag] += 1 + else + tag_rule_map[tag] = 1 + end + end + end + tag_rule_map + end + protected def devise_mailer diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 867d3c9e2b..e250d1341d 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -101,11 +101,6 @@ class AbilityDecorator can [:print], Spree::Order do |order| order.user == user end - - can [:create], Customer - can [:destroy], Customer do |customer| - user.enterprises.include? customer.enterprise - end end def add_product_management_abilities(user) @@ -221,7 +216,9 @@ class AbilityDecorator # Reports page can [:admin, :index, :customers, :group_buys, :bulk_coop, :sales_tax, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management, :xero_invoices], :report - can [:admin, :index, :update], Customer, enterprise_id: Enterprise.managed_by(user).pluck(:id) + can [:create], Customer + can [:admin, :index, :update, :destroy], Customer, enterprise_id: Enterprise.managed_by(user).pluck(:id) + can [:admin, :index], :tag end diff --git a/app/serializers/api/admin/customer_serializer.rb b/app/serializers/api/admin/customer_serializer.rb index 3cb9518a9f..052f49a917 100644 --- a/app/serializers/api/admin/customer_serializer.rb +++ b/app/serializers/api/admin/customer_serializer.rb @@ -6,6 +6,10 @@ class Api::Admin::CustomerSerializer < ActiveModel::Serializer end def tags - object.tag_list.map{ |t| { text: t } } + tag_rule_map = object.enterprise.rules_per_tag + object.tag_list.map do |tag| + { text: tag, rules: tag_rule_map[tag] } + end end + end diff --git a/app/views/admin/customers/index.html.haml b/app/views/admin/customers/index.html.haml index e33b871df4..0a348d4a0a 100644 --- a/app/views/admin/customers/index.html.haml +++ b/app/views/admin/customers/index.html.haml @@ -56,7 +56,7 @@ %input{ :type => 'text', :name => 'code', :id => 'code', 'ng-model' => 'customer.code', 'obj-for-update' => "customer", "attr-for-update" => "code" } %td.tags{ 'ng-show' => 'columns.tags.visible' } .tag_watcher{ 'obj-for-update' => "customer", "attr-for-update" => "tag_list"} - %tags_with_translation{ object: 'customer' } + %tags_with_translation{ object: 'customer', 'find-tags' => 'findTags(query)' } %td.actions %a{ 'ng-click' => "deleteCustomer(customer)", :class => "delete-customer icon-trash no-text" } %input{ :type => "button", 'value' => 'Update', 'ng-click' => 'submitAll()' } diff --git a/config/locales/en.yml b/config/locales/en.yml index 7b5f9b168f..e9de6c9abd 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -80,6 +80,10 @@ en: whats_this: What's this? + tag_has_rules: "Existing rules for this tag: %{num}" + has_one_rule: "has one rule" + has_n_rules: "has %{num} rules" + customers: index: add_customer: "Add customer" diff --git a/config/routes.rb b/config/routes.rb index 8d8d6d27c3..e240f901a8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -117,6 +117,8 @@ Openfoodnetwork::Application.routes.draw do resources :customers, only: [:index, :create, :update, :destroy] + resources :tags, only: [:index], format: :json + resource :content resource :accounts_and_billing_settings, only: [:edit, :update] do diff --git a/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee b/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee index 215f2834ed..6981ef4e4f 100644 --- a/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee +++ b/spec/javascripts/unit/admin/customers/controllers/customers_controller_spec.js.coffee @@ -47,3 +47,32 @@ describe "CustomersCtrl", -> http.flush() expect(scope.customers.length).toBe 1 expect(scope.customers[0]).not.toAngularEqual customer + + describe "scope.findTags", -> + tags = [ + { text: 'one' } + { text: 'two' } + { text: 'three' } + ] + beforeEach -> + http.expectGET('/admin/tags.json?enterprise_id=1').respond 200, tags + + it "retrieves the tag list", -> + promise = scope.findTags('') + result = null + promise.then (data) -> + result = data + http.flush() + expect(result).toAngularEqual tags + + it "filters the tag list", -> + filtered_tags = [ + { text: 'two' } + { text: 'three' } + ] + promise = scope.findTags('t') + result = null + promise.then (data) -> + result = data + http.flush() + expect(result).toAngularEqual filtered_tags diff --git a/spec/serializers/admin/customer_serializer_spec.rb b/spec/serializers/admin/customer_serializer_spec.rb new file mode 100644 index 0000000000..d63f00d4aa --- /dev/null +++ b/spec/serializers/admin/customer_serializer_spec.rb @@ -0,0 +1,14 @@ +describe Api::Admin::CustomerSerializer do + let(:customer) { create(:customer, tag_list: "one, two, three") } + let!(:tag_rule) { create(:tag_rule, enterprise: customer.enterprise, preferred_customer_tags: "two") } + + it "serializes a customer" do + serializer = Api::Admin::CustomerSerializer.new customer + result = JSON.parse(serializer.to_json) + expect(result['email']).to eq customer.email + tags = result['tags'] + expect(tags.length).to eq 3 + expect(tags[0]).to eq({ "text" => 'one', "rules" => nil }) + expect(tags[1]).to eq({ "text" => 'two', "rules" => 1 }) + end +end From 9ac6de4215005889d757a0cb0d1b89a47d2ee397 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 4 May 2016 11:42:07 +1000 Subject: [PATCH 22/24] Admin can set bugherd API key --- .../general_settings_controller_decorator.rb | 15 +++++++++++++ .../spree/app_configuration_decorator.rb | 3 +++ spec/features/admin/external_services_spec.rb | 22 +++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 app/controllers/spree/admin/general_settings_controller_decorator.rb create mode 100644 spec/features/admin/external_services_spec.rb diff --git a/app/controllers/spree/admin/general_settings_controller_decorator.rb b/app/controllers/spree/admin/general_settings_controller_decorator.rb new file mode 100644 index 0000000000..603f74bf63 --- /dev/null +++ b/app/controllers/spree/admin/general_settings_controller_decorator.rb @@ -0,0 +1,15 @@ +module Spree + module Admin + GeneralSettingsController.class_eval do + end + + + module GeneralSettingsEditPreferences + def edit + super + @preferences_general << :bugherd_api_key + end + end + GeneralSettingsController.send(:prepend, GeneralSettingsEditPreferences) + end +end diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb index 6ef1e7b848..c169ce0231 100644 --- a/app/models/spree/app_configuration_decorator.rb +++ b/app/models/spree/app_configuration_decorator.rb @@ -23,4 +23,7 @@ Spree::AppConfiguration.class_eval do # Monitoring preference :last_job_queue_heartbeat_at, :string, default: nil + + # External services + preference :bugherd_api_key, :string, default: nil end diff --git a/spec/features/admin/external_services_spec.rb b/spec/features/admin/external_services_spec.rb new file mode 100644 index 0000000000..ed81c8e6fa --- /dev/null +++ b/spec/features/admin/external_services_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +feature 'External services' do + include AuthenticationWorkflow + + describe "bugherd" do + before do + Spree::Config.bugherd_api_key = nil + login_to_admin_section + end + + it "lets me set an API key" do + visit spree.edit_admin_general_settings_path + + fill_in 'bugherd_api_key', with: 'abc123' + click_button 'Update' + + page.should have_content 'General Settings has been successfully updated!' + expect(Spree::Config.bugherd_api_key).to eq 'abc123' + end + end +end From a11696b85ea5dfdb682b893118a6c1fe0f66e610 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 4 May 2016 12:13:03 +1000 Subject: [PATCH 23/24] Include BugHerd script only if configured, and with configured API key --- app/views/layouts/_bugherd_script.html.haml | 14 +---- .../consumer/external_services_spec.rb | 57 +++++++++++++++++++ spec/support/request/web_helper.rb | 18 ++++++ 3 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 spec/features/consumer/external_services_spec.rb diff --git a/app/views/layouts/_bugherd_script.html.haml b/app/views/layouts/_bugherd_script.html.haml index ad6fe585f5..8a04160cfd 100644 --- a/app/views/layouts/_bugherd_script.html.haml +++ b/app/views/layouts/_bugherd_script.html.haml @@ -1,18 +1,8 @@ -- if Rails.env.staging? or Rails.env.production? +- if (Rails.env.staging? || Rails.env.production?) && Spree::Config.bugherd_api_key.present? :javascript (function (d, t) { var bh = d.createElement(t), s = d.getElementsByTagName(t)[0]; bh.type = 'text/javascript'; - bh.src = '//www.bugherd.com/sidebarv2.js?apikey=4ftxjbgwx7y6ssykayr04w'; + bh.src = '//www.bugherd.com/sidebarv2.js?apikey=#{Spree::Config.bugherd_api_key}'; s.parentNode.insertBefore(bh, s); })(document, 'script'); - - --#- elsif Rails.env.production? - -#:javascript - -#(function (d, t) { - -#var bh = d.createElement(t), s = d.getElementsByTagName(t)[0]; - -#bh.type = 'text/javascript'; - -#bh.src = '//www.bugherd.com/sidebarv2.js?apikey=xro3uv55objies58o2wrua'; - -#s.parentNode.insertBefore(bh, s); - -#})(document, 'script'); diff --git a/spec/features/consumer/external_services_spec.rb b/spec/features/consumer/external_services_spec.rb new file mode 100644 index 0000000000..28be1f8cb6 --- /dev/null +++ b/spec/features/consumer/external_services_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +feature 'External services' do + include AuthenticationWorkflow + include WebHelper + + describe "bugherd" do + describe "limiting inclusion by environment" do + before { Spree::Config.bugherd_api_key = 'abc123' } + + it "is not included in test" do + visit root_path + expect(script_content(with: 'bugherd')).to be_nil + end + + it "is not included in dev" do + Rails.env.stub(:development?) { true } + visit root_path + expect(script_content(with: 'bugherd')).to be_nil + end + + it "is included in staging" do + Rails.env.stub(:staging?) { true } + visit root_path + expect(script_content(with: 'bugherd')).not_to be_nil + end + + it "is included in production" do + Rails.env.stub(:production?) { true } + visit root_path + expect(script_content(with: 'bugherd')).not_to be_nil + end + end + + context "in an environment where BugHerd is displayed" do + before { Rails.env.stub(:staging?) { true } } + + context "when there is no API key set" do + before { Spree::Config.bugherd_api_key = nil } + + it "does not include the BugHerd script" do + visit root_path + expect(script_content(with: 'bugherd')).to be_nil + end + end + + context "when an API key is set" do + before { Spree::Config.bugherd_api_key = 'abc123' } + + it "includes the BugHerd script, with the correct API key" do + visit root_path + expect(script_content(with: 'bugherd')).to include 'abc123' + end + end + end + end +end diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index 30ce677c6c..70c803709e 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -115,6 +115,24 @@ module WebHelper DirtyFormDialog.new(page) end + # Fetch the content of a script block + # eg. script_content with: 'my-script.com' + # Returns nil if not found + # Raises an exception if multiple matching blocks are found + def script_content(opts={}) + elems = page.all('script', visible: false) + + elems = elems.to_a.select { |e| e.text(:all).include? opts[:with] } if opts[:with] + + if elems.none? + nil + elsif elems.many? + raise "Multiple results returned for script_content" + else + elems.first.text(:all) + end + end + # http://www.elabs.se/blog/53-why-wait_until-was-removed-from-capybara # Do not use this without good reason. Capybara's built-in waiting is very effective. def wait_until(secs=nil) From 3e231da472d82a6353f211b5af6c38be881649d0 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 6 May 2016 11:13:15 +1000 Subject: [PATCH 24/24] Translate subjects of enterprise emails Minor text change Fix #906 Thanks to Nicolas Blanc: https://github.com/openfoodfoundation/openfoodnetwork/pull/937 --- app/mailers/enterprise_mailer.rb | 19 +++++++++++++------ config/locales/en.yml | 5 +++++ spec/mailers/enterprise_mailer_spec.rb | 4 ++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/app/mailers/enterprise_mailer.rb b/app/mailers/enterprise_mailer.rb index 248e31f109..dfd3f21636 100644 --- a/app/mailers/enterprise_mailer.rb +++ b/app/mailers/enterprise_mailer.rb @@ -4,19 +4,26 @@ class EnterpriseMailer < Spree::BaseMailer def welcome(enterprise) @enterprise = enterprise - mail(:to => enterprise.email, :from => from_address, - :subject => "#{enterprise.name} is now on #{Spree::Config[:site_name]}") + subject = t('enterprise_mailer.welcome.subject', + enterprise: @enterprise.name, + sitename: Spree::Config[:site_name]) + mail(:to => enterprise.email, + :from => from_address, + :subject => subject) end - def confirmation_instructions(record, token, opts={}) + def confirmation_instructions(record, token) @token = token find_enterprise(record) - mail(subject: "Please confirm your email for #{@enterprise.name}", - to: ( @enterprise.unconfirmed_email || @enterprise.email ), - from: from_address) + subject = t('enterprise_mailer.confirmation_instructions.subject', + enterprise: @enterprise.name) + mail(to: (@enterprise.unconfirmed_email || @enterprise.email), + from: from_address, + subject: subject) end private + def find_enterprise(enterprise) @enterprise = enterprise.is_a?(Enterprise) ? enterprise : Enterprise.find(enterprise) end diff --git a/config/locales/en.yml b/config/locales/en.yml index e9de6c9abd..b1595ba117 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -32,6 +32,11 @@ en: not_confirmed: Your email address could not be confirmed. Perhaps you have already completed this step? confirmation_sent: "Confirmation email sent!" confirmation_not_sent: "Could not send a confirmation email." + enterprise_mailer: + confirmation_instructions: + subject: "Please confirm the email address for %{enterprise}" + welcome: + subject: "%{enterprise} is now on %{sitename}" home: "OFN" title: Open Food Network welcome_to: 'Welcome to ' diff --git a/spec/mailers/enterprise_mailer_spec.rb b/spec/mailers/enterprise_mailer_spec.rb index eaa21b54ae..2399d4146a 100644 --- a/spec/mailers/enterprise_mailer_spec.rb +++ b/spec/mailers/enterprise_mailer_spec.rb @@ -12,7 +12,7 @@ describe EnterpriseMailer do EnterpriseMailer.confirmation_instructions(enterprise, 'token').deliver ActionMailer::Base.deliveries.count.should == 1 mail = ActionMailer::Base.deliveries.first - expect(mail.subject).to eq "Please confirm your email for #{enterprise.name}" + expect(mail.subject).to eq "Please confirm the email address for #{enterprise.name}" expect(mail.to).to include enterprise.email expect(mail.reply_to).to be_nil end @@ -28,7 +28,7 @@ describe EnterpriseMailer do EnterpriseMailer.confirmation_instructions(enterprise, 'token').deliver ActionMailer::Base.deliveries.count.should == 1 mail = ActionMailer::Base.deliveries.first - expect(mail.subject).to eq "Please confirm your email for #{enterprise.name}" + expect(mail.subject).to eq "Please confirm the email address for #{enterprise.name}" expect(mail.to).to include enterprise.unconfirmed_email end end