From 6bbd3f7c13182f3eb1918539113c3b5c1f5699ce Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sun, 26 Apr 2015 11:02:06 +0100 Subject: [PATCH 01/54] Added auth for order_cycle_management_report. This report was breaking supplier enterprises reports due to incorrect authorization. --- app/models/spree/ability_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 9bff908346..dbb0d4fe10 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -112,7 +112,7 @@ class AbilityDecorator can [:admin, :index, :read, :create, :edit], Spree::Classification # Reports page - can [:admin, :index, :customers, :orders_and_distributors, :group_buys, :bulk_coop, :payments, :orders_and_fulfillment, :products_and_inventory], :report + can [:admin, :index, :customers, :orders_and_distributors, :group_buys, :bulk_coop, :payments, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management], :report end def add_order_management_abilities(user) From 9e61a7d083da2134a44cbf56e3a01ee00f677fd7 Mon Sep 17 00:00:00 2001 From: Lynne Davis Date: Sun, 26 Apr 2015 11:03:32 +0100 Subject: [PATCH 02/54] Adding report type drop down to order_cycle_management_report --- .../spree/admin/reports/order_cycle_management.html.haml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/views/spree/admin/reports/order_cycle_management.html.haml b/app/views/spree/admin/reports/order_cycle_management.html.haml index fbe365c742..418cba8b75 100644 --- a/app/views/spree/admin/reports/order_cycle_management.html.haml +++ b/app/views/spree/admin/reports/order_cycle_management.html.haml @@ -20,6 +20,10 @@ include_blank: true) %br %br + = label_tag nil, "Report Type: " + = select_tag(:report_type, options_for_select(@report_types, @report_type)) + %br + %br = check_box_tag :csv = label_tag :csv, "Download as csv" %br @@ -41,4 +45,3 @@ - if @report.table.empty? %tr %td{:colspan => "2"}= t(:none) - From b09ae550c8773212f14f4c3920eedf08a4a81b55 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 30 Apr 2015 10:52:29 +1000 Subject: [PATCH 03/54] Add spec for payment actions --- spec/models/spree/payment_spec.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 spec/models/spree/payment_spec.rb diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb new file mode 100644 index 0000000000..0962bdbf98 --- /dev/null +++ b/spec/models/spree/payment_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +module Spree + describe Payment do + describe "available actions" do + let(:payment) { create(:payment, source: create(:credit_card)) } + + context "for most gateways" do + it "can capture and void" do + payment.actions.sort.should == %w(capture void).sort + end + + describe "when a payment has been taken" do + before do + payment.stub(:state) { 'completed' } + payment.stub(:order) { double(:order, payment_state: 'credit_owed') } + end + + it "can void and credit" do + payment.actions.sort.should == %w(void credit).sort + end + end + end + end + end +end From 8184a7c7b2f4d9d9b4ae726eb65dce16321073c9 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 30 Apr 2015 13:45:44 +1000 Subject: [PATCH 04/54] Pin payments can't void or credit, but they can refund --- app/models/spree/payment_decorator.rb | 15 +++++++++++++++ spec/models/spree/payment_spec.rb | 26 ++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 app/models/spree/payment_decorator.rb diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb new file mode 100644 index 0000000000..5095ae57e9 --- /dev/null +++ b/app/models/spree/payment_decorator.rb @@ -0,0 +1,15 @@ +module Spree + Payment.class_eval do + # Pin payments lacks void and credit methods, but it does have refund + # Here we swap credit out for refund and remove void as a possible action + def actions_with_pin_payment_adaptations + actions = actions_without_pin_payment_adaptations + if payment_method.is_a? Gateway::Pin + actions << 'refund' if actions.include? 'credit' + actions.reject! { |a| ['credit', 'void'].include? a } + end + actions + end + alias_method_chain :actions, :pin_payment_adaptations + end +end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 0962bdbf98..04cf7b4c6f 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' module Spree describe Payment do describe "available actions" do - let(:payment) { create(:payment, source: create(:credit_card)) } - context "for most gateways" do + let(:payment) { create(:payment, source: create(:credit_card)) } + it "can capture and void" do payment.actions.sort.should == %w(capture void).sort end @@ -21,6 +21,28 @@ module Spree end end end + + context "for Pin Payments" do + let(:d) { create(:distributor_enterprise) } + let(:pin) { Gateway::Pin.create! name: 'pin', distributor_ids: [d.id]} + let(:payment) { create(:payment, source: create(:credit_card), payment_method: pin) } + + it "does not void" do + payment.actions.should_not include 'void' + end + + describe "when a payment has been taken" do + before do + payment.stub(:state) { 'completed' } + payment.stub(:order) { double(:order, payment_state: 'credit_owed') } + end + + it "can refund instead of crediting" do + payment.actions.should_not include 'credit' + payment.actions.should include 'refund' + end + end + end end end end From b498c2863292dbbb57818355f836b67c69bc1335 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 30 Apr 2015 15:38:01 +1000 Subject: [PATCH 05/54] Payments can be refunded --- app/models/spree/payment_decorator.rb | 37 +++++++++++++ spec/models/spree/payment_spec.rb | 80 +++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/app/models/spree/payment_decorator.rb b/app/models/spree/payment_decorator.rb index 5095ae57e9..1a80f0269f 100644 --- a/app/models/spree/payment_decorator.rb +++ b/app/models/spree/payment_decorator.rb @@ -11,5 +11,42 @@ module Spree actions end alias_method_chain :actions, :pin_payment_adaptations + + + def refund!(refund_amount=nil) + protect_from_connection_error do + check_environment + + refund_amount = calculate_refund_amount(refund_amount) + + if payment_method.payment_profiles_supported? + response = payment_method.refund((refund_amount * 100).round, source, response_code, gateway_options) + else + response = payment_method.refund((refund_amount * 100).round, response_code, gateway_options) + end + + record_response(response) + + if response.success? + self.class.create({ :order => order, + :source => self, + :payment_method => payment_method, + :amount => refund_amount.abs * -1, + :response_code => response.authorization, + :state => 'completed' }, :without_protection => true) + else + gateway_error(response) + end + end + end + + + private + + def calculate_refund_amount(refund_amount=nil) + refund_amount ||= credit_allowed >= order.outstanding_balance.abs ? order.outstanding_balance.abs : credit_allowed.abs + refund_amount.to_f + end + end end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 04cf7b4c6f..8a781e7700 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -44,5 +44,85 @@ module Spree end end end + + describe "refunding" do + let(:payment) { create(:payment) } + let(:success) { double(:success? => true, authorization: 'abc123') } + let(:failure) { double(:success? => false) } + + it "always checks the environment" do + payment.payment_method.stub(:refund) { success } + payment.should_receive(:check_environment) + payment.refund! + end + + describe "calculating refund amount" do + it "returns the parameter amount when given" do + payment.send(:calculate_refund_amount, 123).should === 123.0 + end + + it "refunds up to the value of the payment when the outstanding balance is larger" do + payment.stub(:credit_allowed) { 123 } + payment.stub(:order) { double(:order, outstanding_balance: 1000) } + payment.send(:calculate_refund_amount).should == 123 + end + + it "refunds up to the outstanding balance of the order when the payment is larger" do + payment.stub(:credit_allowed) { 1000 } + payment.stub(:order) { double(:order, outstanding_balance: 123) } + payment.send(:calculate_refund_amount).should == 123 + end + end + + describe "performing refunds" do + before do + payment.stub(:calculate_refund_amount) { 123 } + payment.payment_method.should_receive(:refund).and_return(success) + end + + it "performs the refund without payment profiles" do + payment.payment_method.stub(:payment_profiles_supported?) { false } + payment.refund! + end + + it "performs the refund with payment profiles" do + payment.payment_method.stub(:payment_profiles_supported?) { true } + payment.refund! + end + end + + it "records the response" do + payment.stub(:calculate_refund_amount) { 123 } + payment.payment_method.stub(:refund).and_return(success) + payment.should_receive(:record_response).with(success) + payment.refund! + end + + it "records a payment on success" do + payment.stub(:calculate_refund_amount) { 123 } + payment.payment_method.stub(:refund).and_return(success) + payment.stub(:record_response) + + expect do + payment.refund! + end.to change(Payment, :count).by(1) + + p = Payment.last + p.order.should == payment.order + p.source.should == payment + p.payment_method.should == payment.payment_method + p.amount.should == -123 + p.response_code.should == success.authorization + p.state.should == 'completed' + end + + it "logs the error on failure" do + payment.stub(:calculate_refund_amount) { 123 } + payment.payment_method.stub(:refund).and_return(failure) + payment.stub(:record_response) + payment.should_receive(:gateway_error).with(failure) + payment.refund! + end + end end end From 0b652a2113fe69787a569874ff3bc43ed3a63c10 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 30 Apr 2015 15:38:12 +1000 Subject: [PATCH 06/54] Add refund icon --- app/assets/stylesheets/admin/icons.css.scss | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 app/assets/stylesheets/admin/icons.css.scss diff --git a/app/assets/stylesheets/admin/icons.css.scss b/app/assets/stylesheets/admin/icons.css.scss new file mode 100644 index 0000000000..e7737ce8e9 --- /dev/null +++ b/app/assets/stylesheets/admin/icons.css.scss @@ -0,0 +1,3 @@ +@import 'plugins/font-awesome'; + +.icon-refund:before { @extend .icon-ok:before } From 2c7a5c06565a638a5c7d9053ead00fc97303c005 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 30 Apr 2015 15:38:17 +1000 Subject: [PATCH 07/54] Update Spree - fixes bug where Payment#method_missing depends on #provider already called. --- Gemfile.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 17dbfdf8c5..1b62201120 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,7 +23,7 @@ GIT GIT remote: git://github.com/openfoodfoundation/spree.git - revision: 4e0075b07acb56864aca89eee3d9670136176c23 + revision: afcc23e489eb604a3e2651598a7c8364e2acc7b3 branch: 1-3-stable specs: spree (1.3.6.beta) @@ -123,7 +123,7 @@ GEM active_link_to (1.0.0) active_model_serializers (0.8.1) activemodel (>= 3.0) - activemerchant (1.46.0) + activemerchant (1.48.0) activesupport (>= 3.2.14, < 5.0.0) builder (>= 2.1.2, < 4.0.0) i18n (>= 0.6.9) @@ -181,7 +181,7 @@ GEM climate_control (0.0.3) activesupport (>= 3.0) cliver (0.3.2) - cocaine (0.5.5) + cocaine (0.5.7) climate_control (>= 0.0.3, < 1.0) coderay (1.0.9) coffee-rails (3.2.2) @@ -191,7 +191,7 @@ GEM coffee-script-source execjs coffee-script-source (1.3.3) - colorize (0.7.5) + colorize (0.7.7) columnize (0.3.6) comfortable_mexican_sofa (1.6.24) active_link_to (~> 1.0.0) @@ -496,7 +496,7 @@ GEM sprockets (>= 2.0.0) turn (0.8.3) ansi - tzinfo (0.3.43) + tzinfo (0.3.44) uglifier (1.2.4) execjs (>= 0.3.0) multi_json (>= 1.0.2) From b7bac326bd792932349042398c817f6defccfb1b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 May 2015 12:05:43 +1000 Subject: [PATCH 08/54] admin order edit: re-label update button to "update and recalculate fees" --- .../admin/orders/_form/relabel_update_button.html.haml.deface | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface diff --git a/app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface b/app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface new file mode 100644 index 0000000000..d4d3aefc8d --- /dev/null +++ b/app/overrides/spree/admin/orders/_form/relabel_update_button.html.haml.deface @@ -0,0 +1,3 @@ +/ replace "code[erb-loud]:contains('button t(:update)')" + += button t(:update_and_recalculate_fees), 'icon-refresh' From e6e063670cd90522a0bd57bb3af2167e1007c06a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 May 2015 14:49:34 +1000 Subject: [PATCH 09/54] Allow managers to remove line items from order Managers of an order cycle and the distributor of an order are allowed to remove a line item from the order. --- .../spree/admin/line_items_controller_decorator.rb | 8 ++++++++ app/models/spree/ability_decorator.rb | 3 +++ 2 files changed, 11 insertions(+) create mode 100644 app/controllers/spree/admin/line_items_controller_decorator.rb diff --git a/app/controllers/spree/admin/line_items_controller_decorator.rb b/app/controllers/spree/admin/line_items_controller_decorator.rb new file mode 100644 index 0000000000..ca83baa00b --- /dev/null +++ b/app/controllers/spree/admin/line_items_controller_decorator.rb @@ -0,0 +1,8 @@ +Spree::Admin::LineItemsController.class_eval do + private + + def load_order + @order = Spree::Order.find_by_number!(params[:order_id]) + authorize! :update, @order + end +end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index e66c03aa72..c6126d7171 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -144,6 +144,9 @@ class AbilityDecorator end can [:admin, :bulk_management], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor) can [:admin, :create], Spree::LineItem + can [:destroy], Spree::LineItem do |item| + user.admin? || user.enterprises.include?(order.distributor) || user == order.order_cycle.manager + end can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::Payment can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::Shipment From 11acb3ba59e647cd0457564af0bd701a787db582 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 May 2015 15:59:46 +1000 Subject: [PATCH 10/54] Allow to remove adjustments Managers of an order cycle and the distributor of an order are allowed to remove an adjustment from the order. --- app/models/spree/ability_decorator.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index c6126d7171..b8539979c2 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -152,6 +152,19 @@ class AbilityDecorator can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::Shipment can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::Adjustment can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::ReturnAuthorization + can [:destroy], Spree::Adjustment do |adjustment| + # Sharing code with destroying a line item. This should be unified and probably applied for other actions as well. + binding.pry + if user.admin? + true + elsif adjustment.adjustable.instance_of? Spree::Order + order = adjustment.adjustable + user.enterprises.include?(order.distributor) || user == order.order_cycle.manager + elsif adjustment.adjustable.instance_of? Spree::LineItem + order = adjustment.adjustable.order + user.enterprises.include?(order.distributor) || user == order.order_cycle.manager + end + end can [:create], OrderCycle From a3664d44483909916dfa2b40fb8e516db739eed8 Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 3 May 2015 12:51:59 +1000 Subject: [PATCH 11/54] Added bindonce and ng-if improvements to Producers templates --- app/views/producers/_fat.html.haml | 56 +++++++++++++-------------- app/views/producers/_skinny.html.haml | 12 +++--- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/app/views/producers/_fat.html.haml b/app/views/producers/_fat.html.haml index 17304abbe3..a6d6661501 100644 --- a/app/views/producers/_fat.html.haml +++ b/app/views/producers/_fat.html.haml @@ -1,68 +1,68 @@ -.row.active_table_row{"ng-show" => "open()", "ng-click" => "toggle()", "ng-class" => "{'open' : !ofn-i_032-closed-sign()}"} +.row.active_table_row{"ng-if" => "open()", "ng-click" => "toggle()", "ng-class" => "{'open' : !ofn-i_032-closed-sign()}"} .columns.small-12.medium-7.large-7.fat / Will add in long description available once clean up HTML formatting producer.long_description %div{"bo-if" => "producer.description"} %label About us - %img.right.show-for-medium-up{"ng-src" => "{{producer.logo}}" } - %p.text-small - {{ producer.description }} + %img.right.show-for-medium-up{"bo-src" => "producer.logo" } + %p.text-small{ "bo-text" => "producer.description"} %div.show-for-medium-up{"bo-if" => "producer.description.length==0"} %label   .columns.small-12.medium-5.large-5.fat - %div{"ng-if" => "producer.supplied_taxons"} + %div{"bo-if" => "producer.supplied_taxons"} %label Shop for %p.trans-sentence %span.fat-taxons{"ng-repeat" => "taxon in producer.supplied_taxons"} %render-svg{path: "{{taxon.icon}}"} - {{taxon.name}} + %span{"bo-text" => "taxon.name"} %div.show-for-medium-up{"ng-if" => "producer.supplied_taxons.length==0"}   - %div{"ng-if" => "producer.email || producer.website || producer.phone"} + %div{"bo-if" => "producer.email || producer.website || producer.phone"} %label Contact - %p.word-wrap{"ng-if" => "producer.phone"} - Call {{ producer.phone }} + %p.word-wrap{"bo-if" => "producer.phone"} + Call + %span{"bo-text" => "producer.phone"} - %p.word-wrap{"ng-if" => "producer.email"} - %a{"ng-href" => "{{producer.email | stripUrl}}", target: "_blank", mailto: true} - %span.email {{ producer.email | stripUrl }} + %p.word-wrap{"bo-if" => "producer.email"} + %a{"bo-href" => "producer.email | stripUrl", target: "_blank", mailto: true} + %span.email{"bo-bind" => "producer.email | stripUrl"} - %p.word-wrap{"ng-if" => "producer.website"} - %a{"ng-href" => "http://{{producer.website | stripUrl}}", target: "_blank" } - %span {{ producer.website | stripUrl }} + %p.word-wrap{"bo-if" => "producer.website"} + %a{"bo-href-i" => "http://{{producer.website | stripUrl}}", target: "_blank" } + %span{"bo-bind" => "producer.website | stripUrl"} - %div{"ng-if" => "producer.twitter || producer.facebook || producer.linkedin || producer.instagram"} + %div{"bo-if" => "producer.twitter || producer.facebook || producer.linkedin || producer.instagram"} %label Follow .follow-icons{bindonce: true} - %span{"ng-if" => "producer.twitter"} - %a{"ng-href" => "http://twitter.com/{{producer.twitter}}", target: "_blank"} + %span{"bo-if" => "producer.twitter"} + %a{"bo-href-i" => "http://twitter.com/{{producer.twitter}}", target: "_blank"} %i.ofn-i_041-twitter - %span{"ng-if" => "producer.facebook"} - %a{"ng-href" => "http://{{producer.facebook | stripUrl}}", target: "_blank"} + %span{"bo-if" => "producer.facebook"} + %a{"bo-href-i" => "http://{{producer.facebook | stripUrl}}", target: "_blank"} %i.ofn-i_044-facebook - %span{"ng-if" => "producer.linkedin"} - %a{"ng-href" => "http://{{producer.linkedin | stripUrl}}", target: "_blank"} + %span{"bo-if" => "producer.linkedin"} + %a{"bo-href-i" => "http://{{producer.linkedin | stripUrl}}", target: "_blank"} %i.ofn-i_042-linkedin - %span{"ng-if" => "producer.instagram"} - %a{"ng-href" => "http://instagram.com/{{producer.instagram}}", target: "_blank"} + %span{"bo-if" => "producer.instagram"} + %a{"bo-href-i" => "http://instagram.com/{{producer.instagram}}", target: "_blank"} %i.ofn-i_043-instagram -.row.active_table_row.pad-top{"ng-show" => "open()", "bo-if" => "producer.hubs"} +.row.active_table_row.pad-top{"ng-if" => "open()", "bo-if" => "producer.hubs"} .columns.small-12 .row .columns.small-12.fat %div{"bo-if" => "producer.name"} %label Shop for - %span.turquoise {{ producer.name }} + %span.turquoise{"bo-text" => "producer.name"} products at: %div.show-for-medium-up{"bo-if" => "!producer.name"}   @@ -73,6 +73,6 @@ "bo-class" => "{primary: hub.active, secondary: !hub.active}"} %i.ofn-i_033-open-sign{"bo-if" => "hub.active"} %i.ofn-i_032-closed-sign{"bo-if" => "!hub.active"} - .hub-name {{hub.name}} - .button-address {{ [hub.address.city, hub.address.state_name] | printArray }} + .hub-name{"bo-text" => "hub.name"} + .button-address{"bo-bind" => "[hub.address.city, hub.address.state_name] | printArray"} diff --git a/app/views/producers/_skinny.html.haml b/app/views/producers/_skinny.html.haml index aaa59b20e8..49ab0a35be 100644 --- a/app/views/producers/_skinny.html.haml +++ b/app/views/producers/_skinny.html.haml @@ -2,19 +2,19 @@ .columns.small-12.medium-4.large-4.skinny-head %span{"bo-if" => "producer.is_distributor" } %a.is_distributor{"bo-href" => "producer.path" } - %i{ng: {class: "producer.producer_icon_font"}} + %i{bo: {class: "producer.producer_icon_font"}} %span.margin-top - %strong {{ producer.name }} + %strong{"bo-text" => "producer.name"} %span.producer-name{"bo-if" => "!producer.is_distributor" } - %i{ng: {class: "producer.producer_icon_font"}} + %i{bo: {class: "producer.producer_icon_font"}} %span.margin-top - %strong {{ producer.name }} + %strong{"bo-text" => "producer.name"} .columns.small-6.medium-3.large-3 - %span.margin-top {{ producer.address.city }} + %span.margin-top{"bo-text" => "producer.address.city"} .columns.small-4.medium-3.large-4 - %span.margin-top {{ producer.address.state_name | uppercase }} + %span.margin-top{"bo-bind" => "producer.address.state_name | uppercase"} .columns.small-2.medium-2.large-1.text-right %span.margin-top %i{"ng-class" => "{'ofn-i_005-caret-down' : !open(), 'ofn-i_006-caret-up' : open()}"} From 16e9f0545ba52e7ac6d8caf015f636d30680a491 Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 3 May 2015 14:02:33 +1000 Subject: [PATCH 12/54] bind-once in products --- app/views/shop/products/_summary.html.haml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/views/shop/products/_summary.html.haml b/app/views/shop/products/_summary.html.haml index 392d7ef31d..dec398f80f 100644 --- a/app/views/shop/products/_summary.html.haml +++ b/app/views/shop/products/_summary.html.haml @@ -7,14 +7,13 @@ .small-10.medium-10.large-11.columns.summary-header %h3 %a{"ng-click" => "triggerProductModal()"} - {{ product.name }} + %span{"bo-text" => "product.name"} %i.ofn-i_057-expand %small %em from %span %enterprise-modal - %i.ofn-i_036-producers - {{ enterprise.name }} + %i.ofn-i_036-producers{"bo-text" => "enterprise.name"} .small-2.medium-2.large-1.columns.text-center .taxon-flag %render-svg{path: "{{product.primary_taxon.icon}}"} From 3fc616cdff059a6beb5d8e4bc8add848dc83099b Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 3 May 2015 14:11:09 +1000 Subject: [PATCH 13/54] bind-once on hubs --- app/views/home/_fat.html.haml | 5 ++--- app/views/home/_skinny.html.haml | 17 +++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/views/home/_fat.html.haml b/app/views/home/_fat.html.haml index b3741e5aec..6796601637 100644 --- a/app/views/home/_fat.html.haml +++ b/app/views/home/_fat.html.haml @@ -5,7 +5,7 @@ .trans-sentence %span.fat-taxons{"ng-repeat" => "taxon in hub.taxons"} %render-svg{path: "{{taxon.icon}}"} - {{taxon.name}} + %span{"bo-text" => "taxon.name"} %div.show-for-medium-up{"bo-if" => "hub.taxons.length==0"}   .columns.small-12.medium-3.large-2.fat @@ -25,7 +25,6 @@ %li{"ng-repeat" => "enterprise in hub.producers"} %enterprise-modal %i.ofn-i_036-producers - %span - {{ enterprise.name }} + %span{"bo-text" => "enterprise.name"} %div.show-for-medium-up{"bo-if" => "hub.producers.length==0"}   diff --git a/app/views/home/_skinny.html.haml b/app/views/home/_skinny.html.haml index aeaae0cae4..cf001371f7 100644 --- a/app/views/home/_skinny.html.haml +++ b/app/views/home/_skinny.html.haml @@ -2,20 +2,21 @@ .columns.small-12.medium-6.large-5.skinny-head %a.hub{"bo-href" => "hub.path", "ng-class" => "{primary: hub.active, secondary: !hub.active}", "ofn-empties-cart" => "hub"} - %i{ng: {class: "hub.icon_font"}} - %span.margin-top.hub-name-listing {{ hub.name | truncate:40}} + %i{bo: {class: "hub.icon_font"}} + %span.margin-top.hub-name-listing{"bo-bind" => "hub.name | truncate:40"} .columns.small-4.medium-2.large-2 - %span.margin-top {{ hub.address.city }} + %span.margin-top{"bo-text" => "hub.address.city"} .columns.small-2.medium-1.large-1 - %span.margin-top {{ hub.address.state_name | uppercase }} + %span.margin-top{"bo-bind" => "hub.address.state_name | uppercase"} .columns.small-6.medium-3.large-4.text-right{"bo-if" => "hub.active"} %a.hub.open_closed{"bo-href" => "hub.path", "ng-class" => "{primary: hub.active, secondary: !hub.active}", "ofn-empties-cart" => "hub"} %i.ofn-i_033-open-sign %span.margin-top{ bo: { if: "current()" } } %em Shopping here - %span.margin-top{ bo: { if: "!current()" } } {{ hub.orders_close_at | sensible_timeframe }} + %span.margin-top{ bo: { if: "!current()" } } + %span{"bo-bind" => "hub.orders_close_at | sensible_timeframe"} .columns.small-6.medium-3.large-4.text-right{"bo-if" => "!hub.active"} %a.hub.open_closed{"bo-href" => "hub.path", "ng-class" => "{primary: hub.active, secondary: !hub.active}", "ofn-empties-cart" => "hub"} @@ -28,12 +29,12 @@ .columns.small-12.medium-6.large-5.skinny-head %a.hub{"ng-click" => "openModal(hub)", "ng-class" => "{primary: hub.active, secondary: !hub.active}"} %i{ng: {class: "hub.icon_font"}} - %span.margin-top.hub-name-listing {{ hub.name | truncate:40}} + %span.margin-top.hub-name-listing{"bo-bind" => "hub.name | truncate:40"} .columns.small-4.medium-2.large-2 - %span.margin-top {{ hub.address.city }} + %span.margin-top{"bo-text" => "hub.address.city"} .columns.small-2.medium-1.large-1 - %span.margin-top {{ hub.address.state_name | uppercase }} + %span.margin-top{"bo-bind" => "hub.address.state_name | uppercase"} .columns.small-6.medium-3.large-4.text-right %span.margin-top{ bo: { if: "!current()" } } From 9c26b3ebb24a5f43c6a288f7b876f1a8fb8bec25 Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 3 May 2015 14:34:42 +1000 Subject: [PATCH 14/54] bind-once in partials --- .../templates/partials/contact.html.haml | 11 ++++------ .../partials/enterprise_header.html.haml | 12 +++++------ .../templates/partials/follow.html.haml | 20 +++++++++---------- .../templates/partials/hub_actions.html.haml | 6 +++--- .../templates/partials/hub_details.html.haml | 4 ++-- 5 files changed, 25 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/templates/partials/contact.html.haml b/app/assets/javascripts/templates/partials/contact.html.haml index caa3bc5e65..165fd69d80 100644 --- a/app/assets/javascripts/templates/partials/contact.html.haml +++ b/app/assets/javascripts/templates/partials/contact.html.haml @@ -1,14 +1,11 @@ %div.contact-container{bindonce: true} %div.modal-centered{"bo-if" => "enterprise.email || enterprise.website || enterprise.phone"} %p.modal-header Contact - %p{"ng-if" => "enterprise.phone"} - {{ enterprise.phone }} + %p{"bo-if" => "enterprise.phone", "bo-text" => "enterprise.phone"} %p.word-wrap{"ng-if" => "enterprise.email"} - %a{"ng-href" => "{{enterprise.email | stripUrl}}", target: "_blank", mailto: true} - %span.email - {{ enterprise.email | stripUrl }} + %a{"bo-href" => "enterprise.email | stripUrl", target: "_blank", mailto: true} + %span.email{"bo-bind" => "enterprise.email | stripUrl"} %p.word-wrap{"ng-if" => "enterprise.website"} - %a{"ng-href" => "http://{{enterprise.website | stripUrl}}", target: "_blank" } - {{ enterprise.website | stripUrl }} + %a{"bo-href-i" => "http://{{enterprise.website | stripUrl}}", target: "_blank", "bo-bind" => "enterprise.website | stripUrl"} diff --git a/app/assets/javascripts/templates/partials/enterprise_header.html.haml b/app/assets/javascripts/templates/partials/enterprise_header.html.haml index 27a57ed90b..c0d9d5d9e0 100644 --- a/app/assets/javascripts/templates/partials/enterprise_header.html.haml +++ b/app/assets/javascripts/templates/partials/enterprise_header.html.haml @@ -1,13 +1,13 @@ -.highlight{"ng-class" => "{'is_distributor' : enterprise.is_distributor}"} +.highlight{bindonce: true, "ng-class" => "{'is_distributor' : enterprise.is_distributor}"} .highlight-top.row .small-12.medium-7.large-8.columns %h3{"ng-if" => "enterprise.is_distributor"} - %a{"bo-href" => "enterprise.path", "ofn-empties-cart" => "enterprise", bindonce: true} + %a{"bo-href" => "enterprise.path", "ofn-empties-cart" => "enterprise"} %i{"ng-class" => "enterprise.icon_font"} - %span {{ enterprise.name }} + %span{"bo-text" => "enterprise.name"} %h3{"ng-if" => "!enterprise.is_distributor", "ng-class" => "{'is_producer' : enterprise.is_primary_producer}"} %i{"ng-class" => "enterprise.icon_font"} - %span {{ enterprise.name }} + %span{"bo-text" => "enterprise.name"} .small-12.medium-5.large-4.columns.text-right.small-only-text-left - %p {{ [enterprise.address.city, enterprise.address.state_name] | printArray}} - %img.hero-img{"ng-src" => "{{enterprise.promo_image}}"} + %p{"bo-bind" => "[enterprise.address.city, enterprise.address.state_name] | printArray"} + %img.hero-img{"bo-src" => "enterprise.promo_image"} diff --git a/app/assets/javascripts/templates/partials/follow.html.haml b/app/assets/javascripts/templates/partials/follow.html.haml index 1f7bd5702b..4e2a00086a 100644 --- a/app/assets/javascripts/templates/partials/follow.html.haml +++ b/app/assets/javascripts/templates/partials/follow.html.haml @@ -1,19 +1,19 @@ -%div.modal-centered{"ng-if" => "enterprise.twitter || enterprise.facebook || enterprise.linkedin || enterprise.instagram"} +%div.modal-centered{bindonce: true, "bo-if" => "enterprise.twitter || enterprise.facebook || enterprise.linkedin || enterprise.instagram"} %p.modal-header Follow - .follow-icons{bindonce: true} - %span{"ng-if" => "enterprise.twitter"} - %a{"ng-href" => "http://twitter.com/{{enterprise.twitter}}", target: "_blank"} + .follow-icons + %span{"bo-if" => "enterprise.twitter"} + %a{"bo-href-i" => "http://twitter.com/{{enterprise.twitter}}", target: "_blank"} %i.ofn-i_041-twitter - %span{"ng-if" => "enterprise.facebook"} - %a{"ng-href" => "http://{{enterprise.facebook | stripUrl}}", target: "_blank"} + %span{"bo-if" => "enterprise.facebook"} + %a{"bo-href-i" => "http://{{enterprise.facebook | stripUrl}}", target: "_blank"} %i.ofn-i_044-facebook - %span{"ng-if" => "enterprise.linkedin"} - %a{"ng-href" => "http://{{enterprise.linkedin | stripUrl}}", target: "_blank"} + %span{"bo-if" => "enterprise.linkedin"} + %a{"bo-href-i" => "http://{{enterprise.linkedin | stripUrl}}", target: "_blank"} %i.ofn-i_042-linkedin - %span{"ng-if" => "enterprise.instagram"} - %a{"ng-href" => "http://instagram.com/{{enterprise.instagram}}", target: "_blank"} + %span{"bo-if" => "enterprise.instagram"} + %a{"bo-href-i" => "http://instagram.com/{{enterprise.instagram}}", target: "_blank"} %i.ofn-i_043-instagram diff --git a/app/assets/javascripts/templates/partials/hub_actions.html.haml b/app/assets/javascripts/templates/partials/hub_actions.html.haml index 61e74afb42..f5b451ef3c 100644 --- a/app/assets/javascripts/templates/partials/hub_actions.html.haml +++ b/app/assets/javascripts/templates/partials/hub_actions.html.haml @@ -2,7 +2,7 @@ .cta-container.small-12.columns %label Shop for - %strong {{enterprise.name}} + %strong{"bo-text" => "enterprise.name"} products at: %a.cta-hub{"ng-repeat" => "hub in enterprise.hubs", "bo-href" => "hub.path", @@ -10,7 +10,7 @@ "ofn-empties-cart" => "hub"} %i.ofn-i_033-open-sign{"bo-if" => "hub.active"} %i.ofn-i_032-closed-sign{"bo-if" => "!hub.active"} - .hub-name {{hub.name}} - .button-address {{ hub.address.city }} , {{hub.address.state_name}} + .hub-name{"bo-text" => "hub.name"} + .button-address{"bo-bind" => "[hub.address.city, hub.address.state_name] | printArray"} / %i.ofn-i_007-caret-right diff --git a/app/assets/javascripts/templates/partials/hub_details.html.haml b/app/assets/javascripts/templates/partials/hub_details.html.haml index 815200821a..8be5ceddac 100644 --- a/app/assets/javascripts/templates/partials/hub_details.html.haml +++ b/app/assets/javascripts/templates/partials/hub_details.html.haml @@ -19,6 +19,6 @@ "ofn-empties-cart" => "enterprise"} %i.ofn-i_033-open-sign{"bo-if" => "enterprise.active"} %i.ofn-i_032-closed-sign{"bo-if" => "!enterprise.active"} - .hub-name {{enterprise.name}} - .button-address {{ enterprise.address.city }} , {{enterprise.address.state_name}} + .hub-name{"bo-text" => "enterprise.name"} + .button-address{"bo-bind" => "[enterprise.address.city, enterprise.address.state_name] | printArray"} / %i.ofn-i_007-caret-right From 8fb11defdb1689415ed87305a48c5dc78b40ed42 Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 3 May 2015 14:41:34 +1000 Subject: [PATCH 15/54] bind-once in groups home page --- app/views/groups/index.html.haml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/views/groups/index.html.haml b/app/views/groups/index.html.haml index 262e188e3c..901cb17ad9 100644 --- a/app/views/groups/index.html.haml +++ b/app/views/groups/index.html.haml @@ -20,13 +20,11 @@ .row.pad-top{bindonce: true} .small-12.medium-6.columns .groups-header - %a{"ng-href" => "/groups/{{group.id}}"} + %a{"bo-href-i" => "/groups/{{group.id}}"} %i.ofn-i_035-groups - %span.group-name - {{ group.name }} + %span.group-name{"bo-text" => "group.name"} .small-3.medium-2.columns - %p - {{ group.state }} + %p{"bo-text" => "group.state"} .small-9.medium-4.columns.groups-icons %p %link-to-service.ofn-i_050-mail-circle{service: '""', ref: 'group.email.split("").reverse().join("")', mailto: true} From 9e70c80d1d29a5088755e75b0b6d7297a58b3262 Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 3 May 2015 14:48:27 +1000 Subject: [PATCH 16/54] bind-once in product modal --- .../javascripts/templates/product_modal.html.haml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/templates/product_modal.html.haml b/app/assets/javascripts/templates/product_modal.html.haml index bf66bd4aea..3db8035acc 100644 --- a/app/assets/javascripts/templates/product_modal.html.haml +++ b/app/assets/javascripts/templates/product_modal.html.haml @@ -1,10 +1,10 @@ -.row +.row{bindonce: true} .columns.small-12.large-6.product-header - %h3 {{product.name}} + %h3{"bo-text" => "product.name"} %span %em from - %span.avenir {{ enterprise.name }} + %span.avenir{"bo-text" => "enterprise.name"} %br @@ -18,11 +18,11 @@ %div{"ng-if" => "product.description"} %hr - %p.text-small {{product.description}} + %p.text-small{"bo-text" => "product.description"} %hr .columns.small-12.large-6 - %img.product-img{"ng-src" => "{{product.largeImage}}", "ng-if" => "product.largeImage"} - %img.product-img.placeholder{"ng-src" => "/assets/noimage/large.png", "ng-if" => "!product.largeImage"} + %img.product-img{"bo-src" => "product.largeImage", "bo-if" => "product.largeImage"} + %img.product-img.placeholder{"bo-src" => "'/assets/noimage/large.png'", "bo-if" => "!product.largeImage"} %ng-include{src: "'partials/close.html'"} From 368402f115586014fd872d1f60f8a36fc8f5f69e Mon Sep 17 00:00:00 2001 From: Rick Giner Date: Sun, 3 May 2015 15:24:16 +1000 Subject: [PATCH 17/54] Changed selectors' ng-repeat. Using existing variable instead of method call --- app/assets/javascripts/templates/filter_selector.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/templates/filter_selector.html.haml b/app/assets/javascripts/templates/filter_selector.html.haml index ecd4fb7d6d..c6990c369f 100644 --- a/app/assets/javascripts/templates/filter_selector.html.haml +++ b/app/assets/javascripts/templates/filter_selector.html.haml @@ -1,4 +1,4 @@ -%div{ style: "display: inline-block" } - %active-selector{ ng: { repeat: "selector in selectors()", show: "ifDefined(selector.fits, true)" } } +%div{bindonce:true, style: "display: inline-block" } + %active-selector{ ng: { repeat: "selector in allSelectors", show: "ifDefined(selector.fits, true)" } } %render-svg{path: "{{selector.object.icon}}", ng: { if: "selector.object.icon"} } - %span {{ selector.object.name }} + %span{"bo-text" => "selector.object.name"} From 8788322492d822d9f9a0d9395c01f1411b9ebf6d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 11:10:50 +1000 Subject: [PATCH 18/54] Alllowing payments in payment reports to access soft-deleted payment methods --- app/controllers/spree/admin/reports_controller_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index f148456813..92653a5aa9 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -323,7 +323,7 @@ Spree::Admin::ReportsController.class_eval do sort_by: proc { |payment_state| payment_state } }, { group_by: proc { |payment| payment.order.distributor }, sort_by: proc { |distributor| distributor.name } }, - { group_by: proc { |payment| payment.payment_method }, + { group_by: proc { |payment| Spree::PaymentMethod.unscoped { payment.payment_method } }, sort_by: proc { |method| method.name } } ] when "itemised_payment_totals" From ad7e5a45bba0415ef8e700b621ab458b6c7fd6a8 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 17:58:36 +1000 Subject: [PATCH 19/54] Add updated merge script --- script/ci/merge_branch_to_master.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/script/ci/merge_branch_to_master.sh b/script/ci/merge_branch_to_master.sh index 0bb07b43e2..6a942c2d39 100755 --- a/script/ci/merge_branch_to_master.sh +++ b/script/ci/merge_branch_to_master.sh @@ -7,4 +7,8 @@ echo "--- Verifying branch is based on current master" exit_unless_master_merged echo "--- Pushing branch" -echo git push origin $BUILDKITE_COMMIT:master +git checkout master +git merge origin/master +git merge origin/$BUILDKITE_BRANCH +git push origin master +git checkout origin/$BUILDKITE_BRANCH From f84e704d99711335dad03c295dfe4c8ee397e126 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 17:59:36 +1000 Subject: [PATCH 20/54] Retry simple push-to-master script --- script/ci/merge_branch_to_master.sh | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/script/ci/merge_branch_to_master.sh b/script/ci/merge_branch_to_master.sh index 6a942c2d39..6e40ed4ef9 100755 --- a/script/ci/merge_branch_to_master.sh +++ b/script/ci/merge_branch_to_master.sh @@ -7,8 +7,4 @@ echo "--- Verifying branch is based on current master" exit_unless_master_merged echo "--- Pushing branch" -git checkout master -git merge origin/master -git merge origin/$BUILDKITE_BRANCH -git push origin master -git checkout origin/$BUILDKITE_BRANCH +git push origin $BUILDKITE_COMMIT:master From 485eee4bddae4d4f9251eba618b913d11945f75a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 18:07:50 +1000 Subject: [PATCH 21/54] Deploy scripts display their output --- script/ci/push_to_production.sh | 4 +++- script/ci/push_to_staging.sh | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/script/ci/push_to_production.sh b/script/ci/push_to_production.sh index b731b538ca..8a8d4bf318 100755 --- a/script/ci/push_to_production.sh +++ b/script/ci/push_to_production.sh @@ -12,4 +12,6 @@ echo "--- Saving baseline data for staging" ssh ofn-staging2 "/home/openfoodweb/apps/openfoodweb/current/script/ci/save_staging_baseline.sh $BUILDKITE_COMMIT" echo "--- Pushing to production" -[[ $(git push production $BUILDKITE_COMMIT:master --force 2>&1) =~ "Done" ]] +output=$(git push production $BUILDKITE_COMMIT:master --force 2>&1) +echo $output +[[ $output =~ "Done" ]] diff --git a/script/ci/push_to_staging.sh b/script/ci/push_to_staging.sh index 582f500a76..0a5a11235a 100755 --- a/script/ci/push_to_staging.sh +++ b/script/ci/push_to_staging.sh @@ -16,4 +16,6 @@ echo "--- Loading baseline data" ssh ofn-staging2 "/home/openfoodweb/apps/openfoodweb/current/script/ci/load_staging_baseline.sh" echo "--- Pushing to staging" -[[ $(git push staging2 $BUILDKITE_COMMIT:master --force 2>&1) =~ "Done" ]] +output=$(git push staging2 $BUILDKITE_COMMIT:master --force 2>&1) +echo $output +[[ $output =~ "Done" ]] From 7b4130972b576890e845bfd4e4246c7f09632264 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 18:12:17 +1000 Subject: [PATCH 22/54] Fix first feature spec sometimes timing out --- spec/features/admin/authentication_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/features/admin/authentication_spec.rb b/spec/features/admin/authentication_spec.rb index 059963fd70..8473551c99 100644 --- a/spec/features/admin/authentication_spec.rb +++ b/spec/features/admin/authentication_spec.rb @@ -10,15 +10,15 @@ feature "Authentication", js: true do scenario "logging into admin redirects home, then back to admin" do # This is the first admin spec, so give a little extra load time for slow systems - Capybara.using_wait_time(60) do + Capybara.using_wait_time(120) do visit spree.admin_path - end - fill_in "Email", with: user.email - fill_in "Password", with: user.password - click_login_button - page.should have_content "DASHBOARD" - current_path.should == spree.admin_path + fill_in "Email", with: user.email + fill_in "Password", with: user.password + click_login_button + page.should have_content "DASHBOARD" + current_path.should == spree.admin_path + end end scenario "viewing my account" do From 3dee29cd124f72b7ce802c4cbfa10aea041be7ec Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 19:37:21 +1000 Subject: [PATCH 23/54] Remove build badge - master branch is always green --- README.markdown | 1 - 1 file changed, 1 deletion(-) diff --git a/README.markdown b/README.markdown index f43856daf6..b9aaf8193c 100644 --- a/README.markdown +++ b/README.markdown @@ -1,4 +1,3 @@ -[![Build status](https://badge.buildkite.com/e18473fffac95ed2735ca75700673bd301cac5cffc64de821a.svg?branch=master)](https://buildkite.com/open-food-foundation/open-food-network) [![Code Climate](https://codeclimate.com/github/openfoodfoundation/openfoodnetwork.png)](https://codeclimate.com/github/openfoodfoundation/openfoodnetwork) # Open Food Network From d109e898d281d5add8882d88e0988298cfbed3ae Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 19:38:20 +1000 Subject: [PATCH 24/54] Preserve newlines when displaying deploy script output --- script/ci/push_to_production.sh | 2 +- script/ci/push_to_staging.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/script/ci/push_to_production.sh b/script/ci/push_to_production.sh index 8a8d4bf318..1666368e08 100755 --- a/script/ci/push_to_production.sh +++ b/script/ci/push_to_production.sh @@ -13,5 +13,5 @@ ssh ofn-staging2 "/home/openfoodweb/apps/openfoodweb/current/script/ci/save_stag echo "--- Pushing to production" output=$(git push production $BUILDKITE_COMMIT:master --force 2>&1) -echo $output +echo "$output" [[ $output =~ "Done" ]] diff --git a/script/ci/push_to_staging.sh b/script/ci/push_to_staging.sh index 0a5a11235a..a38cda0c1b 100755 --- a/script/ci/push_to_staging.sh +++ b/script/ci/push_to_staging.sh @@ -17,5 +17,5 @@ ssh ofn-staging2 "/home/openfoodweb/apps/openfoodweb/current/script/ci/load_stag echo "--- Pushing to staging" output=$(git push staging2 $BUILDKITE_COMMIT:master --force 2>&1) -echo $output +echo "$output" [[ $output =~ "Done" ]] From 6d33dc5070db0f20d205e8ffcc4158741986f2a7 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 19:41:36 +1000 Subject: [PATCH 25/54] Add script to merge master into the current branch if required before running specs --- script/ci/includes.sh | 7 +++++++ script/ci/merge_master_into_branch.sh | 12 ++++++++++++ 2 files changed, 19 insertions(+) create mode 100755 script/ci/merge_master_into_branch.sh diff --git a/script/ci/includes.sh b/script/ci/includes.sh index ee59a732d7..d7619f7d23 100644 --- a/script/ci/includes.sh +++ b/script/ci/includes.sh @@ -12,6 +12,13 @@ function exit_unless_master_merged { fi } +function succeed_if_master_merged { + if [[ `git branch -a --merged origin/$BUILDKITE_BRANCH` == *origin/master* ]]; then + echo "This branch already has the current master merged." + exit 0 + fi +} + function drop_and_recreate_database { # Adapted from: http://stackoverflow.com/questions/12924466/capistrano-with-postgresql-error-database-is-being-accessed-by-other-users psql -U openfoodweb postgres < Date: Wed, 6 May 2015 19:50:06 +1000 Subject: [PATCH 26/54] Add debugging to merge script --- script/ci/merge_branch_to_master.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/ci/merge_branch_to_master.sh b/script/ci/merge_branch_to_master.sh index 6e40ed4ef9..3eaae17d56 100755 --- a/script/ci/merge_branch_to_master.sh +++ b/script/ci/merge_branch_to_master.sh @@ -1,6 +1,6 @@ #!/bin/bash -set -e +set -ex source ./script/ci/includes.sh echo "--- Verifying branch is based on current master" From 50d2ddc05f68c3965793a21c64a30a4ce664c4fd Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 20:00:50 +1000 Subject: [PATCH 27/54] Add progress comments --- script/ci/merge_master_into_branch.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/ci/merge_master_into_branch.sh b/script/ci/merge_master_into_branch.sh index ebbafd865c..a5e05c7759 100755 --- a/script/ci/merge_master_into_branch.sh +++ b/script/ci/merge_master_into_branch.sh @@ -3,8 +3,10 @@ set -e source ./script/ci/includes.sh +echo "--- Checking if master has already been merged" succeed_if_master_merged +echo "--- Merging master into this branch" git checkout $BUILDKITE_BRANCH git merge origin/$BUILDKITE_BRANCH git merge origin/master -m "Auto-merge from CI [skip ci]" From bd6bac887455b30545e610e306739be545d8ebd0 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 6 May 2015 20:02:03 +1000 Subject: [PATCH 28/54] Display deployment output in real time This reverts commit 485eee4bddae4d4f9251eba618b913d11945f75a. --- script/ci/push_to_production.sh | 6 ++---- script/ci/push_to_staging.sh | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/script/ci/push_to_production.sh b/script/ci/push_to_production.sh index 1666368e08..00ade0af07 100755 --- a/script/ci/push_to_production.sh +++ b/script/ci/push_to_production.sh @@ -1,6 +1,6 @@ #!/bin/bash -set -e +set -ex # Add production git remote if required PROD_TEST=`git remote | grep -s 'production' || true` @@ -12,6 +12,4 @@ echo "--- Saving baseline data for staging" ssh ofn-staging2 "/home/openfoodweb/apps/openfoodweb/current/script/ci/save_staging_baseline.sh $BUILDKITE_COMMIT" echo "--- Pushing to production" -output=$(git push production $BUILDKITE_COMMIT:master --force 2>&1) -echo "$output" -[[ $output =~ "Done" ]] +[[ $(git push production $BUILDKITE_COMMIT:master --force 2>&1) =~ "Done" ]] diff --git a/script/ci/push_to_staging.sh b/script/ci/push_to_staging.sh index a38cda0c1b..c634ca53a4 100755 --- a/script/ci/push_to_staging.sh +++ b/script/ci/push_to_staging.sh @@ -1,6 +1,6 @@ #!/bin/bash -set -e +set -ex source ./script/ci/includes.sh # Add staging git remote if required @@ -16,6 +16,4 @@ echo "--- Loading baseline data" ssh ofn-staging2 "/home/openfoodweb/apps/openfoodweb/current/script/ci/load_staging_baseline.sh" echo "--- Pushing to staging" -output=$(git push staging2 $BUILDKITE_COMMIT:master --force 2>&1) -echo "$output" -[[ $output =~ "Done" ]] +[[ $(git push staging2 $BUILDKITE_COMMIT:master --force 2>&1) =~ "Done" ]] From 89b153dc2c8fcd899fe1606b0c8f09cb06a4c4ce Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 29 Apr 2015 13:43:11 +1000 Subject: [PATCH 29/54] Update AJAX request to use sells instead of deprecated is_distributor attribute --- .../javascripts/admin/bulk_order_management.js.coffee | 2 +- spec/javascripts/unit/bulk_order_management_spec.js.coffee | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 4c1a319c1a..03d0890408 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -44,7 +44,7 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ dataFetcher("/api/enterprises/accessible?template=bulk_index&q[is_primary_producer_eq]=true").then (data) -> $scope.suppliers = data $scope.suppliers.unshift blankOption() - dataFetcher("/api/enterprises/accessible?template=bulk_index&q[is_distributor_eq]=true").then (data) -> + dataFetcher("/api/enterprises/accessible?template=bulk_index&q[sells_in][]=own&q[sells_in][]=any").then (data) -> $scope.distributors = data $scope.distributors.unshift blankOption() ocFetcher = dataFetcher("/api/order_cycles/accessible").then (data) -> diff --git a/spec/javascripts/unit/bulk_order_management_spec.js.coffee b/spec/javascripts/unit/bulk_order_management_spec.js.coffee index 35b9e13d61..61164870ed 100644 --- a/spec/javascripts/unit/bulk_order_management_spec.js.coffee +++ b/spec/javascripts/unit/bulk_order_management_spec.js.coffee @@ -22,7 +22,7 @@ describe "AdminOrderMgmtCtrl", -> returnedOrderCycles = [ "oc1", "oc2", "oc3" ] httpBackend.expectGET("/api/users/authorise_api?token=API_KEY").respond success: "Use of API Authorised" httpBackend.expectGET("/api/enterprises/accessible?template=bulk_index&q[is_primary_producer_eq]=true").respond returnedSuppliers - httpBackend.expectGET("/api/enterprises/accessible?template=bulk_index&q[is_distributor_eq]=true").respond returnedDistributors + httpBackend.expectGET("/api/enterprises/accessible?template=bulk_index&q[sells_in][]=own&q[sells_in][]=any").respond returnedDistributors httpBackend.expectGET("/api/order_cycles/accessible").respond returnedOrderCycles spyOn(scope, "initialiseVariables").andCallThrough() spyOn(scope, "fetchOrders").andReturn "nothing" @@ -33,8 +33,8 @@ describe "AdminOrderMgmtCtrl", -> httpBackend.flush() expect(scope.suppliers).toEqual [{ id : '0', name : 'All' }, 'list of suppliers'] - expect(scope.distributors).toEqual [ { id : '0', name : 'All' }, 'list of distributors' ] - expect(scope.orderCycles).toEqual [ { id : '0', name : 'All' }, 'oc1', 'oc2', 'oc3' ] + expect(scope.distributors).toEqual [ { id : '0', name : 'All' }, 'list of distributors' ] + expect(scope.orderCycles).toEqual [ { id : '0', name : 'All' }, 'oc1', 'oc2', 'oc3' ] expect(scope.initialiseVariables.calls.length).toBe 1 expect(scope.fetchOrders.calls.length).toBe 1 From d8c23d37ac02c3cf6f4ecf0dbbeef3fffcc54e24 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 29 Apr 2015 13:44:00 +1000 Subject: [PATCH 30/54] Update accessible_by scope on enterprise, to read from permissions --- app/models/enterprise.rb | 6 ++---- spec/models/enterprise_spec.rb | 11 +++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 1aa76b88a8..3cf00730a8 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -162,14 +162,12 @@ class Enterprise < ActiveRecord::Base end } - # Return enterprises that participate in order cycles that user coordinates, sends to or receives from + # Return enterprises that the user manages and those that have granted P-OC to managed enterprises scope :accessible_by, lambda { |user| if user.has_spree_role?('admin') scoped else - with_order_cycles_outer. - where('order_cycles.id IN (?)', OrderCycle.accessible_by(user)). - select('DISTINCT enterprises.*') + where(id: OpenFoodNetwork::Permissions.new(user).order_cycle_enterprises) end } diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index 69ba68569f..df5dbbfbc7 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -560,20 +560,19 @@ describe Enterprise do end describe "accessible_by" do - it "shows only enterprises that are invloved in order cycles which are common to those managed by the given user" do + it "shows only managed enterprises and enterprises granting them P-OC" do user = create(:user) user.spree_roles = [] e1 = create(:enterprise) e2 = create(:enterprise) e3 = create(:enterprise) - e4 = create(:enterprise) e1.enterprise_roles.build(user: user).save - oc = create(:simple_order_cycle, coordinator: e2, suppliers: [e1], distributors: [e3]) + create(:enterprise_relationship, parent: e2, child: e1, permissions_list: [:add_to_order_cycle]) enterprises = Enterprise.accessible_by user - enterprises.length.should == 3 - enterprises.should include e1, e2, e3 - enterprises.should_not include e4 + enterprises.length.should == 2 + enterprises.should include e1, e2 + enterprises.should_not include e3 end it "shows all enterprises for admin user" do From 9ab16d8cec54129265bc46237cba8781cc57cc9d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 30 Apr 2015 10:09:51 +1000 Subject: [PATCH 31/54] Allowing calls to Api::OrderCyclesController#accessible to specify :as => 'distributor' or 'producer' --- .../admin/bulk_order_management.js.coffee | 2 +- .../api/order_cycles_controller.rb | 11 +- app/models/order_cycle.rb | 19 +- .../api/order_cycles_controller_spec.rb | 257 ++++++++++++------ .../unit/bulk_order_management_spec.js.coffee | 2 +- 5 files changed, 209 insertions(+), 82 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 03d0890408..5d36718e9a 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -47,7 +47,7 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ dataFetcher("/api/enterprises/accessible?template=bulk_index&q[sells_in][]=own&q[sells_in][]=any").then (data) -> $scope.distributors = data $scope.distributors.unshift blankOption() - ocFetcher = dataFetcher("/api/order_cycles/accessible").then (data) -> + ocFetcher = dataFetcher("/api/order_cycles/accessible?as=distributor&q[orders_close_at_gt]=#{formatDate(daysFromToday(-90))}").then (data) -> $scope.orderCycles = data $scope.orderCycles.unshift blankOption() $scope.fetchOrders() diff --git a/app/controllers/api/order_cycles_controller.rb b/app/controllers/api/order_cycles_controller.rb index b4b3486778..23b333625e 100644 --- a/app/controllers/api/order_cycles_controller.rb +++ b/app/controllers/api/order_cycles_controller.rb @@ -9,7 +9,16 @@ module Api end def accessible - @order_cycles = OrderCycle.ransack(params[:q]).result.accessible_by(current_api_user) + @order_cycles = if params[:as] == "distributor" + OrderCycle.ransack(params[:q]).result. + involving_managed_distributors_of(current_api_user).order('updated_at DESC') + elsif params[:as] == "producer" + OrderCycle.ransack(params[:q]).result. + involving_managed_producers_of(current_api_user).order('updated_at DESC') + else + OrderCycle.ransack(params[:q]).result.accessible_by(current_api_user) + end + render params[:template] || :bulk_index end end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index adcd596a8c..d16f1d7b8b 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -26,7 +26,7 @@ class OrderCycle < ActiveRecord::Base closed. where("order_cycles.orders_close_at >= ?", 31.days.ago). order("order_cycles.orders_close_at DESC") } - + scope :soonest_opening, lambda { upcoming.order('order_cycles.orders_open_at ASC') } scope :distributing_product, lambda { |product| @@ -64,6 +64,23 @@ class OrderCycle < ActiveRecord::Base joins('LEFT OUTER JOIN enterprises ON (enterprises.id = exchanges.sender_id OR enterprises.id = exchanges.receiver_id)') } + scope :involving_managed_distributors_of, lambda { |user| + enterprises = Enterprise.managed_by(user) + + # Order cycles where I managed an enterprise at either end of an outgoing exchange + # ie. coordinator or distibutor + joins(:exchanges).merge(Exchange.outgoing). + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises) + } + + scope :involving_managed_producers_of, lambda { |user| + enterprises = Enterprise.managed_by(user) + + # Order cycles where I managed an enterprise at either end of an incoming exchange + # ie. coordinator or producer + joins(:exchanges).merge(Exchange.incoming). + where('exchanges.receiver_id IN (?) OR exchanges.sender_id IN (?)', enterprises, enterprises) + } def self.first_opening_for(distributor) with_distributor(distributor).soonest_opening.first diff --git a/spec/controllers/api/order_cycles_controller_spec.rb b/spec/controllers/api/order_cycles_controller_spec.rb index a9cceac796..3bb9a76602 100644 --- a/spec/controllers/api/order_cycles_controller_spec.rb +++ b/spec/controllers/api/order_cycles_controller_spec.rb @@ -4,109 +4,210 @@ require 'spree/api/testing_support/helpers' module Api describe OrderCyclesController do include Spree::Api::TestingSupport::Helpers + include AuthenticationWorkflow render_views - let!(:oc1) { FactoryGirl.create(:simple_order_cycle) } - let!(:oc2) { FactoryGirl.create(:simple_order_cycle) } - let(:coordinator) { oc1.coordinator } - let(:attributes) { [:id, :name, :suppliers, :distributors] } + describe "managed" do + let!(:oc1) { FactoryGirl.create(:simple_order_cycle) } + let!(:oc2) { FactoryGirl.create(:simple_order_cycle) } + let(:coordinator) { oc1.coordinator } + let(:attributes) { [:id, :name, :suppliers, :distributors] } - before do - stub_authentication! - Spree.user_class.stub :find_by_spree_api_key => current_api_user - end + before do + stub_authentication! + Spree.user_class.stub :find_by_spree_api_key => current_api_user + end - context "as a normal user" do - sign_in_as_user! + context "as a normal user" do + sign_in_as_user! - it "should deny me access to managed order cycles" do - spree_get :managed, { :format => :json } - assert_unauthorized! + it "should deny me access to managed order cycles" do + spree_get :managed, { :format => :json } + assert_unauthorized! + end + end + + context "as an enterprise user" do + sign_in_as_enterprise_user! [:coordinator] + + it "retrieves a list of variants with appropriate attributes" do + get :managed, { :format => :json } + keys = json_response.first.keys.map{ |key| key.to_sym } + attributes.all?{ |attr| keys.include? attr }.should == true + end + end + + context "as an administrator" do + sign_in_as_admin! + + it "retrieves a list of variants with appropriate attributes" do + get :managed, { :format => :json } + keys = json_response.first.keys.map{ |key| key.to_sym } + attributes.all?{ |attr| keys.include? attr }.should == true + end end end - context "as an enterprise user" do - sign_in_as_enterprise_user! [:coordinator] + describe "accessible" do + context "without :as parameter" do + let(:oc_supplier) { create(:supplier_enterprise) } + let(:oc_distributor) { create(:distributor_enterprise) } + let(:other_supplier) { create(:supplier_enterprise) } + let(:oc_supplier_user) do + user = create(:user) + user.spree_roles = [] + user.enterprise_roles.create(enterprise: oc_supplier) + user.save! + user + end + let(:oc_distributor_user) do + user = create(:user) + user.spree_roles = [] + user.enterprise_roles.create(enterprise: oc_distributor) + user.save! + user + end + let(:other_supplier_user) do + user = create(:user) + user.spree_roles = [] + user.enterprise_roles.create(enterprise: other_supplier) + user.save! + user + end + let!(:order_cycle) { create(:simple_order_cycle, suppliers: [oc_supplier], distributors: [oc_distributor]) } - it "retrieves a list of variants with appropriate attributes" do - get :managed, { :format => :json } - keys = json_response.first.keys.map{ |key| key.to_sym } - attributes.all?{ |attr| keys.include? attr }.should == true - end - end + context "as the user of a supplier to an order cycle" do + before :each do + stub_authentication! + Spree.user_class.stub :find_by_spree_api_key => oc_supplier_user + spree_get :accessible, { :template => 'bulk_index', :format => :json } + end - context "as an administrator" do - sign_in_as_admin! + it "gives me access" do + json_response.length.should == 1 + json_response[0]['id'].should == order_cycle.id + end + end - it "retrieves a list of variants with appropriate attributes" do - get :managed, { :format => :json } - keys = json_response.first.keys.map{ |key| key.to_sym } - attributes.all?{ |attr| keys.include? attr }.should == true - end - end + context "as the user of some other supplier" do + before :each do + stub_authentication! + Spree.user_class.stub :find_by_spree_api_key => other_supplier_user + spree_get :accessible, { :template => 'bulk_index', :format => :json } + end - context "using the accessible action to list order cycles" do - let(:oc_supplier) { create(:supplier_enterprise) } - let(:oc_distributor) { create(:distributor_enterprise) } - let(:other_supplier) { create(:supplier_enterprise) } - let(:oc_supplier_user) do - user = create(:user) - user.spree_roles = [] - user.enterprise_roles.create(enterprise: oc_supplier) - user.save! - user - end - let(:oc_distributor_user) do - user = create(:user) - user.spree_roles = [] - user.enterprise_roles.create(enterprise: oc_distributor) - user.save! - user - end - let(:other_supplier_user) do - user = create(:user) - user.spree_roles = [] - user.enterprise_roles.create(enterprise: other_supplier) - user.save! - user - end - let!(:order_cycle) { create(:simple_order_cycle, suppliers: [oc_supplier], distributors: [oc_distributor]) } + it "does not give me access" do + json_response.length.should == 0 + end + end - context "as the user of a supplier to an order cycle" do - before :each do + context "as the user of a hub for the order cycle" do + before :each do + stub_authentication! + Spree.user_class.stub :find_by_spree_api_key => oc_distributor_user + spree_get :accessible, { :template => 'bulk_index', :format => :json } + end + + it "gives me access" do + json_response.length.should == 1 + json_response[0]['id'].should == order_cycle.id + end + end + end + + context "when the :as parameter is set to 'distributor'" do + let(:user) { create_enterprise_user } + let(:distributor) { create(:distributor_enterprise) } + let(:producer) { create(:supplier_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let!(:oc) { create(:simple_order_cycle, coordinator: coordinator, distributors: [distributor], suppliers: [producer]) } + + let(:params) { { format: :json, as: 'distributor' } } + + before do stub_authentication! - Spree.user_class.stub :find_by_spree_api_key => oc_supplier_user - spree_get :accessible, { :template => 'bulk_index', :format => :json } + Spree.user_class.stub :find_by_spree_api_key => user end - it "gives me access" do - json_response.length.should == 1 - json_response[0]['id'].should == order_cycle.id + context "as the manager of a supplier in an order cycle" do + before do + user.enterprise_roles.create(enterprise: producer) + spree_get :accessible, params + end + + it "does not return the order cycle" do + expect(assigns(:order_cycles)).to_not include oc + end + end + + context "as the manager of a distributor in an order cycle" do + before do + user.enterprise_roles.create(enterprise: distributor) + spree_get :accessible, params + end + + it "returns the order cycle" do + expect(assigns(:order_cycles)).to include oc + end + end + + context "as the manager of the coordinator of an order cycle" do + before do + user.enterprise_roles.create(enterprise: coordinator) + spree_get :accessible, params + end + + it "returns the order cycle" do + expect(assigns(:order_cycles)).to include oc + end end end - context "as the user of some other supplier" do - before :each do + context "when the :as parameter is set to 'producer'" do + let(:user) { create_enterprise_user } + let(:distributor) { create(:distributor_enterprise) } + let(:producer) { create(:supplier_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let!(:oc) { create(:simple_order_cycle, coordinator: coordinator, distributors: [distributor], suppliers: [producer]) } + + let(:params) { { format: :json, as: 'producer' } } + + before do stub_authentication! - Spree.user_class.stub :find_by_spree_api_key => other_supplier_user - spree_get :accessible, { :template => 'bulk_index', :format => :json } + Spree.user_class.stub :find_by_spree_api_key => user end - it "does not give me access" do - json_response.length.should == 0 - end - end + context "as the manager of a producer in an order cycle" do + before do + user.enterprise_roles.create(enterprise: producer) + spree_get :accessible, params + end - context "as the user of a hub for the order cycle" do - before :each do - stub_authentication! - Spree.user_class.stub :find_by_spree_api_key => oc_distributor_user - spree_get :accessible, { :template => 'bulk_index', :format => :json } + it "returns the order cycle" do + expect(assigns(:order_cycles)).to include oc + end end - it "gives me access" do - json_response.length.should == 1 - json_response[0]['id'].should == order_cycle.id + context "as the manager of a distributor in an order cycle" do + before do + user.enterprise_roles.create(enterprise: distributor) + spree_get :accessible, params + end + + it "does not return the order cycle" do + expect(assigns(:order_cycles)).to_not include oc + end + end + + context "as the manager of the coordinator of an order cycle" do + before do + user.enterprise_roles.create(enterprise: coordinator) + spree_get :accessible, params + end + + it "returns the order cycle" do + expect(assigns(:order_cycles)).to include oc + end end end end diff --git a/spec/javascripts/unit/bulk_order_management_spec.js.coffee b/spec/javascripts/unit/bulk_order_management_spec.js.coffee index 61164870ed..e92f65ed5b 100644 --- a/spec/javascripts/unit/bulk_order_management_spec.js.coffee +++ b/spec/javascripts/unit/bulk_order_management_spec.js.coffee @@ -23,7 +23,7 @@ describe "AdminOrderMgmtCtrl", -> httpBackend.expectGET("/api/users/authorise_api?token=API_KEY").respond success: "Use of API Authorised" httpBackend.expectGET("/api/enterprises/accessible?template=bulk_index&q[is_primary_producer_eq]=true").respond returnedSuppliers httpBackend.expectGET("/api/enterprises/accessible?template=bulk_index&q[sells_in][]=own&q[sells_in][]=any").respond returnedDistributors - httpBackend.expectGET("/api/order_cycles/accessible").respond returnedOrderCycles + httpBackend.expectGET("/api/order_cycles/accessible?as=distributor&q[orders_close_at_gt]=SomeDate").respond returnedOrderCycles spyOn(scope, "initialiseVariables").andCallThrough() spyOn(scope, "fetchOrders").andReturn "nothing" #spyOn(returnedSuppliers, "unshift") From e640376d630d4d85c240b77f4d725ca5e26d6d15 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 30 Apr 2015 10:52:34 +1000 Subject: [PATCH 32/54] Don't load cancelled orders into bulk order management --- app/assets/javascripts/admin/bulk_order_management.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 5d36718e9a..61f275c9ed 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -60,7 +60,7 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.fetchOrders = -> $scope.loading = true - dataFetcher("/api/orders/managed?template=bulk_index;page=1;per_page=500;q[completed_at_not_null]=true;q[completed_at_gt]=#{$scope.startDate};q[completed_at_lt]=#{$scope.endDate}").then (data) -> + dataFetcher("/api/orders/managed?template=bulk_index;page=1;per_page=500;q[state_not_eq]=canceled;q[completed_at_not_null]=true;q[completed_at_gt]=#{$scope.startDate};q[completed_at_lt]=#{$scope.endDate}").then (data) -> $scope.resetOrders data $scope.loading = false From b5c7607d67bc5ccb34d5072ddd8870d989b9a254 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 30 Apr 2015 15:15:05 +1000 Subject: [PATCH 33/54] Order cycle filter resets date filters on BOM --- .../javascripts/admin/bulk_order_management.js.coffee | 7 +++++++ app/views/api/order_cycles/bulk_show.v1.rabl | 4 +++- spec/javascripts/unit/bulk_order_management_spec.js.coffee | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 61f275c9ed..420f7c3275 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -49,6 +49,8 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.distributors.unshift blankOption() ocFetcher = dataFetcher("/api/order_cycles/accessible?as=distributor&q[orders_close_at_gt]=#{formatDate(daysFromToday(-90))}").then (data) -> $scope.orderCycles = data + $scope.orderCyclesByID = [] + $scope.orderCyclesByID[oc.id] = oc for oc in $scope.orderCycles $scope.orderCycles.unshift blankOption() $scope.fetchOrders() ocFetcher.then -> @@ -162,6 +164,11 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ $scope.supplierFilter = $scope.suppliers[0].id $scope.orderCycleFilter = $scope.orderCycles[0].id $scope.quickSearch = "" + + $scope.$watch "orderCycleFilter", (newVal, oldVal) -> + unless $scope.orderCycleFilter == "0" || angular.equals(newVal, oldVal) + $scope.startDate = $scope.orderCyclesByID[$scope.orderCycleFilter].first_order + $scope.endDate = $scope.orderCyclesByID[$scope.orderCycleFilter].last_order ] daysFromToday = (days) -> diff --git a/app/views/api/order_cycles/bulk_show.v1.rabl b/app/views/api/order_cycles/bulk_show.v1.rabl index a0a5b16927..b012b879e2 100644 --- a/app/views/api/order_cycles/bulk_show.v1.rabl +++ b/app/views/api/order_cycles/bulk_show.v1.rabl @@ -1,9 +1,11 @@ object @order_cycle attributes :id, :name +node( :first_order ) { |order| order.orders_open_at.strftime("%F") } +node( :last_order ) { |order| (order.orders_close_at + 1.day).strftime("%F") } node( :suppliers ) do |oc| partial 'api/enterprises/bulk_index', :object => oc.suppliers end node( :distributors ) do |oc| partial 'api/enterprises/bulk_index', :object => oc.distributors -end \ No newline at end of file +end diff --git a/spec/javascripts/unit/bulk_order_management_spec.js.coffee b/spec/javascripts/unit/bulk_order_management_spec.js.coffee index e92f65ed5b..3377c799db 100644 --- a/spec/javascripts/unit/bulk_order_management_spec.js.coffee +++ b/spec/javascripts/unit/bulk_order_management_spec.js.coffee @@ -43,7 +43,7 @@ describe "AdminOrderMgmtCtrl", -> describe "fetching orders", -> beforeEach -> scope.initialiseVariables() - httpBackend.expectGET("/api/orders/managed?template=bulk_index;page=1;per_page=500;q[completed_at_not_null]=true;q[completed_at_gt]=SomeDate;q[completed_at_lt]=SomeDate").respond "list of orders" + httpBackend.expectGET("/api/orders/managed?template=bulk_index;page=1;per_page=500;q[state_not_eq]=canceled;q[completed_at_not_null]=true;q[completed_at_gt]=SomeDate;q[completed_at_lt]=SomeDate").respond "list of orders" it "makes a call to dataFetcher, with current start and end date parameters", -> scope.fetchOrders() From ed9bbe2c45aeb03f022bbf9c4bbaafb504f6bca7 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 30 Apr 2015 15:26:54 +1000 Subject: [PATCH 34/54] Sorting Hub and Producer filter selectors by name --- .../javascripts/admin/bulk_order_management.js.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_order_management.js.coffee b/app/assets/javascripts/admin/bulk_order_management.js.coffee index 420f7c3275..20ddc177ba 100644 --- a/app/assets/javascripts/admin/bulk_order_management.js.coffee +++ b/app/assets/javascripts/admin/bulk_order_management.js.coffee @@ -1,6 +1,6 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ - "$scope", "$http", "dataFetcher", "blankOption", "pendingChanges", "VariantUnitManager", "OptionValueNamer", "SpreeApiKey" - ($scope, $http, dataFetcher, blankOption, pendingChanges, VariantUnitManager, OptionValueNamer, SpreeApiKey) -> + "$scope", "$http", "$filter", "dataFetcher", "blankOption", "pendingChanges", "VariantUnitManager", "OptionValueNamer", "SpreeApiKey" + ($scope, $http, $filter, dataFetcher, blankOption, pendingChanges, VariantUnitManager, OptionValueNamer, SpreeApiKey) -> $scope.loading = true $scope.initialiseVariables = -> @@ -42,10 +42,10 @@ angular.module("ofn.admin").controller "AdminOrderMgmtCtrl", [ if $scope.spree_api_key_ok $http.defaults.headers.common["X-Spree-Token"] = SpreeApiKey dataFetcher("/api/enterprises/accessible?template=bulk_index&q[is_primary_producer_eq]=true").then (data) -> - $scope.suppliers = data + $scope.suppliers = $filter('orderBy')(data, 'name') $scope.suppliers.unshift blankOption() dataFetcher("/api/enterprises/accessible?template=bulk_index&q[sells_in][]=own&q[sells_in][]=any").then (data) -> - $scope.distributors = data + $scope.distributors = $filter('orderBy')(data, 'name') $scope.distributors.unshift blankOption() ocFetcher = dataFetcher("/api/order_cycles/accessible?as=distributor&q[orders_close_at_gt]=#{formatDate(daysFromToday(-90))}").then (data) -> $scope.orderCycles = data From 28bf7037db72bf0fb790fe1965e443ce1263bf4b Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 30 Apr 2015 16:20:15 +1000 Subject: [PATCH 35/54] Updating methods for retrieving allowed producers, distributors and order cycles for order and fulfillment reports --- .../spree/admin/reports_controller_decorator.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 92653a5aa9..ce714b088e 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -420,13 +420,17 @@ Spree::Admin::ReportsController.class_eval do my_distributors = Enterprise.is_distributor.managed_by(spree_current_user) my_suppliers = Enterprise.is_primary_producer.managed_by(spree_current_user) + permissions = OpenFoodNetwork::Permissions.new(spree_current_user) + # My distributors and any distributors distributing products I supply - @distributors = my_distributors | Enterprise.with_distributed_products_outer.merge(Spree::Product.in_any_supplier(my_suppliers)) + @distributors = permissions.order_cycle_enterprises.is_distributor # My suppliers and any suppliers supplying products I distribute - @suppliers = my_suppliers | my_distributors.map { |d| Spree::Product.in_distributor(d) }.flatten.map(&:supplier).uniq + @suppliers = permissions.order_cycle_enterprises.is_primary_producer + + @order_cycles = OrderCycle.active_or_complete. + involving_managed_distributors_of(spree_current_user).order('orders_close_at DESC') - @order_cycles = OrderCycle.active_or_complete.accessible_by(spree_current_user).order('orders_close_at DESC') @report_types = REPORT_TYPES[:orders_and_fulfillment] @report_type = params[:report_type] From 0a03483e3675eea49a1fbb1f8beff15f96079f40 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 1 May 2015 12:46:20 +1000 Subject: [PATCH 36/54] Adding permissions methods for visible and editable orders and line_items --- lib/open_food_network/permissions.rb | 50 +++++++ .../lib/open_food_network/permissions_spec.rb | 131 ++++++++++++++++++ 2 files changed, 181 insertions(+) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 4edb3d5f39..59fff81a87 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -56,6 +56,51 @@ module OpenFoodNetwork permissions end + # Find enterprises that an admin is allowed to add to an order cycle + def visible_orders + # Any orders that I can edit + editable = editable_orders.pluck(:id) + + # Any orders placed through hubs that my producers have granted P-OC, and which contain my their products + # This is pretty complicated but it's looking for order where at least one of my producers has granted + # P-OC to the distributor AND the order contains products of at least one of THE SAME producers + granted_distributors = granted(:add_to_order_cycle, by: managed_enterprises.is_primary_producer) + produced = Spree::Order.with_line_items_variants_and_products_outer. + where( + "spree_orders.distributor_id IN (?) AND spree_products.supplier_id IN (?)", + granted_distributors, + granting(:add_to_order_cycle, to: granted_distributors).merge(managed_enterprises.is_primary_producer) + ).pluck(:id) + + Spree::Order.where(id: editable | produced) + end + + # Find enterprises that an admin is allowed to add to an order cycle + def editable_orders + # Any orders placed through any hub that I manage + managed = Spree::Order.where(distributor_id: managed_enterprises.pluck(:id)).pluck(:id) + + # Any order that is placed through an order cycle one of my managed enterprises coordinates + coordinated = Spree::Order.where(order_cycle_id: coordinated_order_cycles.pluck(:id)).pluck(:id) + + Spree::Order.where(id: managed | coordinated ) + end + + def visible_line_items + # Any line items that I can edit + editable = editable_line_items.pluck(:id) + + # Any from visible orders, where the product is produced by one of my managed producers + produced = Spree::LineItem.where(order_id: visible_orders.pluck(:id)).joins(:product). + where('spree_products.supplier_id IN (?)', managed_enterprises.is_primary_producer.pluck(:id)) + + Spree::LineItem.where(id: editable | produced) + end + + def editable_line_items + Spree::LineItem.where(order_id: editable_orders) + end + def managed_products managed_enterprise_products_ids = managed_enterprise_products.pluck :id permitted_enterprise_products_ids = related_enterprise_products.pluck :id @@ -85,6 +130,11 @@ module OpenFoodNetwork @managed_enterprises = Enterprise.managed_by(@user) end + def coordinated_order_cycles + return @coordinated_order_cycles unless @coordinated_order_cycles.nil? + @coordinated_order_cycles = OrderCycle.managed_by(@user) + end + def related_enterprises_with(permission) parent_ids = EnterpriseRelationship. permitting(managed_enterprises). diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 88611b857c..d9f4608a60 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -185,5 +185,136 @@ module OpenFoodNetwork permissions.send(:related_enterprise_products).should == [p] end end + + describe "finding orders that are visible in reports" do + let(:distributor) { create(:distributor_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:random_enterprise) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, distributors: [distributor]) } + let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor ) } + let!(:line_item) { create(:line_item, order: order) } + let!(:producer) { create(:supplier_enterprise) } + + before do + permissions.stub(:coordinated_order_cycles) { Enterprise.where("1=0") } + end + + context "as the hub through which the order was placed" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: distributor) } + end + + it "should let me see the order" do + expect(permissions.visible_orders).to include order + end + end + + context "as the coordinator of the order cycle through which the order was placed" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: coordinator) } + permissions.stub(:coordinated_order_cycles) { OrderCycle.where(id: order_cycle) } + end + + it "should let me see the order" do + expect(permissions.visible_orders).to include order + end + end + + context "as a producer which has granted P-OC to the distributor of an order" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: producer) } + create(:enterprise_relationship, parent: producer, child: distributor, permissions_list: [:add_to_order_cycle]) + end + + context "which contains my products" do + before do + line_item.product.supplier = producer + line_item.product.save + end + + it "should let me see the order" do + expect(permissions.visible_orders).to include order + end + end + + context "which does not contain my products" do + it "should not let me see the order" do + expect(permissions.visible_orders).to_not include order + end + end + end + + context "as an enterprise that is a distributor in the order cycle, but not the distributor of the order" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: random_enterprise) } + end + + it "should not let me see the order" do + expect(permissions.visible_orders).to_not include order + end + end + end + + describe "finding line items that are visible in reports" do + let(:distributor) { create(:distributor_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:random_enterprise) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator, distributors: [distributor]) } + let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor ) } + let!(:line_item1) { create(:line_item, order: order) } + let!(:line_item2) { create(:line_item, order: order) } + let!(:producer) { create(:supplier_enterprise) } + + before do + permissions.stub(:coordinated_order_cycles) { Enterprise.where("1=0") } + end + + context "as the hub through which the parent order was placed" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: distributor) } + end + + it "should let me see the line_items" do + expect(permissions.visible_line_items).to include line_item1, line_item2 + end + end + + context "as the coordinator of the order cycle through which the parent order was placed" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: coordinator) } + permissions.stub(:coordinated_order_cycles) { OrderCycle.where(id: order_cycle) } + end + + it "should let me see the line_items" do + expect(permissions.visible_line_items).to include line_item1, line_item2 + end + end + + context "as the manager producer which has granted P-OC to the distributor of the parent order" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: producer) } + create(:enterprise_relationship, parent: producer, child: distributor, permissions_list: [:add_to_order_cycle]) + + line_item1.product.supplier = producer + line_item1.product.save + end + + it "should let me see the line_items pertaining to variants I produce" do + ps = permissions.visible_line_items + expect(ps).to include line_item1 + expect(ps).to_not include line_item2 + end + end + + context "as an enterprise that is a distributor in the order cycle, but not the distributor of the parent order" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where(id: random_enterprise) } + end + + it "should not let me see the line_items" do + expect(permissions.visible_line_items).to_not include line_item1, line_item2 + end + end + end end end From 4259b466f5a737b58999bb1ddaf0f861c5495177 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Sat, 2 May 2015 17:41:18 +1000 Subject: [PATCH 37/54] Using new order and line item permissions to fetch items to display in Orders and Fullfillment reports --- .../admin/reports_controller_decorator.rb | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index ce714b088e..beaee2c9be 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -406,22 +406,16 @@ Spree::Admin::ReportsController.class_eval do end params[:q][:meta_sort] ||= "completed_at.desc" - # -- Search - @search = Spree::Order.complete.not_state(:canceled).managed_by(spree_current_user).search(params[:q]) - orders = @search.result - @line_items = orders.map do |o| - lis = o.line_items.managed_by(spree_current_user) - lis = lis.supplied_by_any(params[:supplier_id_in]) if params[:supplier_id_in].present? - lis - end.flatten - #payments = orders.map { |o| o.payments.select { |payment| payment.completed? } }.flatten # Only select completed payments - - # -- Prepare form options - my_distributors = Enterprise.is_distributor.managed_by(spree_current_user) - my_suppliers = Enterprise.is_primary_producer.managed_by(spree_current_user) - permissions = OpenFoodNetwork::Permissions.new(spree_current_user) + # -- Search + + @search = Spree::Order.complete.not_state(:canceled).search(params[:q]) + orders = permissions.visible_orders.merge(@search.result) + + @line_items = permissions.visible_line_items.merge(Spree::LineItem.where(order_id: orders)) + @line_items = @line_items.supplied_by_any(params[:supplier_id_in]) if params[:supplier_id_in].present? + # My distributors and any distributors distributing products I supply @distributors = permissions.order_cycle_enterprises.is_distributor From f79fba52beab4aa818bc01a5ae7959392b1e1df1 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Sat, 2 May 2015 17:43:21 +1000 Subject: [PATCH 38/54] Hiding personal details of customers, where the user does not manage the distributor of the order or the coordinator of the order cycle --- .../spree/admin/reports_controller_decorator.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index beaee2c9be..ccc142f195 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -416,6 +416,15 @@ Spree::Admin::ReportsController.class_eval do @line_items = permissions.visible_line_items.merge(Spree::LineItem.where(order_id: orders)) @line_items = @line_items.supplied_by_any(params[:supplier_id_in]) if params[:supplier_id_in].present? + line_items_with_hidden_details = @line_items.where("id NOT IN (?)", permissions.editable_line_items) + @line_items.select{ |li| line_items_with_hidden_details.include? li }.each do |line_item| + # TODO We should really be hiding customer code here too, but until we + # have an actual association between order and customer, it's a bit tricky + line_item.order.bill_address.assign_attributes(firstname: "HIDDEN", lastname: "", phone: "", address1: "", address2: "", city: "", zipcode: "", state: nil) + line_item.order.ship_address.assign_attributes(firstname: "HIDDEN", lastname: "", phone: "", address1: "", address2: "", city: "", zipcode: "", state: nil) + line_item.order.assign_attributes(email: "HIDDEN") + end + # My distributors and any distributors distributing products I supply @distributors = permissions.order_cycle_enterprises.is_distributor From 7ffe0f042ee7e925d3a81214c54e54b7138395e0 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 11:48:39 +1000 Subject: [PATCH 39/54] Moving accessible_by scope on Enterprise to permissions --- app/controllers/api/enterprises_controller.rb | 3 +- app/models/enterprise.rb | 9 ------ lib/open_food_network/permissions.rb | 13 +++++++++ .../lib/open_food_network/permissions_spec.rb | 12 ++++++-- spec/models/enterprise_spec.rb | 28 ------------------- 5 files changed, 24 insertions(+), 41 deletions(-) diff --git a/app/controllers/api/enterprises_controller.rb b/app/controllers/api/enterprises_controller.rb index 468f3a22ad..973492b5e7 100644 --- a/app/controllers/api/enterprises_controller.rb +++ b/app/controllers/api/enterprises_controller.rb @@ -13,7 +13,8 @@ module Api end def accessible - @enterprises = Enterprise.ransack(params[:q]).result.accessible_by(current_api_user) + permitted = Permissions.new(current_api_user).enterprises_managed_or_granting_add_to_order_cycle + @enterprises = permitted.ransack(params[:q]).result render params[:template] || :bulk_index end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 3cf00730a8..e921167cf0 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -162,15 +162,6 @@ class Enterprise < ActiveRecord::Base end } - # Return enterprises that the user manages and those that have granted P-OC to managed enterprises - scope :accessible_by, lambda { |user| - if user.has_spree_role?('admin') - scoped - else - where(id: OpenFoodNetwork::Permissions.new(user).order_cycle_enterprises) - end - } - def self.find_near(suburb) enterprises = [] diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 59fff81a87..6b6ef85405 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -15,6 +15,15 @@ module OpenFoodNetwork managed_and_related_enterprises_with :add_to_order_cycle end + def enterprises_managed_or_granting_add_to_order_cycle + # Return enterprises that the user manages and those that have granted P-OC to managed enterprises + if admin? + Enterprise.scoped + else + managed_and_related_enterprises_with :add_to_order_cycle + end + end + # Find enterprises for which an admin is allowed to edit their profile def editable_enterprises managed_and_related_enterprises_with :edit_profile @@ -118,6 +127,10 @@ module OpenFoodNetwork private + def admin? + @user.admin? + end + def managed_and_related_enterprises_with(permission) managed_enterprise_ids = managed_enterprises.pluck :id permitting_enterprise_ids = related_enterprises_with(permission).pluck :id diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index d9f4608a60..e3262949ee 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -12,12 +12,18 @@ module OpenFoodNetwork let(:e) { double(:enterprise) } it "returns managed and related enterprises with add_to_order_cycle permission" do - permissions. - should_receive(:managed_and_related_enterprises_with). + allow(user).to receive(:admin?) { false } + expect(permissions).to receive(:managed_and_related_enterprises_with). with(:add_to_order_cycle). and_return([e]) - permissions.order_cycle_enterprises.should == [e] + expect(permissions.enterprises_managed_or_granting_add_to_order_cycle).to eq [e] + end + + it "shows all enterprises for admin user" do + allow(user).to receive(:admin?) { true } + + expect(permissions.enterprises_managed_or_granting_add_to_order_cycle).to eq [e1, e2] end end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index df5dbbfbc7..df1b58aca6 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -558,34 +558,6 @@ describe Enterprise do enterprises.should include e2 end end - - describe "accessible_by" do - it "shows only managed enterprises and enterprises granting them P-OC" do - user = create(:user) - user.spree_roles = [] - e1 = create(:enterprise) - e2 = create(:enterprise) - e3 = create(:enterprise) - e1.enterprise_roles.build(user: user).save - create(:enterprise_relationship, parent: e2, child: e1, permissions_list: [:add_to_order_cycle]) - - enterprises = Enterprise.accessible_by user - enterprises.length.should == 2 - enterprises.should include e1, e2 - enterprises.should_not include e3 - end - - it "shows all enterprises for admin user" do - user = create(:admin_user) - e1 = create(:enterprise) - e2 = create(:enterprise) - - enterprises = Enterprise.managed_by user - enterprises.length.should == 2 - enterprises.should include e1 - enterprises.should include e2 - end - end end describe "callbacks" do From f0f7e0ee2f164a725d88928e4e9ec9d656d11842 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 12:03:52 +1000 Subject: [PATCH 40/54] Making permissions method managed_and_related_enterprise_with method more specific --- lib/open_food_network/permissions.rb | 16 ++++++++-------- spec/lib/open_food_network/permissions_spec.rb | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 6b6ef85405..953314c8f8 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -5,14 +5,14 @@ module OpenFoodNetwork end def can_manage_complex_order_cycles? - managed_and_related_enterprises_with(:add_to_order_cycle).any? do |e| + managed_and_related_enterprises_granting(:add_to_order_cycle).any? do |e| e.sells == 'any' end end # Find enterprises that an admin is allowed to add to an order cycle def order_cycle_enterprises - managed_and_related_enterprises_with :add_to_order_cycle + managed_and_related_enterprises_granting :add_to_order_cycle end def enterprises_managed_or_granting_add_to_order_cycle @@ -20,17 +20,17 @@ module OpenFoodNetwork if admin? Enterprise.scoped else - managed_and_related_enterprises_with :add_to_order_cycle + managed_and_related_enterprises_granting :add_to_order_cycle end end # Find enterprises for which an admin is allowed to edit their profile def editable_enterprises - managed_and_related_enterprises_with :edit_profile + managed_and_related_enterprises_granting :edit_profile end def variant_override_hubs - managed_and_related_enterprises_with(:add_to_order_cycle).is_hub + managed_and_related_enterprises_granting(:add_to_order_cycle).is_hub end def variant_override_producers @@ -42,7 +42,7 @@ module OpenFoodNetwork # override variants # {hub1_id => [producer1_id, producer2_id, ...], ...} def variant_override_enterprises_per_hub - hubs = managed_and_related_enterprises_with(:add_to_order_cycle).is_distributor + hubs = managed_and_related_enterprises_granting(:add_to_order_cycle).is_distributor # Permissions granted by create_variant_overrides relationship from producer to hub permissions = Hash[ @@ -117,7 +117,7 @@ module OpenFoodNetwork end def managed_product_enterprises - managed_and_related_enterprises_with :manage_products + managed_and_related_enterprises_granting :manage_products end def manages_one_enterprise? @@ -131,7 +131,7 @@ module OpenFoodNetwork @user.admin? end - def managed_and_related_enterprises_with(permission) + def managed_and_related_enterprises_granting(permission) managed_enterprise_ids = managed_enterprises.pluck :id permitting_enterprise_ids = related_enterprises_with(permission).pluck :id diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index e3262949ee..6c89dc3db1 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -13,7 +13,7 @@ module OpenFoodNetwork it "returns managed and related enterprises with add_to_order_cycle permission" do allow(user).to receive(:admin?) { false } - expect(permissions).to receive(:managed_and_related_enterprises_with). + expect(permissions).to receive(:managed_and_related_enterprises_granting). with(:add_to_order_cycle). and_return([e]) @@ -32,7 +32,7 @@ module OpenFoodNetwork it "returns managed and related enterprises with edit_profile permission" do permissions. - should_receive(:managed_and_related_enterprises_with). + should_receive(:managed_and_related_enterprises_granting). with(:edit_profile). and_return([e]) @@ -139,7 +139,7 @@ module OpenFoodNetwork it "returns managed and related enterprises with manage_products permission" do permissions. - should_receive(:managed_and_related_enterprises_with). + should_receive(:managed_and_related_enterprises_granting). with(:manage_products). and_return([e]) @@ -171,13 +171,13 @@ module OpenFoodNetwork it "returns managed enterprises" do permissions.should_receive(:managed_enterprises) { Enterprise.where(id: e1) } - permissions.send(:managed_and_related_enterprises_with, permission).should == [e1] + permissions.send(:managed_and_related_enterprises_granting, permission).should == [e1] end it "returns permitted enterprises" do permissions.should_receive(:related_enterprises_with).with(permission). and_return(Enterprise.where(id: e2)) - permissions.send(:managed_and_related_enterprises_with, permission).should == [e2] + permissions.send(:managed_and_related_enterprises_granting, permission).should == [e2] end end From 5cd528a87d8ce669ca5ed33052bddcc732b89ed7 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 12:09:54 +1000 Subject: [PATCH 41/54] Removing obsolete related_enterprises_with permission method --- lib/open_food_network/permissions.rb | 13 ++----------- spec/lib/open_food_network/permissions_spec.rb | 10 +++++----- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 953314c8f8..3c6526ff30 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -133,7 +133,7 @@ module OpenFoodNetwork def managed_and_related_enterprises_granting(permission) managed_enterprise_ids = managed_enterprises.pluck :id - permitting_enterprise_ids = related_enterprises_with(permission).pluck :id + permitting_enterprise_ids = granting(permission).pluck :id Enterprise.where('id IN (?)', managed_enterprise_ids + permitting_enterprise_ids) end @@ -148,15 +148,6 @@ module OpenFoodNetwork @coordinated_order_cycles = OrderCycle.managed_by(@user) end - def related_enterprises_with(permission) - parent_ids = EnterpriseRelationship. - permitting(managed_enterprises). - with_permission(permission). - pluck(:parent_id) - - Enterprise.where('id IN (?)', parent_ids) - end - def granting(permission, options={}) parent_ids = EnterpriseRelationship. permitting(options[:to] || managed_enterprises). @@ -180,7 +171,7 @@ module OpenFoodNetwork end def related_enterprise_products - Spree::Product.where('supplier_id IN (?)', related_enterprises_with(:manage_products)) + Spree::Product.where('supplier_id IN (?)', granting(:manage_products)) end end end diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 6c89dc3db1..bc26b2da31 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -154,19 +154,19 @@ module OpenFoodNetwork it "returns the enterprises" do permissions.stub(:managed_enterprises) { e2 } - permissions.send(:related_enterprises_with, permission).should == [e1] + permissions.send(:granting, permission).should == [e1] end it "returns an empty array when there are none" do permissions.stub(:managed_enterprises) { e1 } - permissions.send(:related_enterprises_with, permission).should == [] + permissions.send(:granting, permission).should == [] end end describe "finding enterprises that are managed or with a particular permission" do before do permissions.stub(:managed_enterprises) { Enterprise.where('1=0') } - permissions.stub(:related_enterprises_with) { Enterprise.where('1=0') } + permissions.stub(:granting) { Enterprise.where('1=0') } end it "returns managed enterprises" do @@ -175,7 +175,7 @@ module OpenFoodNetwork end it "returns permitted enterprises" do - permissions.should_receive(:related_enterprises_with).with(permission). + permissions.should_receive(:granting).with(permission). and_return(Enterprise.where(id: e2)) permissions.send(:managed_and_related_enterprises_granting, permission).should == [e2] end @@ -186,7 +186,7 @@ module OpenFoodNetwork let!(:p) { create(:simple_product, supplier: e) } it "returns supplied products" do - permissions.should_receive(:related_enterprises_with).with(:manage_products) { [e] } + permissions.should_receive(:granting).with(:manage_products) { [e] } permissions.send(:related_enterprise_products).should == [p] end From 5806f50a849e8c6958d855a62095d5cae93aa238 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 12:14:23 +1000 Subject: [PATCH 42/54] Renaming granting > related_enterprises_granting --- .../order_cycle_permissions.rb | 18 +++++++++--------- lib/open_food_network/permissions.rb | 8 ++++---- spec/lib/open_food_network/permissions_spec.rb | 10 +++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 2af60c5696..14ac9a68be 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -19,7 +19,7 @@ module OpenFoodNetwork if @coordinator.sells == "any" # If the coordinator sells any, relationships come into play - granting(:add_to_order_cycle, to: [@coordinator]).pluck(:id).each do |enterprise_id| + related_enterprises_granting(:add_to_order_cycle, to: [@coordinator]).pluck(:id).each do |enterprise_id| coordinator_permitted << enterprise_id end @@ -30,19 +30,19 @@ module OpenFoodNetwork Enterprise.where(id: coordinator_permitted | all_active) else # Any enterprises that I manage directly, which have granted P-OC to the coordinator - managed_permitted = granting(:add_to_order_cycle, to: [@coordinator], scope: managed_participating_enterprises ).pluck(:id) + managed_permitted = related_enterprises_granting(:add_to_order_cycle, to: [@coordinator], scope: managed_participating_enterprises ).pluck(:id) # Any hubs in this OC that have been granted P-OC by producers I manage in this OC hubs_permitted = granted(:add_to_order_cycle, by: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id) # Any hubs in this OC that have granted P-OC to producers I manage in this OC - hubs_permitting = granting(:add_to_order_cycle, to: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id) + hubs_permitting = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id) # Any producers in this OC that have been granted P-OC by hubs I manage in this OC producers_permitted = granted(:add_to_order_cycle, by: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id) # Any producers in this OC that have granted P-OC to hubs I manage in this OC - producers_permitting = granting(:add_to_order_cycle, to: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id) + producers_permitting = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id) managed_active = [] hubs_active = [] @@ -125,7 +125,7 @@ module OpenFoodNetwork end # Any variants of any producers that have granted the hub P-OC - producers = granting(:add_to_order_cycle, to: [hub], scope: Enterprise.is_primary_producer) + producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: Enterprise.is_primary_producer) permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers) # PLUS any variants that are already in an outgoing exchange of this hub, so things don't break @@ -138,7 +138,7 @@ module OpenFoodNetwork Spree::Variant.where(id: coordinator_variants | permitted_variants | active_variants) else # Any variants produced by MY PRODUCERS that are in this order cycle, where my producer has granted P-OC to the hub - producers = granting(:add_to_order_cycle, to: [hub], scope: managed_participating_producers) + producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: managed_participating_producers) permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers) # PLUS any of my incoming producers' variants that are already in an outgoing exchange of this hub, so things don't break @@ -162,7 +162,7 @@ module OpenFoodNetwork end # Any variants of any producers that have granted the hub P-OC - producers = granting(:add_to_order_cycle, to: [hub], scope: Enterprise.is_primary_producer) + producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: Enterprise.is_primary_producer) permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', producers) # PLUS any variants that are already in an outgoing exchange of this hub, so things don't break @@ -178,7 +178,7 @@ module OpenFoodNetwork granted_producers = granted(:add_to_order_cycle, by: [hub], scope: managed_participating_producers) # Any variants produced by MY PRODUCERS that are in this order cycle, where my producer has granted P-OC to the hub - granting_producers = granting(:add_to_order_cycle, to: [hub], scope: granted_producers) + granting_producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: granted_producers) permitted_variants = Spree::Variant.joins(:product).where('spree_products.supplier_id IN (?)', granting_producers) Spree::Variant.where(id: permitted_variants) @@ -216,7 +216,7 @@ module OpenFoodNetwork # Find my managed hubs in this order cycle hubs = managed_participating_hubs # Any incoming exchange where the producer has granted P-OC to one or more of those hubs - producers = granting(:add_to_order_cycle, to: hubs, scope: Enterprise.is_primary_producer).pluck :id + producers = related_enterprises_granting(:add_to_order_cycle, to: hubs, scope: Enterprise.is_primary_producer).pluck :id permitted_exchanges = @order_cycle.exchanges.incoming.where(sender_id: producers).pluck :id # TODO: remove active_exchanges when we think it is safe to do so diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 3c6526ff30..296ab9843c 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -78,7 +78,7 @@ module OpenFoodNetwork where( "spree_orders.distributor_id IN (?) AND spree_products.supplier_id IN (?)", granted_distributors, - granting(:add_to_order_cycle, to: granted_distributors).merge(managed_enterprises.is_primary_producer) + related_enterprises_granting(:add_to_order_cycle, to: granted_distributors).merge(managed_enterprises.is_primary_producer) ).pluck(:id) Spree::Order.where(id: editable | produced) @@ -133,7 +133,7 @@ module OpenFoodNetwork def managed_and_related_enterprises_granting(permission) managed_enterprise_ids = managed_enterprises.pluck :id - permitting_enterprise_ids = granting(permission).pluck :id + permitting_enterprise_ids = related_enterprises_granting(permission).pluck :id Enterprise.where('id IN (?)', managed_enterprise_ids + permitting_enterprise_ids) end @@ -148,7 +148,7 @@ module OpenFoodNetwork @coordinated_order_cycles = OrderCycle.managed_by(@user) end - def granting(permission, options={}) + def related_enterprises_granting(permission, options={}) parent_ids = EnterpriseRelationship. permitting(options[:to] || managed_enterprises). with_permission(permission). @@ -171,7 +171,7 @@ module OpenFoodNetwork end def related_enterprise_products - Spree::Product.where('supplier_id IN (?)', granting(:manage_products)) + Spree::Product.where('supplier_id IN (?)', related_enterprises_granting(:manage_products)) end end end diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index bc26b2da31..3d796b9f46 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -154,19 +154,19 @@ module OpenFoodNetwork it "returns the enterprises" do permissions.stub(:managed_enterprises) { e2 } - permissions.send(:granting, permission).should == [e1] + permissions.send(:related_enterprises_granting, permission).should == [e1] end it "returns an empty array when there are none" do permissions.stub(:managed_enterprises) { e1 } - permissions.send(:granting, permission).should == [] + permissions.send(:related_enterprises_granting, permission).should == [] end end describe "finding enterprises that are managed or with a particular permission" do before do permissions.stub(:managed_enterprises) { Enterprise.where('1=0') } - permissions.stub(:granting) { Enterprise.where('1=0') } + permissions.stub(:related_enterprises_granting) { Enterprise.where('1=0') } end it "returns managed enterprises" do @@ -175,7 +175,7 @@ module OpenFoodNetwork end it "returns permitted enterprises" do - permissions.should_receive(:granting).with(permission). + permissions.should_receive(:related_enterprises_granting).with(permission). and_return(Enterprise.where(id: e2)) permissions.send(:managed_and_related_enterprises_granting, permission).should == [e2] end @@ -186,7 +186,7 @@ module OpenFoodNetwork let!(:p) { create(:simple_product, supplier: e) } it "returns supplied products" do - permissions.should_receive(:granting).with(:manage_products) { [e] } + permissions.should_receive(:related_enterprises_granting).with(:manage_products) { [e] } permissions.send(:related_enterprise_products).should == [p] end From d8f5669fbbf7e7bfb38c1fa1ac7696b0c0488b98 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 12:18:15 +1000 Subject: [PATCH 43/54] Renaming granted > related_enterprises_granted --- lib/open_food_network/order_cycle_permissions.rb | 8 ++++---- lib/open_food_network/permissions.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/open_food_network/order_cycle_permissions.rb b/lib/open_food_network/order_cycle_permissions.rb index 14ac9a68be..08fe7b2ee4 100644 --- a/lib/open_food_network/order_cycle_permissions.rb +++ b/lib/open_food_network/order_cycle_permissions.rb @@ -33,13 +33,13 @@ module OpenFoodNetwork managed_permitted = related_enterprises_granting(:add_to_order_cycle, to: [@coordinator], scope: managed_participating_enterprises ).pluck(:id) # Any hubs in this OC that have been granted P-OC by producers I manage in this OC - hubs_permitted = granted(:add_to_order_cycle, by: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id) + hubs_permitted = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id) # Any hubs in this OC that have granted P-OC to producers I manage in this OC hubs_permitting = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_producers, scope: @order_cycle.distributors).pluck(:id) # Any producers in this OC that have been granted P-OC by hubs I manage in this OC - producers_permitted = granted(:add_to_order_cycle, by: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id) + producers_permitted = related_enterprises_granted(:add_to_order_cycle, by: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id) # Any producers in this OC that have granted P-OC to hubs I manage in this OC producers_permitting = related_enterprises_granting(:add_to_order_cycle, to: managed_participating_hubs, scope: @order_cycle.suppliers).pluck(:id) @@ -175,7 +175,7 @@ module OpenFoodNetwork Spree::Variant.where(id: coordinator_variants | permitted_variants | active_variants) else # Any of my managed producers in this order cycle granted P-OC by the hub - granted_producers = granted(:add_to_order_cycle, by: [hub], scope: managed_participating_producers) + granted_producers = related_enterprises_granted(:add_to_order_cycle, by: [hub], scope: managed_participating_producers) # Any variants produced by MY PRODUCERS that are in this order cycle, where my producer has granted P-OC to the hub granting_producers = related_enterprises_granting(:add_to_order_cycle, to: [hub], scope: granted_producers) @@ -235,7 +235,7 @@ module OpenFoodNetwork # Find my producers in this order cycle producers = managed_participating_producers.pluck :id # Any outgoing exchange where the distributor has been granted P-OC by one or more of those producers - hubs = granted(:add_to_order_cycle, by: producers, scope: Enterprise.is_hub) + hubs = related_enterprises_granted(:add_to_order_cycle, by: producers, scope: Enterprise.is_hub) permitted_exchanges = @order_cycle.exchanges.outgoing.where(receiver_id: hubs).pluck :id # TODO: remove active_exchanges when we think it is safe to do so diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 296ab9843c..ea4d58208e 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -73,7 +73,7 @@ module OpenFoodNetwork # Any orders placed through hubs that my producers have granted P-OC, and which contain my their products # This is pretty complicated but it's looking for order where at least one of my producers has granted # P-OC to the distributor AND the order contains products of at least one of THE SAME producers - granted_distributors = granted(:add_to_order_cycle, by: managed_enterprises.is_primary_producer) + granted_distributors = related_enterprises_granted(:add_to_order_cycle, by: managed_enterprises.is_primary_producer) produced = Spree::Order.with_line_items_variants_and_products_outer. where( "spree_orders.distributor_id IN (?) AND spree_products.supplier_id IN (?)", @@ -157,7 +157,7 @@ module OpenFoodNetwork (options[:scope] || Enterprise).where('enterprises.id IN (?)', parent_ids) end - def granted(permission, options={}) + def related_enterprises_granted(permission, options={}) child_ids = EnterpriseRelationship. permitted_by(options[:by] || managed_enterprises). with_permission(permission). From bd66091d75a18104f121ddd6288f9b169c234614 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 12:49:07 +1000 Subject: [PATCH 44/54] Push logic for checking of user super admin status down into private method --- .../admin/reports_controller_decorator.rb | 4 +-- lib/open_food_network/permissions.rb | 16 +++++----- .../lib/open_food_network/permissions_spec.rb | 30 ++++++++++++++----- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index ccc142f195..69165da484 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -426,10 +426,10 @@ Spree::Admin::ReportsController.class_eval do end # My distributors and any distributors distributing products I supply - @distributors = permissions.order_cycle_enterprises.is_distributor + @distributors = permissions.order_report_enterprises(:add_to_order_cycle).is_distributor # My suppliers and any suppliers supplying products I distribute - @suppliers = permissions.order_cycle_enterprises.is_primary_producer + @suppliers = permissions.order_report_enterprises(:add_to_order_cycle).is_primary_producer @order_cycles = OrderCycle.active_or_complete. involving_managed_distributors_of(spree_current_user).order('orders_close_at DESC') diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index ea4d58208e..d7126263d5 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -17,11 +17,7 @@ module OpenFoodNetwork def enterprises_managed_or_granting_add_to_order_cycle # Return enterprises that the user manages and those that have granted P-OC to managed enterprises - if admin? - Enterprise.scoped - else - managed_and_related_enterprises_granting :add_to_order_cycle - end + managed_and_related_enterprises_granting :add_to_order_cycle end # Find enterprises for which an admin is allowed to edit their profile @@ -132,10 +128,14 @@ module OpenFoodNetwork end def managed_and_related_enterprises_granting(permission) - managed_enterprise_ids = managed_enterprises.pluck :id - permitting_enterprise_ids = related_enterprises_granting(permission).pluck :id + if admin? + Enterprise.scoped + else + managed_enterprise_ids = managed_enterprises.pluck :id + permitting_enterprise_ids = related_enterprises_granting(permission).pluck :id - Enterprise.where('id IN (?)', managed_enterprise_ids + permitting_enterprise_ids) + Enterprise.where('id IN (?)', managed_enterprise_ids + permitting_enterprise_ids) + end end def managed_enterprises diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 3d796b9f46..de22db9a59 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -8,23 +8,37 @@ module OpenFoodNetwork let(:e1) { create(:enterprise) } let(:e2) { create(:enterprise) } + describe "finding managed and related enterprises granting a particular permission" do + describe "as super admin" do + before { allow(user).to receive(:admin?) { true } } + + it "returns all enterprises" do + expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to eq [e1, e2] + end + end + + describe "as an enterprise user" do + let(:e3) { create(:enterprise) } + before { allow(user).to receive(:admin?) { false } } + + it "returns only my managed enterprises any that have granting them P-OC" do + expect(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) } + expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3) } + expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to eq [e1, e3] + end + end + end + describe "finding enterprises that can be added to an order cycle" do let(:e) { double(:enterprise) } it "returns managed and related enterprises with add_to_order_cycle permission" do - allow(user).to receive(:admin?) { false } expect(permissions).to receive(:managed_and_related_enterprises_granting). with(:add_to_order_cycle). and_return([e]) expect(permissions.enterprises_managed_or_granting_add_to_order_cycle).to eq [e] end - - it "shows all enterprises for admin user" do - allow(user).to receive(:admin?) { true } - - expect(permissions.enterprises_managed_or_granting_add_to_order_cycle).to eq [e1, e2] - end end describe "finding enterprises whose profiles can be edited" do @@ -61,6 +75,7 @@ module OpenFoodNetwork before do permissions.stub(:managed_enterprises) { Enterprise.where(id: hub.id) } + permissions.stub(:admin?) { false } end it "returns enterprises as hub_id => [producer, ...]" do @@ -167,6 +182,7 @@ module OpenFoodNetwork before do permissions.stub(:managed_enterprises) { Enterprise.where('1=0') } permissions.stub(:related_enterprises_granting) { Enterprise.where('1=0') } + permissions.stub(:admin?) { false } end it "returns managed enterprises" do From a7019e7e7829b212dc86bfb95a28274de5be2b6e Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 13:00:36 +1000 Subject: [PATCH 45/54] Adding permissions method for order report enterprises --- app/controllers/api/enterprises_controller.rb | 2 +- .../admin/reports_controller_decorator.rb | 4 +- lib/open_food_network/permissions.rb | 17 +++++++-- .../lib/open_food_network/permissions_spec.rb | 37 ++++++++++++++++++- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/app/controllers/api/enterprises_controller.rb b/app/controllers/api/enterprises_controller.rb index 973492b5e7..f39c34d22e 100644 --- a/app/controllers/api/enterprises_controller.rb +++ b/app/controllers/api/enterprises_controller.rb @@ -13,7 +13,7 @@ module Api end def accessible - permitted = Permissions.new(current_api_user).enterprises_managed_or_granting_add_to_order_cycle + permitted = Permissions.new(current_api_user).order_cycle_enterprises @enterprises = permitted.ransack(params[:q]).result render params[:template] || :bulk_index end diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 69165da484..3aa4f560a1 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -426,10 +426,10 @@ Spree::Admin::ReportsController.class_eval do end # My distributors and any distributors distributing products I supply - @distributors = permissions.order_report_enterprises(:add_to_order_cycle).is_distributor + @distributors = permissions.visible_enterprises_for_order_reports.is_distributor # My suppliers and any suppliers supplying products I distribute - @suppliers = permissions.order_report_enterprises(:add_to_order_cycle).is_primary_producer + @suppliers = permissions.visible_enterprises_for_order_reports.is_primary_producer @order_cycles = OrderCycle.active_or_complete. involving_managed_distributors_of(spree_current_user).order('orders_close_at DESC') diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index d7126263d5..4da645582d 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -11,11 +11,11 @@ module OpenFoodNetwork end # Find enterprises that an admin is allowed to add to an order cycle - def order_cycle_enterprises - managed_and_related_enterprises_granting :add_to_order_cycle + def visible_enterprises_for_order_reports + managed_and_related_enterprises_with :add_to_order_cycle end - def enterprises_managed_or_granting_add_to_order_cycle + def order_cycle_enterprises # Return enterprises that the user manages and those that have granted P-OC to managed enterprises managed_and_related_enterprises_granting :add_to_order_cycle end @@ -138,6 +138,17 @@ module OpenFoodNetwork end end + def managed_and_related_enterprises_with(permission) + if admin? + Enterprise.scoped + else + managed = managed_enterprises.pluck(:id) + granting = related_enterprises_granting(permission).pluck(:id) + granted = related_enterprises_granted(permission).pluck(:id) + Enterprise.where(id: managed | granting | granted) + end + end + def managed_enterprises return @managed_enterprises unless @managed_enterprises.nil? @managed_enterprises = Enterprise.managed_by(@user) diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index de22db9a59..10552572f0 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -29,6 +29,41 @@ module OpenFoodNetwork end end + describe "finding managed and related enterprises granting or granted a particular permission" do + describe "as super admin" do + before { allow(user).to receive(:admin?) { true } } + + it "returns all enterprises" do + expect(permissions.send(:managed_and_related_enterprises_granting, :some_permission)).to eq [e1, e2] + end + end + + describe "as an enterprise user" do + let(:e3) { create(:enterprise) } + let(:e4) { create(:enterprise) } + before { allow(user).to receive(:admin?) { false } } + + it "returns only my managed enterprises any that have granting them P-OC" do + expect(permissions).to receive(:managed_enterprises) { Enterprise.where(id: e1) } + expect(permissions).to receive(:related_enterprises_granting).with(:some_permission) { Enterprise.where(id: e3) } + expect(permissions).to receive(:related_enterprises_granted).with(:some_permission) { Enterprise.where(id: e4) } + expect(permissions.send(:managed_and_related_enterprises_with, :some_permission)).to eq [e1, e3, e4] + end + end + end + + describe "finding enterprises that can be selected in order report filters" do + let(:e) { double(:enterprise) } + + it "returns managed and related enterprises with add_to_order_cycle permission" do + expect(permissions).to receive(:managed_and_related_enterprises_with). + with(:add_to_order_cycle). + and_return([e]) + + expect(permissions.visible_enterprises_for_order_reports).to eq [e] + end + end + describe "finding enterprises that can be added to an order cycle" do let(:e) { double(:enterprise) } @@ -37,7 +72,7 @@ module OpenFoodNetwork with(:add_to_order_cycle). and_return([e]) - expect(permissions.enterprises_managed_or_granting_add_to_order_cycle).to eq [e] + expect(permissions.order_cycle_enterprises).to eq [e] end end From f88fdac710e637d18e4c3251c05a3df9767e65ba Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 13:53:59 +1000 Subject: [PATCH 46/54] Adding module - doh! --- app/controllers/api/enterprises_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/enterprises_controller.rb b/app/controllers/api/enterprises_controller.rb index f39c34d22e..41b24f30a1 100644 --- a/app/controllers/api/enterprises_controller.rb +++ b/app/controllers/api/enterprises_controller.rb @@ -13,7 +13,7 @@ module Api end def accessible - permitted = Permissions.new(current_api_user).order_cycle_enterprises + permitted = OpenFoodNetwork::Permissions.new(current_api_user).order_cycle_enterprises @enterprises = permitted.ransack(params[:q]).result render params[:template] || :bulk_index end From 68b4cb59be1cce148b663f806e921eb4a9fffce1 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 14:46:23 +1000 Subject: [PATCH 47/54] Fixing bulk management specs broken by making order_cycles filter update dates --- .../admin/bulk_order_management_spec.rb | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index c8ad230a00..1058bebfc0 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -238,8 +238,9 @@ feature %q{ end context "order_cycle filter" do - let!(:oc1) { FactoryGirl.create(:simple_order_cycle ) } - let!(:oc2) { FactoryGirl.create(:simple_order_cycle ) } + let!(:distributor) { create(:distributor_enterprise) } + let!(:oc1) { FactoryGirl.create(:simple_order_cycle, distributors: [distributor]) } + let!(:oc2) { FactoryGirl.create(:simple_order_cycle, distributors: [distributor]) } let!(:o1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, order_cycle: oc1 ) } let!(:o2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, order_cycle: oc2 ) } let!(:li1) { FactoryGirl.create(:line_item, order: o1 ) } @@ -255,20 +256,22 @@ feature %q{ find("div.select2-container#s2id_order_cycle_filter").click order_cycle_names.each { |ocn| page.should have_selector "div.select2-drop-active ul.select2-results li", text: ocn } find("div.select2-container#s2id_order_cycle_filter").click - page.should have_selector "tr#li_#{li1.id}", visible: true - page.should have_selector "tr#li_#{li2.id}", visible: true + page.should have_selector "tr#li_#{li1.id}" + page.should have_selector "tr#li_#{li2.id}" select2_select oc1.name, from: "order_cycle_filter" - page.should have_selector "tr#li_#{li1.id}", visible: true - page.should_not have_selector "tr#li_#{li2.id}", visible: true + page.should have_selector "#loading img.spinner" + page.should_not have_selector "#loading img.spinner" + page.should have_selector "tr#li_#{li1.id}" + page.should_not have_selector "tr#li_#{li2.id}" end it "displays all line items when 'All' is selected from order_cycle filter" do select2_select oc1.name, from: "order_cycle_filter" - page.should have_selector "tr#li_#{li1.id}", visible: true - page.should_not have_selector "tr#li_#{li2.id}", visible: true + page.should have_selector "tr#li_#{li1.id}" + page.should_not have_selector "tr#li_#{li2.id}" select2_select "All", from: "order_cycle_filter" - page.should have_selector "tr#li_#{li1.id}", visible: true - page.should have_selector "tr#li_#{li2.id}", visible: true + page.should have_selector "tr#li_#{li1.id}" + page.should have_selector "tr#li_#{li2.id}" end end From 0d5ce5ff57a804a76e380a41f60c2078b524ad49 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 6 May 2015 15:19:48 +1000 Subject: [PATCH 48/54] Fixing issues with reports controller spec --- .../spree/admin/reports_controller_spec.rb | 83 ++++++++++++------- 1 file changed, 55 insertions(+), 28 deletions(-) diff --git a/spec/controllers/spree/admin/reports_controller_spec.rb b/spec/controllers/spree/admin/reports_controller_spec.rb index 432bc4e67f..58d406519b 100644 --- a/spec/controllers/spree/admin/reports_controller_spec.rb +++ b/spec/controllers/spree/admin/reports_controller_spec.rb @@ -1,23 +1,25 @@ require 'spec_helper' describe Spree::Admin::ReportsController do - + # Given two distributors and two suppliers let(:ba) { create(:address) } let(:si) { "pick up on thursday please" } - let(:s1) { create(:supplier_enterprise, address: create(:address)) } - let(:s2) { create(:supplier_enterprise, address: create(:address)) } - let(:s3) { create(:supplier_enterprise, address: create(:address)) } - let(:d1) { create(:distributor_enterprise, address: create(:address)) } - let(:d2) { create(:distributor_enterprise, address: create(:address)) } - let(:d3) { create(:distributor_enterprise, address: create(:address)) } + let(:c1) { create(:distributor_enterprise) } + let(:c2) { create(:distributor_enterprise) } + let(:s1) { create(:supplier_enterprise) } + let(:s2) { create(:supplier_enterprise) } + let(:s3) { create(:supplier_enterprise) } + let(:d1) { create(:distributor_enterprise) } + let(:d2) { create(:distributor_enterprise) } + let(:d3) { create(:distributor_enterprise) } let(:p1) { create(:product, price: 12.34, distributors: [d1], supplier: s1) } let(:p2) { create(:product, price: 23.45, distributors: [d2], supplier: s2) } let(:p3) { create(:product, price: 34.56, distributors: [d3], supplier: s3) } # Given two order cycles with both distributors - let(:ocA) { create(:simple_order_cycle, distributors: [d1, d2], suppliers: [s1, s2, s3], variants: [p1.master, p3.master]) } - let(:ocB) { create(:simple_order_cycle, distributors: [d1, d2], suppliers: [s1, s2, s3], variants: [p2.master]) } + let(:ocA) { create(:simple_order_cycle, coordinator: c1, distributors: [d1, d2], suppliers: [s1, s2, s3], variants: [p1.master, p3.master]) } + let(:ocB) { create(:simple_order_cycle, coordinator: c2, distributors: [d1, d2], suppliers: [s1, s2, s3], variants: [p2.master]) } # orderA1 can only be accessed by s1, s3 and d1 let!(:orderA1) do @@ -53,15 +55,29 @@ describe Spree::Admin::ReportsController do order.save order end - - # As a Distributor Enterprise user for d1 + + # As manager of a coordinator (c1) + context "Coordinator Enterprise User" do + before { login_as_enterprise_user [c1] } + + describe 'Orders & Fulfillment' do + it "shows all orders in order cycles I coordinate" do + spree_get :orders_and_fulfillment + + assigns(:line_items).map(&:order).should include orderA1, orderA2 + assigns(:line_items).map(&:order).should_not include orderB1, orderB2 + end + end + end + + # As a Distributor Enterprise user for d1 context "Distributor Enterprise User" do before { login_as_enterprise_user [d1] } describe 'Orders and Distributors' do it "only shows orders that I have access to" do spree_get :orders_and_distributors - + assigns(:search).result.should include(orderA1, orderB1) assigns(:search).result.should_not include(orderA2) assigns(:search).result.should_not include(orderB2) @@ -71,7 +87,7 @@ describe Spree::Admin::ReportsController do describe 'Bulk Coop' do it "only shows orders that I have access to" do spree_get :bulk_coop - + assigns(:search).result.should include(orderA1, orderB1) assigns(:search).result.should_not include(orderA2) assigns(:search).result.should_not include(orderB2) @@ -81,7 +97,7 @@ describe Spree::Admin::ReportsController do describe 'Payments' do it "only shows orders that I have access to" do spree_get :payments - + assigns(:search).result.should include(orderA1, orderB1) assigns(:search).result.should_not include(orderA2) assigns(:search).result.should_not include(orderB2) @@ -89,12 +105,11 @@ describe Spree::Admin::ReportsController do end describe 'Orders & Fulfillment' do - it "only shows orders that I have access to" do + it "only shows orders that I distribute" do spree_get :orders_and_fulfillment - assigns(:search).result.should include(orderA1, orderB1) - assigns(:search).result.should_not include(orderA2) - assigns(:search).result.should_not include(orderB2) + assigns(:line_items).map(&:order).should include orderA1, orderB1 + assigns(:line_items).map(&:order).should_not include orderA2, orderB2 end it "only shows the selected order cycle" do @@ -114,19 +129,31 @@ describe Spree::Admin::ReportsController do it "only shows product line items that I am supplying" do spree_get :bulk_coop - assigns(:line_items).map(&:product).should include(p1) - assigns(:line_items).map(&:product).should_not include(p2) - assigns(:line_items).map(&:product).should_not include(p3) + assigns(:line_items).map(&:product).should include p1 + assigns(:line_items).map(&:product).should_not include p2, p3 end end describe 'Orders & Fulfillment' do - it "only shows product line items that I am supplying" do - spree_get :orders_and_fulfillment + context "where I have granted P-OC to the distributor" do + before do + create(:enterprise_relationship, parent: s1, child: d1, permissions_list: [:add_to_order_cycle]) + end - assigns(:line_items).map(&:product).should include(p1) - assigns(:line_items).map(&:product).should_not include(p2) - assigns(:line_items).map(&:product).should_not include(p3) + it "only shows product line items that I am supplying" do + spree_get :orders_and_fulfillment + + assigns(:line_items).map(&:product).should include p1 + assigns(:line_items).map(&:product).should_not include p2, p3 + end + end + + context "where I have not granted P-OC to the distributor" do + it "does not show me line_items I supply" do + spree_get :orders_and_fulfillment + + assigns(:line_items).map(&:product).should_not include p1, p2, p3 + end end it "only shows the selected order cycle" do @@ -143,7 +170,7 @@ describe Spree::Admin::ReportsController do it "should build distributors for the current user" do spree_get :products_and_inventory - assigns(:distributors).sort.should == [d1, d2, d3].sort + assigns(:distributors).sort.should == [c1, c2, d1, d2, d3].sort end it "builds suppliers for the current user" do @@ -184,7 +211,7 @@ describe Spree::Admin::ReportsController do it "should build distributors for the current user" do spree_get :customers - assigns(:distributors).sort.should == [d1, d2, d3].sort + assigns(:distributors).sort.should == [c1, c2, d1, d2, d3].sort end it "builds suppliers for the current user" do From f3f07662795f3b4aa281ad24bd0eaf9d3fc666aa Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 7 May 2015 12:48:31 +1000 Subject: [PATCH 49/54] Adding a distributor to order cycle to fix broken feature spec --- spec/features/admin/reports_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/reports_spec.rb b/spec/features/admin/reports_spec.rb index b1685be6db..309e0a3e81 100644 --- a/spec/features/admin/reports_spec.rb +++ b/spec/features/admin/reports_spec.rb @@ -211,8 +211,9 @@ feature %q{ end it "handles order cycles with nil opening or closing times" do - oc = create(:simple_order_cycle, name: "My Order Cycle", orders_open_at: Time.now, orders_close_at: nil) - o = create(:order, order_cycle: oc) + distributor = create(:distributor_enterprise) + oc = create(:simple_order_cycle, name: "My Order Cycle", distributors: [distributor], orders_open_at: Time.now, orders_close_at: nil) + o = create(:order, order_cycle: oc, distributor: distributor) login_to_admin_section visit spree.orders_and_fulfillment_admin_reports_path From 6fb3fa55a15e1dcab42cecbf0bdcb02f771ccf2f Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 7 May 2015 14:23:58 +1000 Subject: [PATCH 50/54] Allow extended time for all parts of this spec to fix intermittent fails --- spec/features/admin/authentication_spec.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/features/admin/authentication_spec.rb b/spec/features/admin/authentication_spec.rb index 059963fd70..8473551c99 100644 --- a/spec/features/admin/authentication_spec.rb +++ b/spec/features/admin/authentication_spec.rb @@ -10,15 +10,15 @@ feature "Authentication", js: true do scenario "logging into admin redirects home, then back to admin" do # This is the first admin spec, so give a little extra load time for slow systems - Capybara.using_wait_time(60) do + Capybara.using_wait_time(120) do visit spree.admin_path - end - fill_in "Email", with: user.email - fill_in "Password", with: user.password - click_login_button - page.should have_content "DASHBOARD" - current_path.should == spree.admin_path + fill_in "Email", with: user.email + fill_in "Password", with: user.password + click_login_button + page.should have_content "DASHBOARD" + current_path.should == spree.admin_path + end end scenario "viewing my account" do From 3520127c41a7928e25ba1ea3d1e7dfd86339ff4e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 13 May 2015 11:06:42 +1000 Subject: [PATCH 51/54] Fix infinite job loop --- app/models/spree/user_decorator.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 8298627547..d8ea312e23 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -1,5 +1,9 @@ Spree.user_class.class_eval do - handle_asynchronously :send_reset_password_instructions + if method_defined? :send_reset_password_instructions_with_delay + Bugsnag.notify RuntimeError.new "send_reset_password_instructions already handled asyncronously - double-calling results in infinite job loop" + else + handle_asynchronously :send_reset_password_instructions + end has_many :enterprise_roles, :dependent => :destroy has_many :enterprises, through: :enterprise_roles From b86872095a4425fe5047a4a800b0ecf868e04382 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 13 May 2015 14:52:17 +1000 Subject: [PATCH 52/54] Add google analytics --- app/views/layouts/darkswarm.html.haml | 1 + app/views/shared/_analytics.html.haml | 8 ++++++++ 2 files changed, 9 insertions(+) create mode 100644 app/views/shared/_analytics.html.haml diff --git a/app/views/layouts/darkswarm.html.haml b/app/views/layouts/darkswarm.html.haml index e1976ab167..a627ba4896 100644 --- a/app/views/layouts/darkswarm.html.haml +++ b/app/views/layouts/darkswarm.html.haml @@ -41,3 +41,4 @@ #footer %loading + = render 'shared/analytics' diff --git a/app/views/shared/_analytics.html.haml b/app/views/shared/_analytics.html.haml new file mode 100644 index 0000000000..ee9ba69923 --- /dev/null +++ b/app/views/shared/_analytics.html.haml @@ -0,0 +1,8 @@ +:javascript + (function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ + (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), + m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) + })(window,document,'script','//www.google-analytics.com/analytics.js','ga'); + + ga('create', 'UA-62912229-1', 'auto'); + ga('send', 'pageview'); From ef064819f9943a81066f0785edb8d2ef8ef096f9 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 15 May 2015 10:41:29 +1000 Subject: [PATCH 53/54] Add spec for order_cycle_management report access --- spec/models/spree/ability_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 0dc9364967..9ed432e4af 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -213,7 +213,7 @@ module Spree end it "should be able to read some reports" do - should have_ability([:admin, :index, :customers, :bulk_coop, :orders_and_fulfillment, :products_and_inventory], for: :report) + should have_ability([:admin, :index, :customers, :bulk_coop, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management], for: :report) end it "should not be able to read other reports" do @@ -400,7 +400,7 @@ module Spree end it "should be able to read some reports" do - should have_ability([:admin, :index, :customers, :sales_tax, :group_buys, :bulk_coop, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory], for: :report) + should have_ability([:admin, :index, :customers, :sales_tax, :group_buys, :bulk_coop, :payments, :orders_and_distributors, :orders_and_fulfillment, :products_and_inventory, :order_cycle_management], for: :report) end it "should not be able to read other reports" do From 312a6299a84d85dc1b5b87291ea3dc7e3edabdb3 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 15 May 2015 21:19:16 +1000 Subject: [PATCH 54/54] Making where clause unambiguous --- app/controllers/spree/admin/reports_controller_decorator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 3aa4f560a1..34f2d640c2 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -416,7 +416,7 @@ Spree::Admin::ReportsController.class_eval do @line_items = permissions.visible_line_items.merge(Spree::LineItem.where(order_id: orders)) @line_items = @line_items.supplied_by_any(params[:supplier_id_in]) if params[:supplier_id_in].present? - line_items_with_hidden_details = @line_items.where("id NOT IN (?)", permissions.editable_line_items) + line_items_with_hidden_details = @line_items.where('"spree_line_items"."id" NOT IN (?)', permissions.editable_line_items) @line_items.select{ |li| line_items_with_hidden_details.include? li }.each do |line_item| # TODO We should really be hiding customer code here too, but until we # have an actual association between order and customer, it's a bit tricky