From 722ccfc83b799f9f78b5e69171ca6cb74aa21545 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 31 Jul 2014 15:20:22 +1000 Subject: [PATCH 01/46] ng_text_field handling options parameter --- app/helpers/angular_form_builder.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/helpers/angular_form_builder.rb b/app/helpers/angular_form_builder.rb index 5d913eb291..56fcab79fa 100644 --- a/app/helpers/angular_form_builder.rb +++ b/app/helpers/angular_form_builder.rb @@ -12,8 +12,9 @@ class AngularFormBuilder < ActionView::Helpers::FormBuilder # @object.send(@fields_for_record_name).first.class.to_s.underscore --> enterprise_fee value = "{{ #{@object.send(@fields_for_record_name).first.class.to_s.underscore}.#{method} }}" + options.reverse_merge!({'id' => angular_id(method)}) - @template.text_field_tag angular_name(method), value, :id => angular_id(method) + @template.text_field_tag angular_name(method), value, options end def ng_hidden_field(method, options = {}) From bd3a4acc15a8062a680ac82c19d21aec3d71df3c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 31 Jul 2014 15:21:13 +1000 Subject: [PATCH 02/46] Giving an example name for an enterprise fee. The example is displayed as input placeholder. See bugherd #439. --- app/views/admin/enterprise_fees/index.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/enterprise_fees/index.html.haml b/app/views/admin/enterprise_fees/index.html.haml index d3e9a0b88c..48517c5931 100644 --- a/app/views/admin/enterprise_fees/index.html.haml +++ b/app/views/admin/enterprise_fees/index.html.haml @@ -27,7 +27,7 @@ = f.ng_hidden_field :id = f.ng_collection_select :enterprise_id, @enterprises, :id, :name, 'enterprise_fee.enterprise_id', :include_blank => true %td= f.ng_select :fee_type, enterprise_fee_type_options, 'enterprise_fee.fee_type' - %td= f.ng_text_field :name + %td= f.ng_text_field :name, { placeholder: 'e.g. packing fee' } %td= f.ng_collection_select :calculator_type, @calculators, :name, :description, 'enterprise_fee.calculator_type', {'class' => 'calculator_type', 'ng-model' => 'calculatorType', 'spree-ensure-calculator-preferences-match-type' => "1"} %td{'ng-bind-html-unsafe-compiled' => 'enterprise_fee.calculator_settings'} %td.actions{'spree-delete-resource' => "1"} From dd42b0c2399289ae0f6b5794fb754de79e10cc78 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 Aug 2014 14:38:44 +1000 Subject: [PATCH 03/46] Split out opening payments into own context --- .../consumer/shopping/checkout_spec.rb | 154 +++++++++--------- 1 file changed, 78 insertions(+), 76 deletions(-) diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 846fc57a9b..c35f01f8e0 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -79,53 +79,24 @@ feature "As a consumer I want to check out my cart", js: true do end end - before do - visit checkout_path - checkout_as_guest - toggle_payment - end - - it "shows all available payment methods" do - page.should have_content pm1.name - page.should have_content pm2.name - page.should have_content pm3.name - end - - describe "Purchasing" do - it "takes us to the order confirmation page when we submit a complete form" do - ActionMailer::Base.deliveries.clear - toggle_shipping - choose sm2.name + context "on the checkout page with payments open" do + before do + visit checkout_path + checkout_as_guest toggle_payment - choose pm1.name - toggle_details - within "#details" do - fill_in "First Name", with: "Will" - fill_in "Last Name", with: "Marshall" - fill_in "Email", with: "test@test.com" - fill_in "Phone", with: "0468363090" - end - toggle_billing - within "#billing" do - fill_in "Address", with: "123 Your Face" - select "Australia", from: "Country" - select "Victoria", from: "State" - fill_in "City", with: "Melbourne" - fill_in "Postcode", with: "3066" - - end - place_order - page.should have_content "Your order has been processed successfully" - ActionMailer::Base.deliveries.length.should == 1 - email = ActionMailer::Base.deliveries.last - site_name = Spree::Config[:site_name] - email.subject.should include "#{site_name} Order Confirmation" end - context "with basic details filled" do - before do + it "shows all available payment methods" do + page.should have_content pm1.name + page.should have_content pm2.name + page.should have_content pm3.name + end + + describe "Purchasing" do + it "takes us to the order confirmation page when we submit a complete form" do + ActionMailer::Base.deliveries.clear toggle_shipping - choose sm1.name + choose sm2.name toggle_payment choose pm1.name toggle_details @@ -137,52 +108,83 @@ feature "As a consumer I want to check out my cart", js: true do end toggle_billing within "#billing" do - fill_in "City", with: "Melbourne" - fill_in "Postcode", with: "3066" fill_in "Address", with: "123 Your Face" select "Australia", from: "Country" select "Victoria", from: "State" - end - toggle_shipping - check "Shipping address same as billing address?" - end + fill_in "City", with: "Melbourne" + fill_in "Postcode", with: "3066" - it "takes us to the order confirmation page when submitted with 'same as billing address' checked" do + end place_order page.should have_content "Your order has been processed successfully" + ActionMailer::Base.deliveries.length.should == 1 + email = ActionMailer::Base.deliveries.last + site_name = Spree::Config[:site_name] + email.subject.should include "#{site_name} Order Confirmation" end - context "with a credit card payment method" do - let!(:pm1) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::Gateway::Bogus") } - - it "takes us to the order confirmation page when submitted with a valid credit card" do + context "with basic details filled" do + before do + toggle_shipping + choose sm1.name toggle_payment - fill_in 'Card Number', with: "4111111111111111" - select 'February', from: 'secrets.card_month' - select (Date.today.year+1).to_s, from: 'secrets.card_year' - fill_in 'Security Code', with: '123' - - place_order - page.should have_content "Your order has been processed successfully" - - # Order should have a payment with the correct amount - o = Spree::Order.complete.first - o.payments.first.amount.should == 11.23 + choose pm1.name + toggle_details + within "#details" do + fill_in "First Name", with: "Will" + fill_in "Last Name", with: "Marshall" + fill_in "Email", with: "test@test.com" + fill_in "Phone", with: "0468363090" + end + toggle_billing + within "#billing" do + fill_in "City", with: "Melbourne" + fill_in "Postcode", with: "3066" + fill_in "Address", with: "123 Your Face" + select "Australia", from: "Country" + select "Victoria", from: "State" + end + toggle_shipping + check "Shipping address same as billing address?" end - it "shows the payment processing failed message when submitted with an invalid credit card" do - toggle_payment - fill_in 'Card Number', with: "9999999988887777" - select 'February', from: 'secrets.card_month' - select (Date.today.year+1).to_s, from: 'secrets.card_year' - fill_in 'Security Code', with: '123' - + it "takes us to the order confirmation page when submitted with 'same as billing address' checked" do place_order - page.should have_content "Payment could not be processed, please check the details you entered" + page.should have_content "Your order has been processed successfully" + end - # Does not show duplicate shipping fee - visit checkout_path - page.all("th", text: "Shipping").count.should == 1 + context "with a credit card payment method" do + let!(:pm1) { create(:payment_method, distributors: [distributor], name: "Roger rabbit", type: "Spree::Gateway::Bogus") } + + it "takes us to the order confirmation page when submitted with a valid credit card" do + toggle_payment + fill_in 'Card Number', with: "4111111111111111" + select 'February', from: 'secrets.card_month' + select (Date.today.year+1).to_s, from: 'secrets.card_year' + fill_in 'Security Code', with: '123' + + place_order + page.should have_content "Your order has been processed successfully" + + # Order should have a payment with the correct amount + o = Spree::Order.complete.first + o.payments.first.amount.should == 11.23 + end + + it "shows the payment processing failed message when submitted with an invalid credit card" do + toggle_payment + fill_in 'Card Number', with: "9999999988887777" + select 'February', from: 'secrets.card_month' + select (Date.today.year+1).to_s, from: 'secrets.card_year' + fill_in 'Security Code', with: '123' + + place_order + page.should have_content "Payment could not be processed, please check the details you entered" + + # Does not show duplicate shipping fee + visit checkout_path + page.all("th", text: "Shipping").count.should == 1 + end end end end From 4ceaec0ef521bc79a5d06921bec53177de212d01 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 Aug 2014 17:34:42 +1000 Subject: [PATCH 04/46] Do not error when checking out with a pre-loaded shipping/billing address --- .../darkswarm/services/checkout.js.coffee | 4 +++ .../consumer/shopping/checkout_spec.rb | 33 ++++++++++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/darkswarm/services/checkout.js.coffee b/app/assets/javascripts/darkswarm/services/checkout.js.coffee index 920c01dad9..cafd4d6718 100644 --- a/app/assets/javascripts/darkswarm/services/checkout.js.coffee +++ b/app/assets/javascripts/darkswarm/services/checkout.js.coffee @@ -32,6 +32,10 @@ Darkswarm.factory 'Checkout', (CurrentOrder, ShippingMethods, PaymentMethods, $h if @ship_address_same_as_billing munged_order.ship_address_attributes = munged_order.bill_address_attributes + # If the order already has a ship and bill address (as with logged in users with + # past orders), and we don't remove id here, then this will set the wrong id for + # ship address, and Rails will error with a 404 for that address. + delete munged_order.ship_address_attributes.id if @paymentMethod()?.method_type == 'gateway' angular.extend munged_order.payments_attributes[0], { diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index c35f01f8e0..54091e854d 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -15,8 +15,8 @@ feature "As a consumer I want to check out my cart", js: true do let(:product) { create(:simple_product, supplier: supplier) } let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor) } - before do + ActionMailer::Base.deliveries.clear add_enterprise_fee enterprise_fee set_order order add_product_to_cart @@ -56,7 +56,7 @@ feature "As a consumer I want to check out my cart", js: true do page.should have_content "Donkeys" end - context "When shipping method requires an address" do + context "when shipping method requires an address" do before do toggle_shipping choose sm1.name @@ -92,9 +92,8 @@ feature "As a consumer I want to check out my cart", js: true do page.should have_content pm3.name end - describe "Purchasing" do + describe "purchasing" do it "takes us to the order confirmation page when we submit a complete form" do - ActionMailer::Base.deliveries.clear toggle_shipping choose sm2.name toggle_payment @@ -117,7 +116,7 @@ feature "As a consumer I want to check out my cart", js: true do end place_order page.should have_content "Your order has been processed successfully" - ActionMailer::Base.deliveries.length.should == 1 + ActionMailer::Base.deliveries.length.should == 2 email = ActionMailer::Base.deliveries.last site_name = Spree::Config[:site_name] email.subject.should include "#{site_name} Order Confirmation" @@ -189,6 +188,30 @@ feature "As a consumer I want to check out my cart", js: true do end end end + + context "when the customer has a pre-set shipping and billing address" do + before do + # Load up the customer's order and give them a shipping and billing address + # This is equivalent to when the customer has ordered before and their addresses + # are pre-populated. + o = Spree::Order.last + o.ship_address = build(:address) + o.bill_address = build(:address) + o.save! + end + + it "checks out successfully" do + visit checkout_path + checkout_as_guest + choose sm2.name + toggle_payment + choose pm1.name + + place_order + page.should have_content "Your order has been processed successfully" + ActionMailer::Base.deliveries.length.should == 2 + end + end end end end From 3524e658f85842cf721d078f500ad9334c129cda Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 11:52:15 +1000 Subject: [PATCH 05/46] Error when creating product and master is invalid, instead of creating a product without a master --- app/models/spree/product_decorator.rb | 1 + app/models/spree/variant_decorator.rb | 6 +++--- spec/models/spree/product_spec.rb | 10 +++++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 4faa54a06d..d344e19935 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -17,6 +17,7 @@ Spree::Product.class_eval do attr_accessible :supplier_id, :primary_taxon_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value, :unit_description, :notes, :images_attributes, :display_as + validates_associated :master, message: "^Price and On Hand must be valid" validates_presence_of :supplier validates :primary_taxon, presence: { message: "^Product Category can't be blank" } diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index 7d4f178488..29d2ed89c1 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -9,14 +9,14 @@ Spree::Variant.class_eval do accepts_nested_attributes_for :images validates_presence_of :unit_value, - if: -> v { %w(weight volume).include? v.product.variant_unit }, + if: -> v { %w(weight volume).include? v.product.andand.variant_unit }, unless: :is_master validates_presence_of :unit_description, - if: -> v { v.product.variant_unit.present? && v.unit_value.nil? }, + if: -> v { v.product.andand.variant_unit.present? && v.unit_value.nil? }, unless: :is_master - before_validation :update_weight_from_unit_value + before_validation :update_weight_from_unit_value, if: -> v { v.product.present? } after_save :update_units scope :with_order_cycles_inner, joins(exchanges: :order_cycle) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index bba8a95301..f82d07674f 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -27,12 +27,20 @@ module Spree product.should_not be_valid end - it "should default available_on to now" do + it "defaults available_on to now" do Timecop.freeze product = Product.new product.available_on.should == Time.now end + it "does not save when master is invalid" do + s = create(:supplier_enterprise) + t = create(:taxon) + product = Product.new supplier_id: s.id, name: "Apples", price: 1, primary_taxon_id: t.id + product.on_hand = "10,000" + product.save.should be_false + end + context "when the product has variants" do let(:product) do product = create(:simple_product) From cb615ba994d1bf268cdd0dd5b973a271cf242540 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 13:07:31 +1000 Subject: [PATCH 06/46] Render enterprise relationships JSON with AMS instead of rabl --- .../admin/enterprise_relationships_controller.rb | 2 +- app/helpers/admin/injection_helper.rb | 4 ++++ .../api/admin/enterprise_relationship_serializer.rb | 11 +++++++++++ .../admin/enterprise_relationships/_data.html.haml | 6 ++---- app/views/admin/json/_enterprise_relationship.rabl | 11 ----------- app/views/admin/json/_enterprise_relationships.rabl | 2 -- 6 files changed, 18 insertions(+), 18 deletions(-) create mode 100644 app/serializers/api/admin/enterprise_relationship_serializer.rb delete mode 100644 app/views/admin/json/_enterprise_relationship.rabl delete mode 100644 app/views/admin/json/_enterprise_relationships.rabl diff --git a/app/controllers/admin/enterprise_relationships_controller.rb b/app/controllers/admin/enterprise_relationships_controller.rb index 212bf3849d..7ef435d7ed 100644 --- a/app/controllers/admin/enterprise_relationships_controller.rb +++ b/app/controllers/admin/enterprise_relationships_controller.rb @@ -10,7 +10,7 @@ module Admin @enterprise_relationship = EnterpriseRelationship.new params[:enterprise_relationship] if @enterprise_relationship.save - render partial: "admin/json/enterprise_relationship", locals: {enterprise_relationship: @enterprise_relationship} + render text: Api::Admin::EnterpriseRelationshipSerializer.new(@enterprise_relationship).to_json else render status: 400, json: {errors: @enterprise_relationship.errors.full_messages.join(', ')} end diff --git a/app/helpers/admin/injection_helper.rb b/app/helpers/admin/injection_helper.rb index 60bc911087..9ce62b2719 100644 --- a/app/helpers/admin/injection_helper.rb +++ b/app/helpers/admin/injection_helper.rb @@ -9,6 +9,10 @@ module Admin admin_inject_json_ams_array("ofn.admin", "all_enterprises", @all_enterprises, Api::Admin::EnterpriseSerializer) end + def admin_inject_enterprise_relationships + admin_inject_json_ams_array "ofn.admin", "enterprise_relationships", @enterprise_relationships, Api::Admin::EnterpriseRelationshipSerializer + end + def admin_inject_enterprise_roles admin_inject_json_ams_array "ofn.admin", "enterpriseRoles", @enterprise_roles, Api::Admin::EnterpriseRoleSerializer end diff --git a/app/serializers/api/admin/enterprise_relationship_serializer.rb b/app/serializers/api/admin/enterprise_relationship_serializer.rb new file mode 100644 index 0000000000..708920d8f5 --- /dev/null +++ b/app/serializers/api/admin/enterprise_relationship_serializer.rb @@ -0,0 +1,11 @@ +class Api::Admin::EnterpriseRelationshipSerializer < ActiveModel::Serializer + attributes :id, :parent_id, :parent_name, :child_id, :child_name + + def parent_name + object.parent.name + end + + def child_name + object.child.name + end +end diff --git a/app/views/admin/enterprise_relationships/_data.html.haml b/app/views/admin/enterprise_relationships/_data.html.haml index 7c13978e90..b6f7215afc 100644 --- a/app/views/admin/enterprise_relationships/_data.html.haml +++ b/app/views/admin/enterprise_relationships/_data.html.haml @@ -1,4 +1,2 @@ -:javascript - angular.module('ofn.admin').value('enterprise_relationships', #{render partial: "admin/json/enterprise_relationships", object: @enterprise_relationships}); - angular.module('ofn.admin').value('my_enterprises', #{render partial: "admin/json/enterprises", object: @my_enterprises}); - angular.module('ofn.admin').value('all_enterprises', #{render partial: "admin/json/enterprises", object: @all_enterprises}); += admin_inject_enterprise_relationships += admin_inject_enterprises diff --git a/app/views/admin/json/_enterprise_relationship.rabl b/app/views/admin/json/_enterprise_relationship.rabl deleted file mode 100644 index 9be152ec5c..0000000000 --- a/app/views/admin/json/_enterprise_relationship.rabl +++ /dev/null @@ -1,11 +0,0 @@ -object @enterprise_relationship - -attributes :id, :parent_id, :child_id - -node :parent_name do |enterprise_relationship| - enterprise_relationship.parent.name -end - -node :child_name do |enterprise_relationship| - enterprise_relationship.child.name -end diff --git a/app/views/admin/json/_enterprise_relationships.rabl b/app/views/admin/json/_enterprise_relationships.rabl deleted file mode 100644 index aad55b9770..0000000000 --- a/app/views/admin/json/_enterprise_relationships.rabl +++ /dev/null @@ -1,2 +0,0 @@ -collection @enterprise_relationships -extends "admin/json/enterprise_relationship" From 70feef1256d341407d810e27f8726b306b1acb3d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 13:26:23 +1000 Subject: [PATCH 07/46] Add EnterpriseRelationshipPermission model --- app/models/enterprise_relationship.rb | 1 + app/models/enterprise_relationship_permission.rb | 2 ++ ...3227_create_enterprise_relationship_permissions.rb | 11 +++++++++++ db/schema.rb | 11 ++++++++++- spec/factories.rb | 6 ++++++ 5 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 app/models/enterprise_relationship_permission.rb create mode 100644 db/migrate/20140825023227_create_enterprise_relationship_permissions.rb diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 4db650f2e7..312ff66572 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -1,6 +1,7 @@ class EnterpriseRelationship < ActiveRecord::Base belongs_to :parent, class_name: 'Enterprise', touch: true belongs_to :child, class_name: 'Enterprise', touch: true + has_many :permissions, class_name: 'EnterpriseRelationshipPermission' validates_presence_of :parent_id, :child_id validates_uniqueness_of :child_id, scope: :parent_id, message: "^That relationship is already established." diff --git a/app/models/enterprise_relationship_permission.rb b/app/models/enterprise_relationship_permission.rb new file mode 100644 index 0000000000..f615a91da7 --- /dev/null +++ b/app/models/enterprise_relationship_permission.rb @@ -0,0 +1,2 @@ +class EnterpriseRelationshipPermission < ActiveRecord::Base +end diff --git a/db/migrate/20140825023227_create_enterprise_relationship_permissions.rb b/db/migrate/20140825023227_create_enterprise_relationship_permissions.rb new file mode 100644 index 0000000000..1423c09db9 --- /dev/null +++ b/db/migrate/20140825023227_create_enterprise_relationship_permissions.rb @@ -0,0 +1,11 @@ +class CreateEnterpriseRelationshipPermissions < ActiveRecord::Migration + def change + create_table :enterprise_relationship_permissions do |t| + t.references :enterprise_relationship + t.string :name, null: false + end + + add_index :enterprise_relationship_permissions, :enterprise_relationship_id, name: 'index_erp_on_erid' + add_foreign_key :enterprise_relationship_permissions, :enterprise_relationships, name: 'erp_enterprise_relationship_id_fk' + end +end diff --git a/db/schema.rb b/db/schema.rb index 2c9c6b24ff..316c341576 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20140815065014) do +ActiveRecord::Schema.define(:version => 20140825023227) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -207,6 +207,13 @@ ActiveRecord::Schema.define(:version => 20140815065014) do add_index "enterprise_groups_enterprises", ["enterprise_group_id"], :name => "index_enterprise_groups_enterprises_on_enterprise_group_id" add_index "enterprise_groups_enterprises", ["enterprise_id"], :name => "index_enterprise_groups_enterprises_on_enterprise_id" + create_table "enterprise_relationship_permissions", :force => true do |t| + t.integer "enterprise_relationship_id" + t.string "name", :null => false + end + + add_index "enterprise_relationship_permissions", ["enterprise_relationship_id"], :name => "index_erp_on_erid" + create_table "enterprise_relationships", :force => true do |t| t.integer "parent_id" t.integer "child_id" @@ -1052,6 +1059,8 @@ ActiveRecord::Schema.define(:version => 20140815065014) do add_foreign_key "enterprise_groups_enterprises", "enterprise_groups", name: "enterprise_groups_enterprises_enterprise_group_id_fk" add_foreign_key "enterprise_groups_enterprises", "enterprises", name: "enterprise_groups_enterprises_enterprise_id_fk" + add_foreign_key "enterprise_relationship_permissions", "enterprise_relationships", name: "erp_enterprise_relationship_id_fk" + add_foreign_key "enterprise_relationships", "enterprises", name: "enterprise_relationships_child_id_fk", column: "child_id" add_foreign_key "enterprise_relationships", "enterprises", name: "enterprise_relationships_parent_id_fk", column: "parent_id" diff --git a/spec/factories.rb b/spec/factories.rb index 849d38abf3..39b3a01035 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -101,6 +101,12 @@ FactoryGirl.define do end factory :enterprise_relationship do + ignore { permissions [] } + after(:create) do |er, proxy| + proxy.permissions.each do |name| + er.permissions.create! name: name + end + end end factory :enterprise_role do From b4e89ad2c0a07e0632697eb21e5b66e2041a9276 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 13:27:01 +1000 Subject: [PATCH 08/46] Convert enterprise relationship permission to string presentation --- .../services/enterprise_relationships.js.coffee | 5 +++++ .../enterprise_relationships_spec.js.coffee | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee diff --git a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee index e07b992112..3185f229dd 100644 --- a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee +++ b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee @@ -16,3 +16,8 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris delete: (er) -> $http.delete('/admin/enterprise_relationships/' + er.id).success (data) => @enterprise_relationships.splice @enterprise_relationships.indexOf(er), 1 + + permission_presentation: (permission) -> + switch permission + when "add_products_to_order_cycle" then "can add products to order cycle from" + when "manage_products" then "can manage the products of" \ No newline at end of file diff --git a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee new file mode 100644 index 0000000000..4e90f65335 --- /dev/null +++ b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee @@ -0,0 +1,16 @@ +describe "enterprise relationships", -> + EnterpriseRelationships = null + enterprise_relationships = [] + + beforeEach -> + module "ofn.admin" + module ($provide) -> + $provide.value "enterprise_relationships", enterprise_relationships + null + + beforeEach inject (_EnterpriseRelationships_) -> + EnterpriseRelationships = _EnterpriseRelationships_ + + it "presents permission names", -> + expect(EnterpriseRelationships.permission_presentation("add_products_to_order_cycle")).toEqual "can add products to order cycle from" + expect(EnterpriseRelationships.permission_presentation("manage_products")).toEqual "can manage the products of" From 50c559964ce1791c78e7f3cad3b6ffafdb1caf29 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 13:28:02 +1000 Subject: [PATCH 09/46] Display enterprise relationship permissions --- ...rprise_relationship_permission_serializer.rb | 3 +++ .../admin/enterprise_relationship_serializer.rb | 2 ++ .../_enterprise_relationship.html.haml | 5 ++++- .../admin/enterprise_relationships_spec.rb | 17 +++++++++++------ 4 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 app/serializers/api/admin/enterprise_relationship_permission_serializer.rb diff --git a/app/serializers/api/admin/enterprise_relationship_permission_serializer.rb b/app/serializers/api/admin/enterprise_relationship_permission_serializer.rb new file mode 100644 index 0000000000..80ce487738 --- /dev/null +++ b/app/serializers/api/admin/enterprise_relationship_permission_serializer.rb @@ -0,0 +1,3 @@ +class Api::Admin::EnterpriseRelationshipPermissionSerializer < ActiveModel::Serializer + attributes :id, :name +end diff --git a/app/serializers/api/admin/enterprise_relationship_serializer.rb b/app/serializers/api/admin/enterprise_relationship_serializer.rb index 708920d8f5..5030620384 100644 --- a/app/serializers/api/admin/enterprise_relationship_serializer.rb +++ b/app/serializers/api/admin/enterprise_relationship_serializer.rb @@ -1,6 +1,8 @@ class Api::Admin::EnterpriseRelationshipSerializer < ActiveModel::Serializer attributes :id, :parent_id, :parent_name, :child_id, :child_name + has_many :permissions + def parent_name object.parent.name end diff --git a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml index 3f93027ca1..56f0d46c01 100644 --- a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml +++ b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml @@ -1,5 +1,8 @@ %td {{ enterprise_relationship.parent_name }} -%td permits +%td + %ul + %li{"ng-repeat" => "permission in enterprise_relationship.permissions"} + {{ EnterpriseRelationships.permission_presentation(permission.name) }} %td {{ enterprise_relationship.child_name }} %td.actions %a.delete-enterprise-relationship.icon-trash.no-text{'ng-click' => 'delete(enterprise_relationship)'} diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 1443cd4c34..112529376a 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -14,8 +14,9 @@ feature %q{ scenario "listing relationships" do # Given some enterprises with relationships e1, e2, e3, e4 = create(:enterprise), create(:enterprise), create(:enterprise), create(:enterprise) - create(:enterprise_relationship, parent: e1, child: e2) - create(:enterprise_relationship, parent: e3, child: e4) + create(:enterprise_relationship, parent: e1, child: e2, permissions: [:add_products_to_order_cycle]) + create(:enterprise_relationship, parent: e2, child: e3, permissions: [:manage_products]) + create(:enterprise_relationship, parent: e3, child: e4, permissions: [:add_products_to_order_cycle, :manage_products]) # When I go to the relationships page click_link 'Enterprises' @@ -23,8 +24,10 @@ feature %q{ # Then I should see the relationships within('table#enterprise-relationships') do - page.should have_relationship e1, e2 - page.should have_relationship e3, e4 + page.should have_relationship e1, e2, ['can add products to order cycle from'] + page.should have_relationship e2, e3, ['can manage the products of'] + page.should have_relationship e3, e4, + ['can add products to order cycle from', 'can manage the products of'] end end @@ -108,7 +111,9 @@ feature %q{ private - def have_relationship(parent, child) - have_table_row [parent.name, 'permits', child.name, ''] + def have_relationship(parent, child, perms=[]) + perms = perms.join(' ') || 'permits' + + have_table_row [parent.name, perms, child.name, ''] end end From 057ad9c6d3d54517123fba24b896dd6508e5ae35 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 14:39:24 +1000 Subject: [PATCH 10/46] Set enterprise relationship permissions from a list --- app/models/enterprise_relationship.rb | 5 +++++ spec/models/enterprise_relationship_spec.rb | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 312ff66572..5c10aafbbf 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -14,4 +14,9 @@ class EnterpriseRelationship < ActiveRecord::Base scope :involving_enterprises, ->(enterprises) { where('parent_id IN (?) OR child_id IN (?)', enterprises, enterprises) } + + + def permissions_list=(perms) + perms.andand.each { |name| permissions.build name: name } + end end diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index b715674594..6cdf3db657 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -29,5 +29,19 @@ describe EnterpriseRelationship do EnterpriseRelationship.involving_enterprises([e3]).should == [] end end + + describe "creating with a permission list" do + it "creates permissions with a list" do + er = EnterpriseRelationship.create! parent: e1, child: e2, permissions_list: ['one', 'two'] + er.reload + er.permissions.map(&:name).sort.should == ['one', 'two'].sort + end + + it "does nothing when the list is nil" do + er = EnterpriseRelationship.create! parent: e1, child: e2, permissions_list: nil + er.reload + er.permissions.should be_empty + end + end end end From 3932884dba98544e311d224498ca63b67eb10250 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 14:59:10 +1000 Subject: [PATCH 11/46] Admin can create enterprise relationships with permissions --- .../enterprise_relationships_controller.js.coffee | 3 ++- .../admin/services/enterprise_relationships.js.coffee | 9 +++++++-- app/views/admin/enterprise_relationships/_form.html.haml | 6 +++++- spec/features/admin/enterprise_relationships_spec.rb | 9 +++++++-- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/admin/controllers/enterprise_relationships_controller.js.coffee b/app/assets/javascripts/admin/controllers/enterprise_relationships_controller.js.coffee index 665753a522..88524fb330 100644 --- a/app/assets/javascripts/admin/controllers/enterprise_relationships_controller.js.coffee +++ b/app/assets/javascripts/admin/controllers/enterprise_relationships_controller.js.coffee @@ -1,9 +1,10 @@ angular.module("ofn.admin").controller "AdminEnterpriseRelationshipsCtrl", ($scope, EnterpriseRelationships, Enterprises) -> $scope.EnterpriseRelationships = EnterpriseRelationships $scope.Enterprises = Enterprises + $scope.permissions = {} $scope.create = -> - $scope.EnterpriseRelationships.create($scope.parent_id, $scope.child_id) + $scope.EnterpriseRelationships.create($scope.parent_id, $scope.child_id, $scope.permissions) $scope.delete = (enterprise_relationship) -> if confirm("Are you sure?") diff --git a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee index 3185f229dd..7a799f2f28 100644 --- a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee +++ b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee @@ -1,12 +1,17 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterprise_relationships) -> new class EnterpriseRelationships create_errors: "" + all_permissions: [ + 'add_products_to_order_cycle' + 'manage_products' + ] constructor: -> @enterprise_relationships = enterprise_relationships - create: (parent_id, child_id) -> - $http.post('/admin/enterprise_relationships', {enterprise_relationship: {parent_id: parent_id, child_id: child_id}}).success (data, status) => + create: (parent_id, child_id, permissions) -> + permissions = (name for name, enabled of permissions when enabled) + $http.post('/admin/enterprise_relationships', {enterprise_relationship: {parent_id: parent_id, child_id: child_id, permissions_list: permissions}}).success (data, status) => @enterprise_relationships.unshift(data) @create_errors = "" diff --git a/app/views/admin/enterprise_relationships/_form.html.haml b/app/views/admin/enterprise_relationships/_form.html.haml index 1a737ec3a5..abb96f3c8f 100644 --- a/app/views/admin/enterprise_relationships/_form.html.haml +++ b/app/views/admin/enterprise_relationships/_form.html.haml @@ -1,7 +1,11 @@ %tr %td %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} - %td permits + %td + %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} + %label + %input{type: "checkbox", "ng-model" => "permissions[permission]"} + {{ EnterpriseRelationships.permission_presentation(permission) }} %td %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} %td.actions diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 112529376a..7eadf37ffa 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -38,11 +38,16 @@ feature %q{ visit admin_enterprise_relationships_path select 'One', from: 'enterprise_relationship_parent_id' + check 'can add products to order cycle from' + check 'can manage the products of' + uncheck 'can manage the products of' select 'Two', from: 'enterprise_relationship_child_id' click_button 'Create' - page.should have_relationship e1, e2 - EnterpriseRelationship.where(parent_id: e1, child_id: e2).should be_present + page.should have_relationship e1, e2, ['can add products to order cycle from'] + er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first + er.should be_present + er.permissions.map(&:name).should == ['add_products_to_order_cycle'] end From c3224ce668a1ab61cceb76f91477ab386dc1b6fc Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 15:05:49 +1000 Subject: [PATCH 12/46] Style permission list items, order perms consistently by name --- app/assets/stylesheets/admin/relationships.css.sass | 3 +++ app/models/enterprise_relationship_permission.rb | 1 + 2 files changed, 4 insertions(+) diff --git a/app/assets/stylesheets/admin/relationships.css.sass b/app/assets/stylesheets/admin/relationships.css.sass index a6467958b7..a5b8879125 100644 --- a/app/assets/stylesheets/admin/relationships.css.sass +++ b/app/assets/stylesheets/admin/relationships.css.sass @@ -8,6 +8,9 @@ table#enterprise-relationships, table#enterprise-roles + ul + list-style-type: none + th.actions, td.actions width: 16% .errors diff --git a/app/models/enterprise_relationship_permission.rb b/app/models/enterprise_relationship_permission.rb index f615a91da7..0833c1386e 100644 --- a/app/models/enterprise_relationship_permission.rb +++ b/app/models/enterprise_relationship_permission.rb @@ -1,2 +1,3 @@ class EnterpriseRelationshipPermission < ActiveRecord::Base + default_scope order('name') end From 1a995aeddabfdb4be9fcbe50f48276b9ceae1127 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 15:20:46 +1000 Subject: [PATCH 13/46] Simplify enterprise_relationship factory - leverage permissions_list= model method --- spec/factories.rb | 6 ------ spec/features/admin/enterprise_relationships_spec.rb | 6 +++--- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/spec/factories.rb b/spec/factories.rb index 39b3a01035..849d38abf3 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -101,12 +101,6 @@ FactoryGirl.define do end factory :enterprise_relationship do - ignore { permissions [] } - after(:create) do |er, proxy| - proxy.permissions.each do |name| - er.permissions.create! name: name - end - end end factory :enterprise_role do diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 7eadf37ffa..20c7b28ac2 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -14,9 +14,9 @@ feature %q{ scenario "listing relationships" do # Given some enterprises with relationships e1, e2, e3, e4 = create(:enterprise), create(:enterprise), create(:enterprise), create(:enterprise) - create(:enterprise_relationship, parent: e1, child: e2, permissions: [:add_products_to_order_cycle]) - create(:enterprise_relationship, parent: e2, child: e3, permissions: [:manage_products]) - create(:enterprise_relationship, parent: e3, child: e4, permissions: [:add_products_to_order_cycle, :manage_products]) + create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [:add_products_to_order_cycle]) + create(:enterprise_relationship, parent: e2, child: e3, permissions_list: [:manage_products]) + create(:enterprise_relationship, parent: e3, child: e4, permissions_list: [:add_products_to_order_cycle, :manage_products]) # When I go to the relationships page click_link 'Enterprises' From 45a44844ca0cba0bc893edc2aa566d472ded5fb3 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 25 Aug 2014 16:38:17 +1000 Subject: [PATCH 14/46] Remove old rabl spec --- .../enterprise_relationships_rabl_spec.rb | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 spec/views/admin/json/enterprise_relationships_rabl_spec.rb diff --git a/spec/views/admin/json/enterprise_relationships_rabl_spec.rb b/spec/views/admin/json/enterprise_relationships_rabl_spec.rb deleted file mode 100644 index 9b72a0d45b..0000000000 --- a/spec/views/admin/json/enterprise_relationships_rabl_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe "admin/json/_enterprise_relationships.json.rabl" do - let(:parent) { create(:enterprise) } - let(:child) { create(:enterprise) } - let(:enterprise_relationship) { create(:enterprise_relationship, parent: parent, child: child) } - let(:render) { Rabl.render([enterprise_relationship], 'admin/json/enterprise_relationships', view_path: 'app/views', scope: RablHelper::FakeContext.instance) } - - it "renders a list of enterprise relationships" do - render.should have_json_type(Array).at_path '' - render.should have_json_type(Object).at_path '0' - end - - it "renders enterprise ids" do - render.should be_json_eql(parent.id).at_path '0/parent_id' - render.should be_json_eql(child.id).at_path '0/child_id' - end - - it "renders enterprise names" do - render.should be_json_eql(parent.name.to_json).at_path '0/parent_name' - render.should be_json_eql(child.name.to_json).at_path '0/child_name' - end -end From 0d9e07d4849503402ea701a431b746024e15094c Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 15 Aug 2014 15:10:44 +1000 Subject: [PATCH 15/46] Make restore script compatible with OSX --- script/restore.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/restore.sh b/script/restore.sh index 00457225ac..ea87a84631 100755 --- a/script/restore.sh +++ b/script/restore.sh @@ -6,4 +6,4 @@ set -e echo "drop database open_food_network_dev" | psql -h localhost -U ofn open_food_network_test echo "create database open_food_network_dev" | psql -h localhost -U ofn open_food_network_test -zcat $1 |psql -h localhost -U ofn open_food_network_dev +gunzip -c $1 |psql -h localhost -U ofn open_food_network_dev From 310d1b37260f49588c4302eaf529d5834005adb9 Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 15 Aug 2014 18:15:01 +1000 Subject: [PATCH 16/46] Zeus server does not crash when editing controllers --- app/controllers/spree/admin/reports_controller_decorator.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 28174724dd..18d9bffbdd 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -4,11 +4,8 @@ require 'open_food_network/products_and_inventory_report' require 'open_food_network/group_buy_report' require 'open_food_network/order_grouper' require 'open_food_network/customers_report' -require 'open_food_network/model_class_from_controller_name' Spree::Admin::ReportsController.class_eval do - include OpenFoodNetwork::ModelClassFromControllerName - # Fetches user's distributors, suppliers and order_cycles before_filter :load_data, only: [:customers, :products_and_inventory] From 435819acc4b8ee702242fe3f88903f99f2352eff Mon Sep 17 00:00:00 2001 From: Rob H Date: Mon, 25 Aug 2014 17:57:33 +1000 Subject: [PATCH 17/46] Removing unit text from total units column --- .../admin/reports_controller_decorator.rb | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 18d9bffbdd..0abfd783b5 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -6,7 +6,7 @@ require 'open_food_network/order_grouper' require 'open_food_network/customers_report' Spree::Admin::ReportsController.class_eval do - # Fetches user's distributors, suppliers and order_cycles + # Fetches user's distributors, suppliers and order_cycles before_filter :load_data, only: [:customers, :products_and_inventory] # Render a partial for orders and fulfillment description @@ -362,9 +362,9 @@ Spree::Admin::ReportsController.class_eval do 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_distributors = Enterprise.is_distributor.managed_by(spree_current_user) my_suppliers = Enterprise.is_primary_producer.managed_by(spree_current_user) # My distributors and any distributors distributing products I supply @@ -385,21 +385,11 @@ Spree::Admin::ReportsController.class_eval do header = ["Producer", "Product", "Variant", "Amount", "Total Units", "Curr. Cost per Unit", "Total Cost", "Status", "Incoming Transport"] - ovn = OpenFoodNetwork::OptionValueNamer.new() - columns = [ proc { |line_items| line_items.first.variant.product.supplier.name }, proc { |line_items| line_items.first.variant.product.name }, proc { |line_items| line_items.first.variant.full_name }, proc { |line_items| line_items.sum { |li| li.quantity } }, - proc { |line_items| ovn.name(OpenStruct.new({ - unit_value: ( line_items.map{ |li| li.variant.unit_value.nil? }.any? ? 0 : line_items.sum { |li| li.quantity * li.variant.unit_value } ), - unit_description: line_items.first.variant.unit_description, - product: OpenStruct.new({ - variant_unit: line_items.first.product.variant_unit, - variant_unit_scale: line_items.first.product.variant_unit_scale, - variant_unit_name: line_items.first.product.variant_unit_name - }) - }))}, + proc { |line_items| total_units(line_items) }, proc { |line_items| line_items.first.variant.price }, proc { |line_items| line_items.sum { |li| li.quantity * li.price } }, proc { |line_items| "" }, @@ -604,10 +594,10 @@ Spree::Admin::ReportsController.class_eval do def load_data my_distributors = Enterprise.is_distributor.managed_by(spree_current_user) my_suppliers = Enterprise.is_primary_producer.managed_by(spree_current_user) - distributors_of_my_products = Enterprise.with_distributed_products_outer.merge(Spree::Product.in_any_supplier(my_suppliers)) - @distributors = my_distributors | distributors_of_my_products + distributors_of_my_products = Enterprise.with_distributed_products_outer.merge(Spree::Product.in_any_supplier(my_suppliers)) + @distributors = my_distributors | distributors_of_my_products suppliers_of_products_I_distribute = my_distributors.map { |d| Spree::Product.in_distributor(d) }.flatten.map(&:supplier).uniq - @suppliers = my_suppliers | suppliers_of_products_I_distribute + @suppliers = my_suppliers | suppliers_of_products_I_distribute @order_cycles = OrderCycle.active_or_complete.accessible_by(spree_current_user).order('orders_close_at DESC') end @@ -625,4 +615,13 @@ Spree::Admin::ReportsController.class_eval do end reports end + + def total_units(line_items) + return " " if line_items.map{ |li| li.variant.unit_value.nil? }.any? + total_units = line_items.sum do |li| + scale_factor = ( li.product.variant_unit == 'weight' ? 1000 : 1 ) + li.quantity * li.variant.unit_value / scale_factor + end + total_units.round(3) + end end From 0462b3e55d83084c8f21f4b94d1eec8fbbfe2f58 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 14:40:34 +1000 Subject: [PATCH 18/46] Prevent duplicate enterprise roles --- .../20140826043521_prevent_duplicate_enterprise_roles.rb | 5 +++++ db/schema.rb | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20140826043521_prevent_duplicate_enterprise_roles.rb diff --git a/db/migrate/20140826043521_prevent_duplicate_enterprise_roles.rb b/db/migrate/20140826043521_prevent_duplicate_enterprise_roles.rb new file mode 100644 index 0000000000..5b4fc6e7fc --- /dev/null +++ b/db/migrate/20140826043521_prevent_duplicate_enterprise_roles.rb @@ -0,0 +1,5 @@ +class PreventDuplicateEnterpriseRoles < ActiveRecord::Migration + def change + add_index :enterprise_roles, [:enterprise_id, :user_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 316c341576..dbb50d6b33 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20140825023227) do +ActiveRecord::Schema.define(:version => 20140826043521) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -228,6 +228,7 @@ ActiveRecord::Schema.define(:version => 20140825023227) do t.integer "enterprise_id" end + add_index "enterprise_roles", ["enterprise_id", "user_id"], :name => "index_enterprise_roles_on_enterprise_id_and_user_id", :unique => true add_index "enterprise_roles", ["enterprise_id"], :name => "index_enterprise_roles_on_enterprise_id" add_index "enterprise_roles", ["user_id", "enterprise_id"], :name => "index_enterprise_roles_on_user_id_and_enterprise_id", :unique => true add_index "enterprise_roles", ["user_id"], :name => "index_enterprise_roles_on_user_id" @@ -562,9 +563,9 @@ ActiveRecord::Schema.define(:version => 20140825023227) do t.string "email" t.text "special_instructions" t.integer "distributor_id" + t.integer "order_cycle_id" t.string "currency" t.string "last_ip_address" - t.integer "order_cycle_id" t.integer "cart_id" end From 400f2ea9b9a72469135e34bfe28f4bbbaa8f1360 Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 27 Aug 2014 15:09:41 +1000 Subject: [PATCH 19/46] Don't add payment forms to checkout DOM unless required --- .../checkout/payment_controller.js.coffee | 4 ++-- .../darkswarm/mixins/fieldset_mixin.js.coffee | 9 ++++----- app/views/checkout/_form.html.haml | 10 +++++----- app/views/checkout/_payment.html.haml | 12 ++++++------ app/views/checkout/edit.html.haml | 8 ++++---- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/checkout/payment_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/checkout/payment_controller.js.coffee index 9ae7c14b90..71f893aa64 100644 --- a/app/assets/javascripts/darkswarm/controllers/checkout/payment_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/checkout/payment_controller.js.coffee @@ -6,7 +6,7 @@ Darkswarm.controller "PaymentCtrl", ($scope, $timeout) -> {key: "January", value: "1"}, {key: "February", value: "2"}, {key: "March", value: "3"}, - {key: "April", value: "4"}, + {key: "April", value: "4"}, {key: "May", value: "5"}, {key: "June", value: "6"}, {key: "July", value: "7"}, @@ -20,4 +20,4 @@ Darkswarm.controller "PaymentCtrl", ($scope, $timeout) -> $scope.years = [moment().year()..(moment().year()+15)] $scope.secrets.card_month = "1" $scope.secrets.card_year = moment().year() - $timeout $scope.onTimeout + $timeout $scope.onTimeout diff --git a/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee b/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee index 26b7ac3518..b9f59e93a8 100644 --- a/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee +++ b/app/assets/javascripts/darkswarm/mixins/fieldset_mixin.js.coffee @@ -1,7 +1,7 @@ window.FieldsetMixin = ($scope)-> $scope.next = (event = false)-> event.preventDefault() if event - $scope.show $scope.nextPanel + $scope.show $scope.nextPanel $scope.onTimeout = -> if $scope[$scope.name].$valid @@ -36,7 +36,6 @@ window.FieldsetMixin = ($scope)-> when "number" then "must be number" when "email" then "must be email address" - #server_errors = $scope.Order.errors[path.replace('order.', '')] - #errors.push server_errors if server_errors? - (errors.filter (error) -> error?).join ", " - + #server_errors = $scope.Order.errors[path.replace('order.', '')] + #errors.push server_errors if server_errors? + (errors.filter (error) -> error?).join ", " \ No newline at end of file diff --git a/app/views/checkout/_form.html.haml b/app/views/checkout/_form.html.haml index aa2312ccee..a64be0ac44 100644 --- a/app/views/checkout/_form.html.haml +++ b/app/views/checkout/_form.html.haml @@ -1,5 +1,5 @@ -= f_form_for current_order, url: main_app.update_checkout_path, - html: {name: "checkout", += f_form_for current_order, url: main_app.update_checkout_path, + html: {name: "checkout", id: "checkout_form", novalidate: true, name: "checkout"} do |f| @@ -10,8 +10,8 @@ angular.module('Darkswarm').value('order', #{render "checkout/order"}) %div - / %h3.text-center.pad-top - / Checkout from + / %h3.text-center.pad-top + / Checkout from / = current_distributor.name = render partial: "checkout/details", locals: {f: f} @@ -19,7 +19,7 @@ = render partial: "checkout/shipping", locals: {f: f} = render partial: "checkout/payment", locals: {f: f} %p - %button.button.primary{type: :submit, + %button.button.primary{type: :submit, "ng-click" => "purchase($event)", "ng-disabled" => "checkout.$invalid"} Place order now diff --git a/app/views/checkout/_payment.html.haml b/app/views/checkout/_payment.html.haml index e6a0fd373c..b655659738 100644 --- a/app/views/checkout/_payment.html.haml +++ b/app/views/checkout/_payment.html.haml @@ -1,7 +1,7 @@ %fieldset#payment %ng-form{"ng-controller" => "PaymentCtrl", name: "payment"} - %h5{"ng-class" => "{valid: payment.$valid, dirty: payment.$dirty}"} + %h5{"ng-class" => "{valid: payment.$valid, dirty: payment.$dirty}"} %span.right %label.label.round.alert.right %i.ofn-i_009-close @@ -14,16 +14,16 @@ %accordion-heading .row .small-8.medium-10.columns - %em + %em %small {{ Checkout.paymentMethod().name }} .small-4.medium-2.columns.text-right %span.accordion-up - %em + %em %small Hide - %i.ofn-i_053-point-up + %i.ofn-i_053-point-up %span.accordion-down - %em + %em %small Expand %i.ofn-i_052-point-down @@ -38,7 +38,7 @@ "ng-model" => "order.payment_method_id" = method.name - .row{"ng-show" => "order.payment_method_id == #{method.id}"} + .row{"ng-if" => "order.payment_method_id == #{method.id}"} .small-12.columns = render partial: "spree/checkout/payment/#{method.method_type}", :locals => { :payment_method => method } diff --git a/app/views/checkout/edit.html.haml b/app/views/checkout/edit.html.haml index 7fffdbeb22..2744627d03 100644 --- a/app/views/checkout/edit.html.haml +++ b/app/views/checkout/edit.html.haml @@ -1,16 +1,16 @@ -= inject_enterprises += inject_enterprises .darkswarm - content_for :order_cycle_form do - + %closing Checkout now - %p + %p Order ready for %strong = pickup_time current_order_cycle = render partial: "shopping_shared/details" - + %accordion{"close-others" => "true"} %checkout.row{"ng-controller" => "CheckoutCtrl"} .small-12.medium-8.large-9.columns From 7124dc57fdb1ee98b4e290ca09afd5fecc03e3d2 Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 27 Aug 2014 16:59:26 +1000 Subject: [PATCH 20/46] Requiring a state in checkout --- app/views/checkout/_billing.html.haml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/checkout/_billing.html.haml b/app/views/checkout/_billing.html.haml index 2fceedb523..1e7adec1ad 100644 --- a/app/views/checkout/_billing.html.haml +++ b/app/views/checkout/_billing.html.haml @@ -19,13 +19,13 @@ {{ summary() | printArray }} .small-4.medium-2.columns.text-right %span.accordion-up - %em + %em %small Hide - %i.ofn-i_053-point-up + %i.ofn-i_053-point-up %span.accordion-down - %em + %em %small Expand - %i.ofn-i_052-point-down + %i.ofn-i_052-point-down = f.fields_for :bill_address, @order.bill_address do |ba| .row @@ -40,7 +40,7 @@ .small-6.columns = ba.select :state_id, @order.billing_address.country.states.map{|c|[c.name, c.id]}, {include_blank: false}, - "ng-model" => "order.bill_address.state_id" + "ng-model" => "order.bill_address.state_id", required: true .row .small-6.columns = validated_input "Postcode", "order.bill_address.zipcode" From a4be0ff55a2ef4cb7dd78412ca6a5018db6ab167 Mon Sep 17 00:00:00 2001 From: Rob H Date: Wed, 27 Aug 2014 17:07:30 +1000 Subject: [PATCH 21/46] Comment out ERPs which have confusing names, use old 'permits' --- app/views/admin/enterprise_relationships/_form.html.haml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/views/admin/enterprise_relationships/_form.html.haml b/app/views/admin/enterprise_relationships/_form.html.haml index abb96f3c8f..b9c2f265cd 100644 --- a/app/views/admin/enterprise_relationships/_form.html.haml +++ b/app/views/admin/enterprise_relationships/_form.html.haml @@ -2,10 +2,11 @@ %td %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} %td - %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} - %label - %input{type: "checkbox", "ng-model" => "permissions[permission]"} - {{ EnterpriseRelationships.permission_presentation(permission) }} + permits + / %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} + / %label + / %input{type: "checkbox", "ng-model" => "permissions[permission]"} + / {{ EnterpriseRelationships.permission_presentation(permission) }} %td %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} %td.actions From 5e8bdce67df7bcc9a1307df481fde4d4157326ce Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 09:10:39 +1000 Subject: [PATCH 22/46] Refactor spec --- app/helpers/order_cycles_helper.rb | 2 +- spec/features/admin/order_cycles_spec.rb | 28 ++++++++++-------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index 92cd968e0b..3fc36bfc4d 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -4,7 +4,7 @@ module OrderCyclesHelper end def coordinating_enterprises - Enterprise.is_distributor.managed_by(spree_current_user).order('name') + Enterprise.is_distributor.managed_by(spree_current_user).by_name end def order_cycle_local_remote_class(distributor, order_cycle) diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 2423a6dc05..d8adbd0226 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -436,12 +436,13 @@ feature %q{ context "as an enterprise user" do - let(:supplier1) { create(:supplier_enterprise, name: 'First Supplier') } - let(:supplier2) { create(:supplier_enterprise, name: 'Another Supplier') } - let(:distributor1) { create(:distributor_enterprise, name: 'First Distributor') } - let(:distributor2) { create(:distributor_enterprise, name: 'Another Distributor') } + let!(:supplier1) { create(:supplier_enterprise, name: 'First Supplier') } + let!(:supplier2) { create(:supplier_enterprise, name: 'Another Supplier') } + let!(:distributor1) { create(:distributor_enterprise, name: 'First Distributor') } + let!(:distributor2) { create(:distributor_enterprise, name: 'Another Distributor') } let!(:distributor1_fee) { create(:enterprise_fee, enterprise: distributor1, name: 'First Distributor Fee') } - before(:each) do + + before do product = create(:product, supplier: supplier1) product.distributors << distributor1 product.save! @@ -486,17 +487,12 @@ feature %q{ select 'First Distributor', from: 'new_distributor_id' click_button 'Add distributor' - # Should only have suppliers / distributors listed which the user can manage - within "#new_supplier_id" do - page.should_not have_content supplier2.name - end - within "#new_distributor_id" do - page.should_not have_content distributor2.name - end - within "#order_cycle_coordinator_id" do - page.should_not have_content distributor2.name - page.should_not have_content supplier1.name - page.should_not have_content supplier2.name + # Should only have suppliers / distributors listed which the user is managing + page.should_not have_select 'new_supplier_id', with_options: [supplier2.name] + page.should_not have_select 'new_distributor_id', with_options: [distributor2.name] + + [distributor2.name, supplier1.name, supplier2.name].each do |enterprise_name| + page.should_not have_select 'order_cycle_coordinator_id', with_options: [enterprise_name] end click_button 'Create' From a7689973bef9ddb1c617d43b34e7689fe5b9e702 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 09:23:48 +1000 Subject: [PATCH 23/46] Semantically name enterprises in spec --- spec/features/admin/order_cycles_spec.rb | 55 ++++++++++++------------ 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index d8adbd0226..8bbd544f16 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -436,27 +436,27 @@ feature %q{ context "as an enterprise user" do - let!(:supplier1) { create(:supplier_enterprise, name: 'First Supplier') } - let!(:supplier2) { create(:supplier_enterprise, name: 'Another Supplier') } - let!(:distributor1) { create(:distributor_enterprise, name: 'First Distributor') } - let!(:distributor2) { create(:distributor_enterprise, name: 'Another Distributor') } - let!(:distributor1_fee) { create(:enterprise_fee, enterprise: distributor1, name: 'First Distributor Fee') } + let!(:supplier_managed) { create(:supplier_enterprise, name: 'Managed supplier') } + let!(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Unmanaged supplier') } + let!(:distributor_managed) { create(:distributor_enterprise, name: 'Managed distributor') } + let!(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Unmanaged Distributor') } + let!(:distributor_managed_fee) { create(:enterprise_fee, enterprise: distributor_managed, name: 'Managed distributor Fee') } before do - product = create(:product, supplier: supplier1) - product.distributors << distributor1 + product = create(:product, supplier: supplier_managed) + product.distributors << distributor_managed product.save! @new_user = create_enterprise_user - @new_user.enterprise_roles.build(enterprise: supplier1).save - @new_user.enterprise_roles.build(enterprise: distributor1).save + @new_user.enterprise_roles.build(enterprise: supplier_managed).save + @new_user.enterprise_roles.build(enterprise: distributor_managed).save login_to_admin_as @new_user end scenario "viewing a list of order cycles I am coordinating" do - oc_user_coordinating = create(:simple_order_cycle, { suppliers: [supplier1, supplier2], coordinator: supplier1, distributors: [distributor1, distributor2], name: 'Order Cycle 1' } ) - oc_for_other_user = create(:simple_order_cycle, { coordinator: supplier2, name: 'Order Cycle 2' } ) + oc_user_coordinating = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_unmanaged], name: 'Order Cycle 1' } ) + oc_for_other_user = create(:simple_order_cycle, { coordinator: supplier_unmanaged, name: 'Order Cycle 2' } ) click_link "Order Cycles" @@ -465,8 +465,8 @@ feature %q{ page.should_not have_content oc_for_other_user.name # The order cycle should not show enterprises that I don't manage - page.should_not have_selector 'td.suppliers', text: supplier2.name - page.should_not have_selector 'td.distributors', text: distributor2.name + page.should_not have_selector 'td.suppliers', text: supplier_unmanaged.name + page.should_not have_selector 'td.distributors', text: distributor_unmanaged.name end scenario "creating a new order cycle" do @@ -477,21 +477,22 @@ feature %q{ fill_in 'order_cycle_orders_open_at', with: '2012-11-06 06:00:00' fill_in 'order_cycle_orders_close_at', with: '2012-11-13 17:00:00' - select 'First Supplier', from: 'new_supplier_id' + select 'Managed supplier', from: 'new_supplier_id' click_button 'Add supplier' - select 'First Distributor', from: 'order_cycle_coordinator_id' + select 'Managed distributor', from: 'order_cycle_coordinator_id' click_button 'Add coordinator fee' - select 'First Distributor Fee', from: 'order_cycle_coordinator_fee_0_id' + select 'Managed distributor Fee', from: 'order_cycle_coordinator_fee_0_id' - select 'First Distributor', from: 'new_distributor_id' + select 'Managed distributor', from: 'new_distributor_id' click_button 'Add distributor' - # Should only have suppliers / distributors listed which the user is managing - page.should_not have_select 'new_supplier_id', with_options: [supplier2.name] - page.should_not have_select 'new_distributor_id', with_options: [distributor2.name] + # Should only have suppliers / distributors listed which the user is managing or + # has E2E permission to add products to order cycles + page.should_not have_select 'new_supplier_id', with_options: [supplier_unmanaged.name] + page.should_not have_select 'new_distributor_id', with_options: [distributor_unmanaged.name] - [distributor2.name, supplier1.name, supplier2.name].each do |enterprise_name| + [distributor_unmanaged.name, supplier_managed.name, supplier_unmanaged.name].each do |enterprise_name| page.should_not have_select 'order_cycle_coordinator_id', with_options: [enterprise_name] end @@ -499,15 +500,15 @@ feature %q{ flash_message.should == "Your order cycle has been created." order_cycle = OrderCycle.find_by_name('My order cycle') - order_cycle.coordinator.should == distributor1 + order_cycle.coordinator.should == distributor_managed end scenario "editing an order cycle" do - oc = create(:simple_order_cycle, { suppliers: [supplier1, supplier2], coordinator: supplier1, distributors: [distributor1, distributor2], name: 'Order Cycle 1' } ) + oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_unmanaged], name: 'Order Cycle 1' } ) visit edit_admin_order_cycle_path(oc) - # I should not see exchanges for supplier2 or distributor2 + # I should not see exchanges for supplier_unmanaged or distributor_unmanaged page.all('tr.supplier').count.should == 1 page.all('tr.distributor').count.should == 1 @@ -516,9 +517,9 @@ feature %q{ page.should have_content "Your order cycle has been updated." oc.reload - oc.suppliers.sort.should == [supplier1, supplier2] - oc.coordinator.should == supplier1 - oc.distributors.sort.should == [distributor1, distributor2] + oc.suppliers.sort.should == [supplier_managed, supplier_unmanaged] + oc.coordinator.should == supplier_managed + oc.distributors.sort.should == [distributor_managed, distributor_unmanaged] end scenario "cloning an order cycle" do From 8548a1a67e0531ca29bf660338b23f088ebaf947 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 09:51:18 +1000 Subject: [PATCH 24/46] Determine producer options on order cycle screen through OpenFoodNetwork::Permissions class --- .../admin/order_cycles_controller.rb | 1 + app/views/admin/order_cycles/_form.html.haml | 2 +- lib/open_food_network/permissions.rb | 19 +++++++++++++++++++ .../lib/open_food_network/permissions_spec.rb | 16 ++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 lib/open_food_network/permissions.rb create mode 100644 spec/lib/open_food_network/permissions_spec.rb diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 566c660112..58c04a63bf 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -1,3 +1,4 @@ +require 'open_food_network/permissions' require 'open_food_network/order_cycle_form_applicator' module Admin diff --git a/app/views/admin/order_cycles/_form.html.haml b/app/views/admin/order_cycles/_form.html.haml index 68dfed9f1a..4087ebcfd5 100644 --- a/app/views/admin/order_cycles/_form.html.haml +++ b/app/views/admin/order_cycles/_form.html.haml @@ -25,7 +25,7 @@ %tr.products{'ng-show' => 'exchange.showProducts'} = render 'exchange_supplied_products_form' -= select_tag :new_supplier_id, options_from_collection_for_select(Enterprise.is_primary_producer.managed_by(spree_current_user).by_name, :id, :name), {'ng-model' => 'new_supplier_id'} += select_tag :new_supplier_id, options_from_collection_for_select(OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_producers, :id, :name), {'ng-model' => 'new_supplier_id'} = f.submit 'Add supplier', 'ng-click' => 'addSupplier($event)' diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb new file mode 100644 index 0000000000..80ceba55ee --- /dev/null +++ b/lib/open_food_network/permissions.rb @@ -0,0 +1,19 @@ +module OpenFoodNetwork + class Permissions + def initialize(user) + @user = user + end + + def order_cycle_producers + managed_producers + end + + + private + + def managed_producers + Enterprise.managed_by(@user).is_primary_producer.by_name + end + + end +end diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb new file mode 100644 index 0000000000..0c0449c145 --- /dev/null +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -0,0 +1,16 @@ +require 'open_food_network/permissions' + +module OpenFoodNetwork + describe Permissions do + let(:user) { double(:user) } + let(:permissions) { Permissions.new(user) } + let(:producer) { double(:enterprise) } + + describe "finding producers that can be added to an order cycle" do + it "returns managed producers" do + permissions.stub(:managed_producers) { [producer] } + permissions.order_cycle_producers.should == [producer] + end + end + end +end From b9e582149785b39a571262641d2c2e34608536d8 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 10:32:31 +1000 Subject: [PATCH 25/46] Add EnterpriseRelationship scopes permitting and with_permission --- app/models/enterprise_relationship.rb | 10 +++++++++- spec/models/enterprise_relationship_spec.rb | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 5c10aafbbf..8e2f3bcf06 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -9,12 +9,20 @@ class EnterpriseRelationship < ActiveRecord::Base scope :with_enterprises, joins('LEFT JOIN enterprises AS parent_enterprises ON parent_enterprises.id = enterprise_relationships.parent_id'). joins('LEFT JOIN enterprises AS child_enterprises ON child_enterprises.id = enterprise_relationships.child_id') - scope :by_name, with_enterprises.order('parent_enterprises.name, child_enterprises.name') scope :involving_enterprises, ->(enterprises) { where('parent_id IN (?) OR child_id IN (?)', enterprises, enterprises) } + scope :permitting, ->(enterprises) { where('child_id IN (?)', enterprises) } + + scope :with_permission, ->(permission) { + joins(:permissions). + where('enterprise_relationship_permissions.name = ?', permission) + } + + scope :by_name, with_enterprises.order('parent_enterprises.name, child_enterprises.name') + def permissions_list=(perms) perms.andand.each { |name| permissions.build name: name } diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index 6cdf3db657..3201e1db5c 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -43,5 +43,24 @@ describe EnterpriseRelationship do er.permissions.should be_empty end end + + it "finds relationships that grant permissions to some enterprises" do + er1 = create(:enterprise_relationship, parent: e2, child: e1) + er2 = create(:enterprise_relationship, parent: e3, child: e2) + er3 = create(:enterprise_relationship, parent: e1, child: e3) + + EnterpriseRelationship.permitting([e1, e2]).sort.should == [er1, er2] + end + + it "finds relationships that grant a particular permission" do + er1 = create(:enterprise_relationship, parent: e1, child: e2, + permissions_list: ['one', 'two']) + er2 = create(:enterprise_relationship, parent: e2, child: e3, + permissions_list: ['two', 'three']) + er3 = create(:enterprise_relationship, parent: e3, child: e1, + permissions_list: ['three', 'four']) + + EnterpriseRelationship.with_permission('two').sort.should == [er1, er2].sort + end end end From 34602244caa529b627242075af18f561652b5d3b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 11:03:16 +1000 Subject: [PATCH 26/46] Show permitted suppliers in order cycle add supplier select box --- lib/open_food_network/permissions.rb | 23 ++++++++++-- spec/features/admin/order_cycles_spec.rb | 11 +++++- .../lib/open_food_network/permissions_spec.rb | 37 +++++++++++++++++-- 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 80ceba55ee..a961a3f779 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -5,15 +5,32 @@ module OpenFoodNetwork end def order_cycle_producers - managed_producers + (managed_producers + related_producers_with(:add_products_to_order_cycle)). + sort_by(&:name) end private - def managed_producers - Enterprise.managed_by(@user).is_primary_producer.by_name + def managed_enterprises + Enterprise.managed_by(@user) end + def managed_producers + managed_enterprises.is_primary_producer.by_name + 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 related_producers_with(permission) + related_enterprises_with(permission).is_primary_producer + end end end diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 8bbd544f16..9aad54f3d6 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -438,9 +438,14 @@ feature %q{ let!(:supplier_managed) { create(:supplier_enterprise, name: 'Managed supplier') } let!(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Unmanaged supplier') } + let!(:supplier_permitted) { create(:supplier_enterprise, name: 'Permitted supplier') } let!(:distributor_managed) { create(:distributor_enterprise, name: 'Managed distributor') } let!(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Unmanaged Distributor') } - let!(:distributor_managed_fee) { create(:enterprise_fee, enterprise: distributor_managed, name: 'Managed distributor Fee') } + let!(:distributor_managed_fee) { create(:enterprise_fee, enterprise: distributor_managed, name: 'Managed distributor fee') } + let!(:supplier_permitted_relationship) do + create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed, + permissions_list: [:add_products_to_order_cycle]) + end before do product = create(:product, supplier: supplier_managed) @@ -479,10 +484,12 @@ feature %q{ select 'Managed supplier', from: 'new_supplier_id' click_button 'Add supplier' + select 'Permitted supplier', from: 'new_supplier_id' + click_button 'Add supplier' select 'Managed distributor', from: 'order_cycle_coordinator_id' click_button 'Add coordinator fee' - select 'Managed distributor Fee', from: 'order_cycle_coordinator_fee_0_id' + select 'Managed distributor fee', from: 'order_cycle_coordinator_fee_0_id' select 'Managed distributor', from: 'new_distributor_id' click_button 'Add distributor' diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 0c0449c145..573a6e91f3 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -4,12 +4,41 @@ module OpenFoodNetwork describe Permissions do let(:user) { double(:user) } let(:permissions) { Permissions.new(user) } - let(:producer) { double(:enterprise) } + let(:permission) { 'one' } + let(:e1) { create(:enterprise) } + let(:e2) { create(:enterprise) } describe "finding producers that can be added to an order cycle" do - it "returns managed producers" do - permissions.stub(:managed_producers) { [producer] } - permissions.order_cycle_producers.should == [producer] + let(:producer1) { double(:enterprise1, name: 'A') } + let(:producer2) { double(:enterprise2, name: 'B') } + let(:producer3) { double(:enterprise3, name: 'C') } + + it "returns managed producers and related+permitted enterprises, sorted by name" do + permissions.stub(:managed_producers) { [producer1, producer3] } + permissions.stub(:related_producers_with) { [producer2] } + + permissions.order_cycle_producers.should == [producer1, producer2, producer3] + end + end + + describe "finding related enterprises with a particular permission" do + let!(:er) { create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [permission]) } + + it "returns the enterprises" do + permissions.stub(:managed_enterprises) { e2 } + permissions.send(:related_enterprises_with, 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 == [] + end + end + + describe "finding related producers with a particular permission" do + it "returns permitted related enterprises that are also producers" do + permissions.stub_chain(:related_enterprises_with, :is_primary_producer) { [e1] } + permissions.send(:related_producers_with, permission).should == [e1] end end end From 099a5b0b7b32455f3441342c14cbd9d3b8b3af71 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 11:38:31 +1000 Subject: [PATCH 27/46] Show E2E related enterprise exchanges in OC --- app/views/admin/order_cycles/show.rep | 2 +- lib/open_food_network/permissions.rb | 7 ++++ spec/features/admin/order_cycles_spec.rb | 10 +++--- .../lib/open_food_network/permissions_spec.rb | 32 +++++++++++++++++++ 4 files changed, 45 insertions(+), 6 deletions(-) diff --git a/app/views/admin/order_cycles/show.rep b/app/views/admin/order_cycles/show.rep index 04fc659813..fb71602eb6 100644 --- a/app/views/admin/order_cycles/show.rep +++ b/app/views/admin/order_cycles/show.rep @@ -9,7 +9,7 @@ r.element :order_cycle, @order_cycle do r.element :id end - r.list_of :exchanges, @order_cycle.exchanges.managed_by(spree_current_user).order('id ASC') do |exchange| + r.list_of :exchanges, OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_exchanges(@order_cycle).order('id ASC') do |exchange| r.element :id r.element :sender_id r.element :receiver_id diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index a961a3f779..55ec1b74a8 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -4,11 +4,18 @@ module OpenFoodNetwork @user = user end + # Find producers for which an admin is allowed to add their products to an order cycle def order_cycle_producers (managed_producers + related_producers_with(:add_products_to_order_cycle)). sort_by(&:name) end + # Find the exchanges of an order cycle that an admin can manage + def order_cycle_exchanges(order_cycle) + enterprises = managed_enterprises + related_enterprises_with(:add_products_to_order_cycle) + order_cycle.exchanges.to_enterprises(enterprises).from_enterprises(enterprises) + end + private diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 9aad54f3d6..26488fd9f4 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -511,12 +511,12 @@ feature %q{ end scenario "editing an order cycle" do - oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_unmanaged], name: 'Order Cycle 1' } ) + oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_unmanaged], name: 'Order Cycle 1' } ) visit edit_admin_order_cycle_path(oc) # I should not see exchanges for supplier_unmanaged or distributor_unmanaged - page.all('tr.supplier').count.should == 1 + page.all('tr.supplier').count.should == 2 page.all('tr.distributor').count.should == 1 # When I save, then those exchanges should remain @@ -524,13 +524,13 @@ feature %q{ page.should have_content "Your order cycle has been updated." oc.reload - oc.suppliers.sort.should == [supplier_managed, supplier_unmanaged] + oc.suppliers.sort.should == [supplier_managed, supplier_permitted, supplier_unmanaged].sort oc.coordinator.should == supplier_managed - oc.distributors.sort.should == [distributor_managed, distributor_unmanaged] + oc.distributors.sort.should == [distributor_managed, distributor_unmanaged].sort end scenario "cloning an order cycle" do - oc = create(:simple_order_cycle) + oc = create(:simple_order_cycle, coordinator: distributor_managed) click_link "Order Cycles" first('a.clone-order-cycle').click diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index 573a6e91f3..f44e9daa89 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -21,6 +21,38 @@ module OpenFoodNetwork end end + describe "finding exchanges of an order cycle that an admin can manage" do + let(:oc) { create(:simple_order_cycle) } + let!(:ex) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2) } + + before do + permissions.stub(:managed_enterprises) { [] } + permissions.stub(:related_enterprises_with) { [] } + end + + it "returns exchanges involving enterprises managed by the user" do + permissions.stub(:managed_enterprises) { [e1, e2] } + permissions.order_cycle_exchanges(oc).should == [ex] + end + + it "returns exchanges involving enterprises with E2E permission" do + permissions.stub(:related_enterprises_with) { [e1, e2] } + permissions.order_cycle_exchanges(oc).should == [ex] + end + + it "does not return exchanges involving only the sender" do + permissions.stub(:managed_enterprises) { [e1] } + permissions.order_cycle_exchanges(oc).should == [] + end + + it "does not return exchanges involving only the receiver" do + permissions.stub(:managed_enterprises) { [e2] } + permissions.order_cycle_exchanges(oc).should == [] + end + end + + ######################################## + describe "finding related enterprises with a particular permission" do let!(:er) { create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [permission]) } From 5ef13d3c5aaeb5d91249a17c260436b4ea0faabe Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 14:46:23 +1000 Subject: [PATCH 28/46] Change 'add products to OC' permission into the more general 'add enterprise to OC' --- .../enterprise_relationships.js.coffee | 6 ++--- app/helpers/order_cycles_helper.rb | 4 +++ app/views/admin/order_cycles/_form.html.haml | 2 +- lib/open_food_network/permissions.rb | 20 +++++--------- .../admin/enterprise_relationships_spec.rb | 14 +++++----- spec/features/admin/order_cycles_spec.rb | 2 +- .../enterprise_relationships_spec.js.coffee | 2 +- .../lib/open_food_network/permissions_spec.rb | 27 +++++++++---------- 8 files changed, 36 insertions(+), 41 deletions(-) diff --git a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee index 7a799f2f28..cad556efd8 100644 --- a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee +++ b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee @@ -2,7 +2,7 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris new class EnterpriseRelationships create_errors: "" all_permissions: [ - 'add_products_to_order_cycle' + 'add_to_order_cycle' 'manage_products' ] @@ -24,5 +24,5 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris permission_presentation: (permission) -> switch permission - when "add_products_to_order_cycle" then "can add products to order cycle from" - when "manage_products" then "can manage the products of" \ No newline at end of file + when "add_to_order_cycle" then "can add to order cycle" + when "manage_products" then "can manage the products of" diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index 3fc36bfc4d..e8e51815a6 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -3,6 +3,10 @@ module OrderCyclesHelper @current_order_cycle ||= current_order(false).andand.order_cycle end + def order_cycle_producer_enterprises + OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises.is_primary_producer.by_name + end + def coordinating_enterprises Enterprise.is_distributor.managed_by(spree_current_user).by_name end diff --git a/app/views/admin/order_cycles/_form.html.haml b/app/views/admin/order_cycles/_form.html.haml index 4087ebcfd5..aac9f07d6b 100644 --- a/app/views/admin/order_cycles/_form.html.haml +++ b/app/views/admin/order_cycles/_form.html.haml @@ -25,7 +25,7 @@ %tr.products{'ng-show' => 'exchange.showProducts'} = render 'exchange_supplied_products_form' -= select_tag :new_supplier_id, options_from_collection_for_select(OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_producers, :id, :name), {'ng-model' => 'new_supplier_id'} += select_tag :new_supplier_id, options_from_collection_for_select(order_cycle_producer_enterprises, :id, :name), {'ng-model' => 'new_supplier_id'} = f.submit 'Add supplier', 'ng-click' => 'addSupplier($event)' diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 55ec1b74a8..99c0aa0c15 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -4,15 +4,17 @@ module OpenFoodNetwork @user = user end - # Find producers for which an admin is allowed to add their products to an order cycle - def order_cycle_producers - (managed_producers + related_producers_with(:add_products_to_order_cycle)). - sort_by(&:name) + # Find enterprises that an admin is allowed to add to an order cycle + def order_cycle_enterprises + managed_enterprise_ids = managed_enterprises.pluck :id + permitted_enterprise_ids = related_enterprises_with(:add_to_order_cycle).pluck :id + + Enterprise.where('id IN (?)', managed_enterprise_ids + permitted_enterprise_ids) end # Find the exchanges of an order cycle that an admin can manage def order_cycle_exchanges(order_cycle) - enterprises = managed_enterprises + related_enterprises_with(:add_products_to_order_cycle) + enterprises = managed_enterprises + related_enterprises_with(:add_to_order_cycle) order_cycle.exchanges.to_enterprises(enterprises).from_enterprises(enterprises) end @@ -23,10 +25,6 @@ module OpenFoodNetwork Enterprise.managed_by(@user) end - def managed_producers - managed_enterprises.is_primary_producer.by_name - end - def related_enterprises_with(permission) parent_ids = EnterpriseRelationship. permitting(managed_enterprises). @@ -35,9 +33,5 @@ module OpenFoodNetwork Enterprise.where('id IN (?)', parent_ids) end - - def related_producers_with(permission) - related_enterprises_with(permission).is_primary_producer - end end end diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 20c7b28ac2..ba7dbb2fec 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -14,9 +14,9 @@ feature %q{ scenario "listing relationships" do # Given some enterprises with relationships e1, e2, e3, e4 = create(:enterprise), create(:enterprise), create(:enterprise), create(:enterprise) - create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [:add_products_to_order_cycle]) + create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [:add_to_order_cycle]) create(:enterprise_relationship, parent: e2, child: e3, permissions_list: [:manage_products]) - create(:enterprise_relationship, parent: e3, child: e4, permissions_list: [:add_products_to_order_cycle, :manage_products]) + create(:enterprise_relationship, parent: e3, child: e4, permissions_list: [:add_to_order_cycle, :manage_products]) # When I go to the relationships page click_link 'Enterprises' @@ -24,10 +24,10 @@ feature %q{ # Then I should see the relationships within('table#enterprise-relationships') do - page.should have_relationship e1, e2, ['can add products to order cycle from'] + page.should have_relationship e1, e2, ['can add to order cycle'] page.should have_relationship e2, e3, ['can manage the products of'] page.should have_relationship e3, e4, - ['can add products to order cycle from', 'can manage the products of'] + ['can add to order cycle', 'can manage the products of'] end end @@ -38,16 +38,16 @@ feature %q{ visit admin_enterprise_relationships_path select 'One', from: 'enterprise_relationship_parent_id' - check 'can add products to order cycle from' + check 'can add to order cycle' check 'can manage the products of' uncheck 'can manage the products of' select 'Two', from: 'enterprise_relationship_child_id' click_button 'Create' - page.should have_relationship e1, e2, ['can add products to order cycle from'] + page.should have_relationship e1, e2, ['can add to order cycle'] er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first er.should be_present - er.permissions.map(&:name).should == ['add_products_to_order_cycle'] + er.permissions.map(&:name).should == ['add_to_order_cycle'] end diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 26488fd9f4..9dee3ebd87 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -444,7 +444,7 @@ feature %q{ let!(:distributor_managed_fee) { create(:enterprise_fee, enterprise: distributor_managed, name: 'Managed distributor fee') } let!(:supplier_permitted_relationship) do create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed, - permissions_list: [:add_products_to_order_cycle]) + permissions_list: [:add_to_order_cycle]) end before do diff --git a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee index 4e90f65335..9701377a73 100644 --- a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee @@ -12,5 +12,5 @@ describe "enterprise relationships", -> EnterpriseRelationships = _EnterpriseRelationships_ it "presents permission names", -> - expect(EnterpriseRelationships.permission_presentation("add_products_to_order_cycle")).toEqual "can add products to order cycle from" + expect(EnterpriseRelationships.permission_presentation("add_to_order_cycle")).toEqual "can add to order cycle" expect(EnterpriseRelationships.permission_presentation("manage_products")).toEqual "can manage the products of" diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index f44e9daa89..b0a004cbf1 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -8,16 +8,20 @@ module OpenFoodNetwork let(:e1) { create(:enterprise) } let(:e2) { create(:enterprise) } - describe "finding producers that can be added to an order cycle" do - let(:producer1) { double(:enterprise1, name: 'A') } - let(:producer2) { double(:enterprise2, name: 'B') } - let(:producer3) { double(:enterprise3, name: 'C') } + describe "finding enterprises that can be added to an order cycle" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where('1=0') } + permissions.stub(:related_enterprises_with) { Enterprise.where('1=0') } + end - it "returns managed producers and related+permitted enterprises, sorted by name" do - permissions.stub(:managed_producers) { [producer1, producer3] } - permissions.stub(:related_producers_with) { [producer2] } + it "returns managed enterprises" do + permissions.stub(:managed_enterprises) { Enterprise.where(id: e1) } + permissions.order_cycle_enterprises.should == [e1] + end - permissions.order_cycle_producers.should == [producer1, producer2, producer3] + it "returns permitted enterprises" do + permissions.stub(:related_enterprises_with) { Enterprise.where(id: e2) } + permissions.order_cycle_enterprises.should == [e2] end end @@ -66,12 +70,5 @@ module OpenFoodNetwork permissions.send(:related_enterprises_with, permission).should == [] end end - - describe "finding related producers with a particular permission" do - it "returns permitted related enterprises that are also producers" do - permissions.stub_chain(:related_enterprises_with, :is_primary_producer) { [e1] } - permissions.send(:related_producers_with, permission).should == [e1] - end - end end end From 628d87b69a18249805ddbd1a2e45bed18385c985 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 15:00:13 +1000 Subject: [PATCH 29/46] Add to OC permission allows adding distributors to order cycle --- app/helpers/order_cycles_helper.rb | 6 +++++- app/views/admin/order_cycles/_form.html.haml | 2 +- spec/features/admin/order_cycles_spec.rb | 13 ++++++++++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index e8e51815a6..4dc13be4fa 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -8,7 +8,11 @@ module OrderCyclesHelper end def coordinating_enterprises - Enterprise.is_distributor.managed_by(spree_current_user).by_name + order_cycle_hub_enterprises + end + + def order_cycle_hub_enterprises + OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises.is_distributor.by_name end def order_cycle_local_remote_class(distributor, order_cycle) diff --git a/app/views/admin/order_cycles/_form.html.haml b/app/views/admin/order_cycles/_form.html.haml index aac9f07d6b..e859374a3b 100644 --- a/app/views/admin/order_cycles/_form.html.haml +++ b/app/views/admin/order_cycles/_form.html.haml @@ -50,7 +50,7 @@ %tr.products{'ng-show' => 'exchange.showProducts'} = render 'exchange_distributed_products_form' -= select_tag :new_distributor_id, options_from_collection_for_select(Enterprise.is_distributor.managed_by(spree_current_user).by_name, :id, :name), {'ng-model' => 'new_distributor_id'} += select_tag :new_distributor_id, options_from_collection_for_select(order_cycle_hub_enterprises, :id, :name), {'ng-model' => 'new_distributor_id'} = f.submit 'Add distributor', 'ng-click' => 'addDistributor($event)' .actions diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 9dee3ebd87..368eb0b82a 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -441,11 +441,16 @@ feature %q{ let!(:supplier_permitted) { create(:supplier_enterprise, name: 'Permitted supplier') } let!(:distributor_managed) { create(:distributor_enterprise, name: 'Managed distributor') } let!(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Unmanaged Distributor') } + let!(:distributor_permitted) { create(:distributor_enterprise, name: 'Permitted distributor') } let!(:distributor_managed_fee) { create(:enterprise_fee, enterprise: distributor_managed, name: 'Managed distributor fee') } let!(:supplier_permitted_relationship) do create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed, permissions_list: [:add_to_order_cycle]) end + let!(:distributor_permitted_relationship) do + create(:enterprise_relationship, parent: distributor_permitted, child: distributor_managed, + permissions_list: [:add_to_order_cycle]) + end before do product = create(:product, supplier: supplier_managed) @@ -493,6 +498,8 @@ feature %q{ select 'Managed distributor', from: 'new_distributor_id' click_button 'Add distributor' + select 'Permitted distributor', from: 'new_distributor_id' + click_button 'Add distributor' # Should only have suppliers / distributors listed which the user is managing or # has E2E permission to add products to order cycles @@ -511,13 +518,13 @@ feature %q{ end scenario "editing an order cycle" do - oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_unmanaged], name: 'Order Cycle 1' } ) + oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_permitted, distributor_unmanaged], name: 'Order Cycle 1' } ) visit edit_admin_order_cycle_path(oc) # I should not see exchanges for supplier_unmanaged or distributor_unmanaged page.all('tr.supplier').count.should == 2 - page.all('tr.distributor').count.should == 1 + page.all('tr.distributor').count.should == 2 # When I save, then those exchanges should remain click_button 'Update' @@ -526,7 +533,7 @@ feature %q{ oc.reload oc.suppliers.sort.should == [supplier_managed, supplier_permitted, supplier_unmanaged].sort oc.coordinator.should == supplier_managed - oc.distributors.sort.should == [distributor_managed, distributor_unmanaged].sort + oc.distributors.sort.should == [distributor_managed, distributor_permitted, distributor_unmanaged].sort end scenario "cloning an order cycle" do From a5debc19dce91abeaebcd7de4bc74f119124b4a1 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 15:25:17 +1000 Subject: [PATCH 30/46] Permit edits to exchanges involving enterprises permitted via E2E relationships --- .../admin/order_cycles_controller.rb | 6 +++-- app/helpers/order_cycles_helper.rb | 8 ++++-- .../order_cycles/_exchange_form.html.haml | 2 +- spec/features/admin/order_cycles_spec.rb | 26 ++++++++++++++++++- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 58c04a63bf..63daf918de 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -3,6 +3,8 @@ require 'open_food_network/order_cycle_form_applicator' module Admin class OrderCyclesController < ResourceController + include OrderCyclesHelper + before_filter :load_order_cycle_set, :only => :index def show @@ -24,7 +26,7 @@ module Admin respond_to do |format| if @order_cycle.save - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go! + OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, order_cycle_permitted_enterprises).go! flash[:notice] = 'Your order cycle has been created.' format.html { redirect_to admin_order_cycles_path } @@ -41,7 +43,7 @@ module Admin respond_to do |format| if @order_cycle.update_attributes(params[:order_cycle]) - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go! + OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, order_cycle_permitted_enterprises).go! flash[:notice] = 'Your order cycle has been updated.' format.html { redirect_to admin_order_cycles_path } diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index 4dc13be4fa..dfd957a143 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -3,8 +3,12 @@ module OrderCyclesHelper @current_order_cycle ||= current_order(false).andand.order_cycle end + def order_cycle_permitted_enterprises + OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises + end + def order_cycle_producer_enterprises - OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises.is_primary_producer.by_name + order_cycle_permitted_enterprises.is_primary_producer.by_name end def coordinating_enterprises @@ -12,7 +16,7 @@ module OrderCyclesHelper end def order_cycle_hub_enterprises - OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises.is_distributor.by_name + order_cycle_permitted_enterprises.is_distributor.by_name end def order_cycle_local_remote_class(distributor, order_cycle) diff --git a/app/views/admin/order_cycles/_exchange_form.html.haml b/app/views/admin/order_cycles/_exchange_form.html.haml index 3539892034..8f81b9f03f 100644 --- a/app/views/admin/order_cycles/_exchange_form.html.haml +++ b/app/views/admin/order_cycles/_exchange_form.html.haml @@ -23,4 +23,4 @@ = f.submit 'Add fee', 'ng-click' => 'addExchangeFee($event, exchange)' %td.actions - %a{'ng-click' => 'removeExchange($event, exchange)', :class => "icon-trash no-text"} + %a{'ng-click' => 'removeExchange($event, exchange)', :class => "icon-trash no-text remove-exchange"} diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 368eb0b82a..76ee3bdfc2 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -514,10 +514,12 @@ feature %q{ flash_message.should == "Your order cycle has been created." order_cycle = OrderCycle.find_by_name('My order cycle') + order_cycle.suppliers.sort.should == [supplier_managed, supplier_permitted].sort order_cycle.coordinator.should == distributor_managed + order_cycle.distributors.sort.should == [distributor_managed, distributor_permitted].sort end - scenario "editing an order cycle" do + scenario "editing an order cycle does not affect exchanges we don't manage" do oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_permitted, distributor_unmanaged], name: 'Order Cycle 1' } ) visit edit_admin_order_cycle_path(oc) @@ -536,6 +538,28 @@ feature %q{ oc.distributors.sort.should == [distributor_managed, distributor_permitted, distributor_unmanaged].sort end + scenario "editing an order cycle" do + oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_permitted, distributor_unmanaged], name: 'Order Cycle 1' } ) + + visit edit_admin_order_cycle_path(oc) + + # When I remove all the exchanges and save + page.find("tr.supplier-#{supplier_managed.id} a.remove-exchange").click + page.find("tr.supplier-#{supplier_permitted.id} a.remove-exchange").click + page.find("tr.distributor-#{distributor_managed.id} a.remove-exchange").click + page.find("tr.distributor-#{distributor_permitted.id} a.remove-exchange").click + click_button 'Update' + + # Then the exchanges should be removed + page.should have_content "Your order cycle has been updated." + + oc.reload + oc.suppliers.should == [supplier_unmanaged] + oc.coordinator.should == supplier_managed + oc.distributors.should == [distributor_unmanaged] + end + + scenario "cloning an order cycle" do oc = create(:simple_order_cycle, coordinator: distributor_managed) From 1871d42e68e37a3bd4414743e6febdefa8d95aae Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Tue, 26 Aug 2014 15:42:35 +1000 Subject: [PATCH 31/46] Switch to correct grammatical ordering of child/parent enterprise on enterprise relationships page --- app/models/enterprise_relationship.rb | 2 +- .../_enterprise_relationship.html.haml | 4 ++-- app/views/admin/enterprise_relationships/_form.html.haml | 4 ++-- spec/features/admin/enterprise_relationships_spec.rb | 2 +- spec/models/enterprise_relationship_spec.rb | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 8e2f3bcf06..dba99cc7b3 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -21,7 +21,7 @@ class EnterpriseRelationship < ActiveRecord::Base where('enterprise_relationship_permissions.name = ?', permission) } - scope :by_name, with_enterprises.order('parent_enterprises.name, child_enterprises.name') + scope :by_name, with_enterprises.order('child_enterprises.name, parent_enterprises.name') def permissions_list=(perms) diff --git a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml index 56f0d46c01..6a7dcdc7f4 100644 --- a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml +++ b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml @@ -1,8 +1,8 @@ -%td {{ enterprise_relationship.parent_name }} +%td {{ enterprise_relationship.child_name }} %td %ul %li{"ng-repeat" => "permission in enterprise_relationship.permissions"} {{ EnterpriseRelationships.permission_presentation(permission.name) }} -%td {{ enterprise_relationship.child_name }} +%td {{ enterprise_relationship.parent_name }} %td.actions %a.delete-enterprise-relationship.icon-trash.no-text{'ng-click' => 'delete(enterprise_relationship)'} diff --git a/app/views/admin/enterprise_relationships/_form.html.haml b/app/views/admin/enterprise_relationships/_form.html.haml index b9c2f265cd..2fe9402d3a 100644 --- a/app/views/admin/enterprise_relationships/_form.html.haml +++ b/app/views/admin/enterprise_relationships/_form.html.haml @@ -1,6 +1,6 @@ %tr %td - %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} + %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} %td permits / %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} @@ -8,7 +8,7 @@ / %input{type: "checkbox", "ng-model" => "permissions[permission]"} / {{ EnterpriseRelationships.permission_presentation(permission) }} %td - %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} + %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} %td.actions %input{type: "button", value: "Create", "ng-click" => "create()"} .errors {{ EnterpriseRelationships.create_errors }} diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index ba7dbb2fec..0dadb25848 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -119,6 +119,6 @@ feature %q{ def have_relationship(parent, child, perms=[]) perms = perms.join(' ') || 'permits' - have_table_row [parent.name, perms, child.name, ''] + have_table_row [child.name, perms, parent.name, ''] end end diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index 3201e1db5c..3a6c48e891 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -6,10 +6,10 @@ describe EnterpriseRelationship do let(:e2) { create(:enterprise, name: 'B') } let(:e3) { create(:enterprise, name: 'C') } - it "sorts by parent, child enterprise name" do - er1 = create(:enterprise_relationship, parent: e1, child: e3) - er2 = create(:enterprise_relationship, parent: e2, child: e1) - er3 = create(:enterprise_relationship, parent: e1, child: e2) + it "sorts by child, parent enterprise name" do + er1 = create(:enterprise_relationship, parent: e3, child: e1) + er2 = create(:enterprise_relationship, parent: e1, child: e2) + er3 = create(:enterprise_relationship, parent: e2, child: e1) EnterpriseRelationship.by_name.should == [er3, er1, er2] end From 7b89e6aa80753d9729350315b4ce41f4264306c9 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 28 Aug 2014 10:12:06 +1000 Subject: [PATCH 32/46] Revert "Comment out ERPs which have confusing names, use old 'permits'" This reverts commit a4be0ff55a2ef4cb7dd78412ca6a5018db6ab167. --- app/views/admin/enterprise_relationships/_form.html.haml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/views/admin/enterprise_relationships/_form.html.haml b/app/views/admin/enterprise_relationships/_form.html.haml index 2fe9402d3a..86086c8781 100644 --- a/app/views/admin/enterprise_relationships/_form.html.haml +++ b/app/views/admin/enterprise_relationships/_form.html.haml @@ -2,11 +2,10 @@ %td %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} %td - permits - / %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} - / %label - / %input{type: "checkbox", "ng-model" => "permissions[permission]"} - / {{ EnterpriseRelationships.permission_presentation(permission) }} + %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} + %label + %input{type: "checkbox", "ng-model" => "permissions[permission]"} + {{ EnterpriseRelationships.permission_presentation(permission) }} %td %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} %td.actions From bfd9ffd84aed701b3799a02fbed60173e70b54af Mon Sep 17 00:00:00 2001 From: Rob H Date: Thu, 28 Aug 2014 10:03:53 +1000 Subject: [PATCH 33/46] Adding missing data-bindings for country/state --- app/views/checkout/_billing.html.haml | 2 +- app/views/checkout/_details.html.haml | 16 ++++++++-------- app/views/checkout/_shipping.html.haml | 13 +++++++------ spec/features/consumer/shopping/checkout_spec.rb | 2 +- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/app/views/checkout/_billing.html.haml b/app/views/checkout/_billing.html.haml index 1e7adec1ad..c052eb1b05 100644 --- a/app/views/checkout/_billing.html.haml +++ b/app/views/checkout/_billing.html.haml @@ -47,7 +47,7 @@ .small-6.columns.right = ba.select :country_id, available_countries.map{|c|[c.name, c.id]}, - {include_blank: false}, "ng-model" => "order.bill_address.country_id" + "ng-model" => "order.bill_address.country_id", required: true .row .small-12.columns.text-right diff --git a/app/views/checkout/_details.html.haml b/app/views/checkout/_details.html.haml index 6bb24d4682..6eb149804f 100644 --- a/app/views/checkout/_details.html.haml +++ b/app/views/checkout/_details.html.haml @@ -9,23 +9,23 @@ %i.ofn-i_051-check-big Your details - %accordion-group{"is-open" => "accordion.details", + %accordion-group{"is-open" => "accordion.details", "ng-class" => "{valid: details.$valid, open: accordion.details}"} %accordion-heading .row .small-8.medium-10.columns - %em + %em %small {{ summary() | printArray }} .small-4.medium-2.columns.text-right %span.accordion-up - %em + %em %small Hide - %i.ofn-i_053-point-up + %i.ofn-i_053-point-up %span.accordion-down - %em + %em %small Expand - %i.ofn-i_052-point-down + %i.ofn-i_052-point-down .row .small-6.columns @@ -36,10 +36,10 @@ .row .small-6.columns = validated_input 'Email', 'order.email', type: :email, "ofn-focus" => "accordion['details']" - + .small-6.columns = validated_input 'Phone', 'order.bill_address.phone' - + .row .small-12.columns.text-right %button.primary{"ng-disabled" => "details.$invalid", "ng-click" => "next($event)"} Next diff --git a/app/views/checkout/_shipping.html.haml b/app/views/checkout/_shipping.html.haml index 8904facff4..27089527a2 100644 --- a/app/views/checkout/_shipping.html.haml +++ b/app/views/checkout/_shipping.html.haml @@ -14,16 +14,16 @@ %accordion-heading .row .small-8.medium-10.columns - %em + %em %small {{ Checkout.shippingMethod().name }} .small-4.medium-2.columns.text-right %span.accordion-up - %em + %em %small Hide - %i.ofn-i_053-point-up + %i.ofn-i_053-point-up %span.accordion-down - %em + %em %small Expand %i.ofn-i_052-point-down @@ -61,13 +61,14 @@ .small-6.columns = validated_input "City", "order.ship_address.city" .small-6.columns - = sa.select :state_id, @order.shipping_address.country.states.map{|c|[c.name, c.id]} + = sa.select :state_id, @order.shipping_address.country.states.map{|c|[c.name, c.id]}, {include_blank: false}, + "ng-model" => "order.ship_address.state_id", required: true .row .small-6.columns = validated_input "Postcode", "order.ship_address.zipcode" .small-6.columns.right = sa.select :country_id, available_countries.map{|c|[c.name, c.id]}, - {include_blank: false} + "ng-model" => "order.ship_address.country_id", required: true .row .small-6.columns = validated_input "First Name", "order.ship_address.firstname" diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 54091e854d..b26e924495 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -23,7 +23,7 @@ feature "As a consumer I want to check out my cart", js: true do end it "shows the current distributor on checkout" do - visit checkout_path + visit checkout_path page.should have_content distributor.name end From 5ede8d169faa0d95942b45d5b81229a4367432b1 Mon Sep 17 00:00:00 2001 From: Rob H Date: Thu, 28 Aug 2014 14:32:57 +1000 Subject: [PATCH 34/46] Revert a4be0ff..7b89e6a for deployment --- .../enterprise_relationships.js.coffee | 6 +- .../admin/order_cycles_controller.rb | 7 +- app/helpers/order_cycles_helper.rb | 14 +-- app/models/enterprise_relationship.rb | 10 +- .../_enterprise_relationship.html.haml | 4 +- .../enterprise_relationships/_form.html.haml | 15 +-- .../order_cycles/_exchange_form.html.haml | 2 +- app/views/admin/order_cycles/_form.html.haml | 4 +- app/views/admin/order_cycles/show.rep | 2 +- lib/open_food_network/permissions.rb | 37 ------ .../admin/enterprise_relationships_spec.rb | 16 +-- spec/features/admin/order_cycles_spec.rb | 113 ++++++------------ .../enterprise_relationships_spec.js.coffee | 2 +- .../lib/open_food_network/permissions_spec.rb | 74 ------------ spec/models/enterprise_relationship_spec.rb | 27 +---- 15 files changed, 73 insertions(+), 260 deletions(-) delete mode 100644 lib/open_food_network/permissions.rb delete mode 100644 spec/lib/open_food_network/permissions_spec.rb diff --git a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee index cad556efd8..7a799f2f28 100644 --- a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee +++ b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee @@ -2,7 +2,7 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris new class EnterpriseRelationships create_errors: "" all_permissions: [ - 'add_to_order_cycle' + 'add_products_to_order_cycle' 'manage_products' ] @@ -24,5 +24,5 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris permission_presentation: (permission) -> switch permission - when "add_to_order_cycle" then "can add to order cycle" - when "manage_products" then "can manage the products of" + when "add_products_to_order_cycle" then "can add products to order cycle from" + when "manage_products" then "can manage the products of" \ No newline at end of file diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 63daf918de..566c660112 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -1,10 +1,7 @@ -require 'open_food_network/permissions' require 'open_food_network/order_cycle_form_applicator' module Admin class OrderCyclesController < ResourceController - include OrderCyclesHelper - before_filter :load_order_cycle_set, :only => :index def show @@ -26,7 +23,7 @@ module Admin respond_to do |format| if @order_cycle.save - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, order_cycle_permitted_enterprises).go! + OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go! flash[:notice] = 'Your order cycle has been created.' format.html { redirect_to admin_order_cycles_path } @@ -43,7 +40,7 @@ module Admin respond_to do |format| if @order_cycle.update_attributes(params[:order_cycle]) - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, order_cycle_permitted_enterprises).go! + OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go! flash[:notice] = 'Your order cycle has been updated.' format.html { redirect_to admin_order_cycles_path } diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index dfd957a143..92cd968e0b 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -3,20 +3,8 @@ module OrderCyclesHelper @current_order_cycle ||= current_order(false).andand.order_cycle end - def order_cycle_permitted_enterprises - OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises - end - - def order_cycle_producer_enterprises - order_cycle_permitted_enterprises.is_primary_producer.by_name - end - def coordinating_enterprises - order_cycle_hub_enterprises - end - - def order_cycle_hub_enterprises - order_cycle_permitted_enterprises.is_distributor.by_name + Enterprise.is_distributor.managed_by(spree_current_user).order('name') end def order_cycle_local_remote_class(distributor, order_cycle) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index dba99cc7b3..5c10aafbbf 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -9,20 +9,12 @@ class EnterpriseRelationship < ActiveRecord::Base scope :with_enterprises, joins('LEFT JOIN enterprises AS parent_enterprises ON parent_enterprises.id = enterprise_relationships.parent_id'). joins('LEFT JOIN enterprises AS child_enterprises ON child_enterprises.id = enterprise_relationships.child_id') + scope :by_name, with_enterprises.order('parent_enterprises.name, child_enterprises.name') scope :involving_enterprises, ->(enterprises) { where('parent_id IN (?) OR child_id IN (?)', enterprises, enterprises) } - scope :permitting, ->(enterprises) { where('child_id IN (?)', enterprises) } - - scope :with_permission, ->(permission) { - joins(:permissions). - where('enterprise_relationship_permissions.name = ?', permission) - } - - scope :by_name, with_enterprises.order('child_enterprises.name, parent_enterprises.name') - def permissions_list=(perms) perms.andand.each { |name| permissions.build name: name } diff --git a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml index 6a7dcdc7f4..56f0d46c01 100644 --- a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml +++ b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml @@ -1,8 +1,8 @@ -%td {{ enterprise_relationship.child_name }} +%td {{ enterprise_relationship.parent_name }} %td %ul %li{"ng-repeat" => "permission in enterprise_relationship.permissions"} {{ EnterpriseRelationships.permission_presentation(permission.name) }} -%td {{ enterprise_relationship.parent_name }} +%td {{ enterprise_relationship.child_name }} %td.actions %a.delete-enterprise-relationship.icon-trash.no-text{'ng-click' => 'delete(enterprise_relationship)'} diff --git a/app/views/admin/enterprise_relationships/_form.html.haml b/app/views/admin/enterprise_relationships/_form.html.haml index 86086c8781..b9c2f265cd 100644 --- a/app/views/admin/enterprise_relationships/_form.html.haml +++ b/app/views/admin/enterprise_relationships/_form.html.haml @@ -1,13 +1,14 @@ %tr - %td - %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} - %td - %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} - %label - %input{type: "checkbox", "ng-model" => "permissions[permission]"} - {{ EnterpriseRelationships.permission_presentation(permission) }} %td %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} + %td + permits + / %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} + / %label + / %input{type: "checkbox", "ng-model" => "permissions[permission]"} + / {{ EnterpriseRelationships.permission_presentation(permission) }} + %td + %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} %td.actions %input{type: "button", value: "Create", "ng-click" => "create()"} .errors {{ EnterpriseRelationships.create_errors }} diff --git a/app/views/admin/order_cycles/_exchange_form.html.haml b/app/views/admin/order_cycles/_exchange_form.html.haml index 8f81b9f03f..3539892034 100644 --- a/app/views/admin/order_cycles/_exchange_form.html.haml +++ b/app/views/admin/order_cycles/_exchange_form.html.haml @@ -23,4 +23,4 @@ = f.submit 'Add fee', 'ng-click' => 'addExchangeFee($event, exchange)' %td.actions - %a{'ng-click' => 'removeExchange($event, exchange)', :class => "icon-trash no-text remove-exchange"} + %a{'ng-click' => 'removeExchange($event, exchange)', :class => "icon-trash no-text"} diff --git a/app/views/admin/order_cycles/_form.html.haml b/app/views/admin/order_cycles/_form.html.haml index e859374a3b..68dfed9f1a 100644 --- a/app/views/admin/order_cycles/_form.html.haml +++ b/app/views/admin/order_cycles/_form.html.haml @@ -25,7 +25,7 @@ %tr.products{'ng-show' => 'exchange.showProducts'} = render 'exchange_supplied_products_form' -= select_tag :new_supplier_id, options_from_collection_for_select(order_cycle_producer_enterprises, :id, :name), {'ng-model' => 'new_supplier_id'} += select_tag :new_supplier_id, options_from_collection_for_select(Enterprise.is_primary_producer.managed_by(spree_current_user).by_name, :id, :name), {'ng-model' => 'new_supplier_id'} = f.submit 'Add supplier', 'ng-click' => 'addSupplier($event)' @@ -50,7 +50,7 @@ %tr.products{'ng-show' => 'exchange.showProducts'} = render 'exchange_distributed_products_form' -= select_tag :new_distributor_id, options_from_collection_for_select(order_cycle_hub_enterprises, :id, :name), {'ng-model' => 'new_distributor_id'} += select_tag :new_distributor_id, options_from_collection_for_select(Enterprise.is_distributor.managed_by(spree_current_user).by_name, :id, :name), {'ng-model' => 'new_distributor_id'} = f.submit 'Add distributor', 'ng-click' => 'addDistributor($event)' .actions diff --git a/app/views/admin/order_cycles/show.rep b/app/views/admin/order_cycles/show.rep index fb71602eb6..04fc659813 100644 --- a/app/views/admin/order_cycles/show.rep +++ b/app/views/admin/order_cycles/show.rep @@ -9,7 +9,7 @@ r.element :order_cycle, @order_cycle do r.element :id end - r.list_of :exchanges, OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_exchanges(@order_cycle).order('id ASC') do |exchange| + r.list_of :exchanges, @order_cycle.exchanges.managed_by(spree_current_user).order('id ASC') do |exchange| r.element :id r.element :sender_id r.element :receiver_id diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb deleted file mode 100644 index 99c0aa0c15..0000000000 --- a/lib/open_food_network/permissions.rb +++ /dev/null @@ -1,37 +0,0 @@ -module OpenFoodNetwork - class Permissions - def initialize(user) - @user = user - end - - # Find enterprises that an admin is allowed to add to an order cycle - def order_cycle_enterprises - managed_enterprise_ids = managed_enterprises.pluck :id - permitted_enterprise_ids = related_enterprises_with(:add_to_order_cycle).pluck :id - - Enterprise.where('id IN (?)', managed_enterprise_ids + permitted_enterprise_ids) - end - - # Find the exchanges of an order cycle that an admin can manage - def order_cycle_exchanges(order_cycle) - enterprises = managed_enterprises + related_enterprises_with(:add_to_order_cycle) - order_cycle.exchanges.to_enterprises(enterprises).from_enterprises(enterprises) - end - - - private - - def managed_enterprises - Enterprise.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 - end -end diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 0dadb25848..20c7b28ac2 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -14,9 +14,9 @@ feature %q{ scenario "listing relationships" do # Given some enterprises with relationships e1, e2, e3, e4 = create(:enterprise), create(:enterprise), create(:enterprise), create(:enterprise) - create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [:add_to_order_cycle]) + create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [:add_products_to_order_cycle]) create(:enterprise_relationship, parent: e2, child: e3, permissions_list: [:manage_products]) - create(:enterprise_relationship, parent: e3, child: e4, permissions_list: [:add_to_order_cycle, :manage_products]) + create(:enterprise_relationship, parent: e3, child: e4, permissions_list: [:add_products_to_order_cycle, :manage_products]) # When I go to the relationships page click_link 'Enterprises' @@ -24,10 +24,10 @@ feature %q{ # Then I should see the relationships within('table#enterprise-relationships') do - page.should have_relationship e1, e2, ['can add to order cycle'] + page.should have_relationship e1, e2, ['can add products to order cycle from'] page.should have_relationship e2, e3, ['can manage the products of'] page.should have_relationship e3, e4, - ['can add to order cycle', 'can manage the products of'] + ['can add products to order cycle from', 'can manage the products of'] end end @@ -38,16 +38,16 @@ feature %q{ visit admin_enterprise_relationships_path select 'One', from: 'enterprise_relationship_parent_id' - check 'can add to order cycle' + check 'can add products to order cycle from' check 'can manage the products of' uncheck 'can manage the products of' select 'Two', from: 'enterprise_relationship_child_id' click_button 'Create' - page.should have_relationship e1, e2, ['can add to order cycle'] + page.should have_relationship e1, e2, ['can add products to order cycle from'] er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first er.should be_present - er.permissions.map(&:name).should == ['add_to_order_cycle'] + er.permissions.map(&:name).should == ['add_products_to_order_cycle'] end @@ -119,6 +119,6 @@ feature %q{ def have_relationship(parent, child, perms=[]) perms = perms.join(' ') || 'permits' - have_table_row [child.name, perms, parent.name, ''] + have_table_row [parent.name, perms, child.name, ''] end end diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 76ee3bdfc2..2423a6dc05 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -436,37 +436,26 @@ feature %q{ context "as an enterprise user" do - let!(:supplier_managed) { create(:supplier_enterprise, name: 'Managed supplier') } - let!(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Unmanaged supplier') } - let!(:supplier_permitted) { create(:supplier_enterprise, name: 'Permitted supplier') } - let!(:distributor_managed) { create(:distributor_enterprise, name: 'Managed distributor') } - let!(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Unmanaged Distributor') } - let!(:distributor_permitted) { create(:distributor_enterprise, name: 'Permitted distributor') } - let!(:distributor_managed_fee) { create(:enterprise_fee, enterprise: distributor_managed, name: 'Managed distributor fee') } - let!(:supplier_permitted_relationship) do - create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed, - permissions_list: [:add_to_order_cycle]) - end - let!(:distributor_permitted_relationship) do - create(:enterprise_relationship, parent: distributor_permitted, child: distributor_managed, - permissions_list: [:add_to_order_cycle]) - end - - before do - product = create(:product, supplier: supplier_managed) - product.distributors << distributor_managed + let(:supplier1) { create(:supplier_enterprise, name: 'First Supplier') } + let(:supplier2) { create(:supplier_enterprise, name: 'Another Supplier') } + let(:distributor1) { create(:distributor_enterprise, name: 'First Distributor') } + let(:distributor2) { create(:distributor_enterprise, name: 'Another Distributor') } + let!(:distributor1_fee) { create(:enterprise_fee, enterprise: distributor1, name: 'First Distributor Fee') } + before(:each) do + product = create(:product, supplier: supplier1) + product.distributors << distributor1 product.save! @new_user = create_enterprise_user - @new_user.enterprise_roles.build(enterprise: supplier_managed).save - @new_user.enterprise_roles.build(enterprise: distributor_managed).save + @new_user.enterprise_roles.build(enterprise: supplier1).save + @new_user.enterprise_roles.build(enterprise: distributor1).save login_to_admin_as @new_user end scenario "viewing a list of order cycles I am coordinating" do - oc_user_coordinating = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_unmanaged], name: 'Order Cycle 1' } ) - oc_for_other_user = create(:simple_order_cycle, { coordinator: supplier_unmanaged, name: 'Order Cycle 2' } ) + oc_user_coordinating = create(:simple_order_cycle, { suppliers: [supplier1, supplier2], coordinator: supplier1, distributors: [distributor1, distributor2], name: 'Order Cycle 1' } ) + oc_for_other_user = create(:simple_order_cycle, { coordinator: supplier2, name: 'Order Cycle 2' } ) click_link "Order Cycles" @@ -475,8 +464,8 @@ feature %q{ page.should_not have_content oc_for_other_user.name # The order cycle should not show enterprises that I don't manage - page.should_not have_selector 'td.suppliers', text: supplier_unmanaged.name - page.should_not have_selector 'td.distributors', text: distributor_unmanaged.name + page.should_not have_selector 'td.suppliers', text: supplier2.name + page.should_not have_selector 'td.distributors', text: distributor2.name end scenario "creating a new order cycle" do @@ -487,81 +476,57 @@ feature %q{ fill_in 'order_cycle_orders_open_at', with: '2012-11-06 06:00:00' fill_in 'order_cycle_orders_close_at', with: '2012-11-13 17:00:00' - select 'Managed supplier', from: 'new_supplier_id' - click_button 'Add supplier' - select 'Permitted supplier', from: 'new_supplier_id' + select 'First Supplier', from: 'new_supplier_id' click_button 'Add supplier' - select 'Managed distributor', from: 'order_cycle_coordinator_id' + select 'First Distributor', from: 'order_cycle_coordinator_id' click_button 'Add coordinator fee' - select 'Managed distributor fee', from: 'order_cycle_coordinator_fee_0_id' + select 'First Distributor Fee', from: 'order_cycle_coordinator_fee_0_id' - select 'Managed distributor', from: 'new_distributor_id' - click_button 'Add distributor' - select 'Permitted distributor', from: 'new_distributor_id' + select 'First Distributor', from: 'new_distributor_id' click_button 'Add distributor' - # Should only have suppliers / distributors listed which the user is managing or - # has E2E permission to add products to order cycles - page.should_not have_select 'new_supplier_id', with_options: [supplier_unmanaged.name] - page.should_not have_select 'new_distributor_id', with_options: [distributor_unmanaged.name] - - [distributor_unmanaged.name, supplier_managed.name, supplier_unmanaged.name].each do |enterprise_name| - page.should_not have_select 'order_cycle_coordinator_id', with_options: [enterprise_name] + # Should only have suppliers / distributors listed which the user can manage + within "#new_supplier_id" do + page.should_not have_content supplier2.name + end + within "#new_distributor_id" do + page.should_not have_content distributor2.name + end + within "#order_cycle_coordinator_id" do + page.should_not have_content distributor2.name + page.should_not have_content supplier1.name + page.should_not have_content supplier2.name end click_button 'Create' flash_message.should == "Your order cycle has been created." order_cycle = OrderCycle.find_by_name('My order cycle') - order_cycle.suppliers.sort.should == [supplier_managed, supplier_permitted].sort - order_cycle.coordinator.should == distributor_managed - order_cycle.distributors.sort.should == [distributor_managed, distributor_permitted].sort + order_cycle.coordinator.should == distributor1 end - scenario "editing an order cycle does not affect exchanges we don't manage" do - oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_permitted, distributor_unmanaged], name: 'Order Cycle 1' } ) + scenario "editing an order cycle" do + oc = create(:simple_order_cycle, { suppliers: [supplier1, supplier2], coordinator: supplier1, distributors: [distributor1, distributor2], name: 'Order Cycle 1' } ) visit edit_admin_order_cycle_path(oc) - # I should not see exchanges for supplier_unmanaged or distributor_unmanaged - page.all('tr.supplier').count.should == 2 - page.all('tr.distributor').count.should == 2 + # I should not see exchanges for supplier2 or distributor2 + page.all('tr.supplier').count.should == 1 + page.all('tr.distributor').count.should == 1 # When I save, then those exchanges should remain click_button 'Update' page.should have_content "Your order cycle has been updated." oc.reload - oc.suppliers.sort.should == [supplier_managed, supplier_permitted, supplier_unmanaged].sort - oc.coordinator.should == supplier_managed - oc.distributors.sort.should == [distributor_managed, distributor_permitted, distributor_unmanaged].sort + oc.suppliers.sort.should == [supplier1, supplier2] + oc.coordinator.should == supplier1 + oc.distributors.sort.should == [distributor1, distributor2] end - scenario "editing an order cycle" do - oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_permitted, distributor_unmanaged], name: 'Order Cycle 1' } ) - - visit edit_admin_order_cycle_path(oc) - - # When I remove all the exchanges and save - page.find("tr.supplier-#{supplier_managed.id} a.remove-exchange").click - page.find("tr.supplier-#{supplier_permitted.id} a.remove-exchange").click - page.find("tr.distributor-#{distributor_managed.id} a.remove-exchange").click - page.find("tr.distributor-#{distributor_permitted.id} a.remove-exchange").click - click_button 'Update' - - # Then the exchanges should be removed - page.should have_content "Your order cycle has been updated." - - oc.reload - oc.suppliers.should == [supplier_unmanaged] - oc.coordinator.should == supplier_managed - oc.distributors.should == [distributor_unmanaged] - end - - scenario "cloning an order cycle" do - oc = create(:simple_order_cycle, coordinator: distributor_managed) + oc = create(:simple_order_cycle) click_link "Order Cycles" first('a.clone-order-cycle').click diff --git a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee index 9701377a73..4e90f65335 100644 --- a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee @@ -12,5 +12,5 @@ describe "enterprise relationships", -> EnterpriseRelationships = _EnterpriseRelationships_ it "presents permission names", -> - expect(EnterpriseRelationships.permission_presentation("add_to_order_cycle")).toEqual "can add to order cycle" + expect(EnterpriseRelationships.permission_presentation("add_products_to_order_cycle")).toEqual "can add products to order cycle from" expect(EnterpriseRelationships.permission_presentation("manage_products")).toEqual "can manage the products of" diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb deleted file mode 100644 index b0a004cbf1..0000000000 --- a/spec/lib/open_food_network/permissions_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -require 'open_food_network/permissions' - -module OpenFoodNetwork - describe Permissions do - let(:user) { double(:user) } - let(:permissions) { Permissions.new(user) } - let(:permission) { 'one' } - let(:e1) { create(:enterprise) } - let(:e2) { create(:enterprise) } - - describe "finding enterprises that can be added to an order cycle" do - before do - permissions.stub(:managed_enterprises) { Enterprise.where('1=0') } - permissions.stub(:related_enterprises_with) { Enterprise.where('1=0') } - end - - it "returns managed enterprises" do - permissions.stub(:managed_enterprises) { Enterprise.where(id: e1) } - permissions.order_cycle_enterprises.should == [e1] - end - - it "returns permitted enterprises" do - permissions.stub(:related_enterprises_with) { Enterprise.where(id: e2) } - permissions.order_cycle_enterprises.should == [e2] - end - end - - describe "finding exchanges of an order cycle that an admin can manage" do - let(:oc) { create(:simple_order_cycle) } - let!(:ex) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2) } - - before do - permissions.stub(:managed_enterprises) { [] } - permissions.stub(:related_enterprises_with) { [] } - end - - it "returns exchanges involving enterprises managed by the user" do - permissions.stub(:managed_enterprises) { [e1, e2] } - permissions.order_cycle_exchanges(oc).should == [ex] - end - - it "returns exchanges involving enterprises with E2E permission" do - permissions.stub(:related_enterprises_with) { [e1, e2] } - permissions.order_cycle_exchanges(oc).should == [ex] - end - - it "does not return exchanges involving only the sender" do - permissions.stub(:managed_enterprises) { [e1] } - permissions.order_cycle_exchanges(oc).should == [] - end - - it "does not return exchanges involving only the receiver" do - permissions.stub(:managed_enterprises) { [e2] } - permissions.order_cycle_exchanges(oc).should == [] - end - end - - ######################################## - - describe "finding related enterprises with a particular permission" do - let!(:er) { create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [permission]) } - - it "returns the enterprises" do - permissions.stub(:managed_enterprises) { e2 } - permissions.send(:related_enterprises_with, 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 == [] - end - end - end -end diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index 3a6c48e891..6cdf3db657 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -6,10 +6,10 @@ describe EnterpriseRelationship do let(:e2) { create(:enterprise, name: 'B') } let(:e3) { create(:enterprise, name: 'C') } - it "sorts by child, parent enterprise name" do - er1 = create(:enterprise_relationship, parent: e3, child: e1) - er2 = create(:enterprise_relationship, parent: e1, child: e2) - er3 = create(:enterprise_relationship, parent: e2, child: e1) + it "sorts by parent, child enterprise name" do + er1 = create(:enterprise_relationship, parent: e1, child: e3) + er2 = create(:enterprise_relationship, parent: e2, child: e1) + er3 = create(:enterprise_relationship, parent: e1, child: e2) EnterpriseRelationship.by_name.should == [er3, er1, er2] end @@ -43,24 +43,5 @@ describe EnterpriseRelationship do er.permissions.should be_empty end end - - it "finds relationships that grant permissions to some enterprises" do - er1 = create(:enterprise_relationship, parent: e2, child: e1) - er2 = create(:enterprise_relationship, parent: e3, child: e2) - er3 = create(:enterprise_relationship, parent: e1, child: e3) - - EnterpriseRelationship.permitting([e1, e2]).sort.should == [er1, er2] - end - - it "finds relationships that grant a particular permission" do - er1 = create(:enterprise_relationship, parent: e1, child: e2, - permissions_list: ['one', 'two']) - er2 = create(:enterprise_relationship, parent: e2, child: e3, - permissions_list: ['two', 'three']) - er3 = create(:enterprise_relationship, parent: e3, child: e1, - permissions_list: ['three', 'four']) - - EnterpriseRelationship.with_permission('two').sort.should == [er1, er2].sort - end end end From ba3f97ca1f4a7ee0463ef95c6f83e95a01ebf74e Mon Sep 17 00:00:00 2001 From: Rob H Date: Thu, 28 Aug 2014 14:45:24 +1000 Subject: [PATCH 35/46] Fixing enterprise relationships spec --- spec/features/admin/enterprise_relationships_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 20c7b28ac2..757f8bc2e6 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -38,16 +38,16 @@ feature %q{ visit admin_enterprise_relationships_path select 'One', from: 'enterprise_relationship_parent_id' - check 'can add products to order cycle from' - check 'can manage the products of' - uncheck 'can manage the products of' + #check 'can add products to order cycle from' + #check 'can manage the products of' + #uncheck 'can manage the products of' select 'Two', from: 'enterprise_relationship_child_id' click_button 'Create' - page.should have_relationship e1, e2, ['can add products to order cycle from'] + page.should have_relationship e1, e2 #, ['can add products to order cycle from'] er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first er.should be_present - er.permissions.map(&:name).should == ['add_products_to_order_cycle'] + #er.permissions.map(&:name).should == ['add_products_to_order_cycle'] end From 0b61872d964d4ddebb870aa27b722f93a5d028a8 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 Aug 2014 16:38:10 +1000 Subject: [PATCH 36/46] Revert 5ede8d1, reinstating a4be0ff..7b89e6a --- .../enterprise_relationships.js.coffee | 6 +- .../admin/order_cycles_controller.rb | 7 +- app/helpers/order_cycles_helper.rb | 14 ++- app/models/enterprise_relationship.rb | 10 +- .../_enterprise_relationship.html.haml | 4 +- .../enterprise_relationships/_form.html.haml | 15 ++- .../order_cycles/_exchange_form.html.haml | 2 +- app/views/admin/order_cycles/_form.html.haml | 4 +- app/views/admin/order_cycles/show.rep | 2 +- lib/open_food_network/permissions.rb | 37 ++++++ .../admin/enterprise_relationships_spec.rb | 21 ++-- spec/features/admin/order_cycles_spec.rb | 113 ++++++++++++------ .../enterprise_relationships_spec.js.coffee | 2 +- .../lib/open_food_network/permissions_spec.rb | 74 ++++++++++++ spec/models/enterprise_relationship_spec.rb | 27 ++++- 15 files changed, 263 insertions(+), 75 deletions(-) create mode 100644 lib/open_food_network/permissions.rb create mode 100644 spec/lib/open_food_network/permissions_spec.rb diff --git a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee index 7a799f2f28..cad556efd8 100644 --- a/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee +++ b/app/assets/javascripts/admin/services/enterprise_relationships.js.coffee @@ -2,7 +2,7 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris new class EnterpriseRelationships create_errors: "" all_permissions: [ - 'add_products_to_order_cycle' + 'add_to_order_cycle' 'manage_products' ] @@ -24,5 +24,5 @@ angular.module("ofn.admin").factory 'EnterpriseRelationships', ($http, enterpris permission_presentation: (permission) -> switch permission - when "add_products_to_order_cycle" then "can add products to order cycle from" - when "manage_products" then "can manage the products of" \ No newline at end of file + when "add_to_order_cycle" then "can add to order cycle" + when "manage_products" then "can manage the products of" diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 566c660112..63daf918de 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -1,7 +1,10 @@ +require 'open_food_network/permissions' require 'open_food_network/order_cycle_form_applicator' module Admin class OrderCyclesController < ResourceController + include OrderCyclesHelper + before_filter :load_order_cycle_set, :only => :index def show @@ -23,7 +26,7 @@ module Admin respond_to do |format| if @order_cycle.save - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go! + OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, order_cycle_permitted_enterprises).go! flash[:notice] = 'Your order cycle has been created.' format.html { redirect_to admin_order_cycles_path } @@ -40,7 +43,7 @@ module Admin respond_to do |format| if @order_cycle.update_attributes(params[:order_cycle]) - OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, managed_enterprises).go! + OpenFoodNetwork::OrderCycleFormApplicator.new(@order_cycle, order_cycle_permitted_enterprises).go! flash[:notice] = 'Your order cycle has been updated.' format.html { redirect_to admin_order_cycles_path } diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index 92cd968e0b..dfd957a143 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -3,8 +3,20 @@ module OrderCyclesHelper @current_order_cycle ||= current_order(false).andand.order_cycle end + def order_cycle_permitted_enterprises + OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_enterprises + end + + def order_cycle_producer_enterprises + order_cycle_permitted_enterprises.is_primary_producer.by_name + end + def coordinating_enterprises - Enterprise.is_distributor.managed_by(spree_current_user).order('name') + order_cycle_hub_enterprises + end + + def order_cycle_hub_enterprises + order_cycle_permitted_enterprises.is_distributor.by_name end def order_cycle_local_remote_class(distributor, order_cycle) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 5c10aafbbf..dba99cc7b3 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -9,12 +9,20 @@ class EnterpriseRelationship < ActiveRecord::Base scope :with_enterprises, joins('LEFT JOIN enterprises AS parent_enterprises ON parent_enterprises.id = enterprise_relationships.parent_id'). joins('LEFT JOIN enterprises AS child_enterprises ON child_enterprises.id = enterprise_relationships.child_id') - scope :by_name, with_enterprises.order('parent_enterprises.name, child_enterprises.name') scope :involving_enterprises, ->(enterprises) { where('parent_id IN (?) OR child_id IN (?)', enterprises, enterprises) } + scope :permitting, ->(enterprises) { where('child_id IN (?)', enterprises) } + + scope :with_permission, ->(permission) { + joins(:permissions). + where('enterprise_relationship_permissions.name = ?', permission) + } + + scope :by_name, with_enterprises.order('child_enterprises.name, parent_enterprises.name') + def permissions_list=(perms) perms.andand.each { |name| permissions.build name: name } diff --git a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml index 56f0d46c01..6a7dcdc7f4 100644 --- a/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml +++ b/app/views/admin/enterprise_relationships/_enterprise_relationship.html.haml @@ -1,8 +1,8 @@ -%td {{ enterprise_relationship.parent_name }} +%td {{ enterprise_relationship.child_name }} %td %ul %li{"ng-repeat" => "permission in enterprise_relationship.permissions"} {{ EnterpriseRelationships.permission_presentation(permission.name) }} -%td {{ enterprise_relationship.child_name }} +%td {{ enterprise_relationship.parent_name }} %td.actions %a.delete-enterprise-relationship.icon-trash.no-text{'ng-click' => 'delete(enterprise_relationship)'} diff --git a/app/views/admin/enterprise_relationships/_form.html.haml b/app/views/admin/enterprise_relationships/_form.html.haml index b9c2f265cd..86086c8781 100644 --- a/app/views/admin/enterprise_relationships/_form.html.haml +++ b/app/views/admin/enterprise_relationships/_form.html.haml @@ -1,14 +1,13 @@ %tr - %td - %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} - %td - permits - / %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} - / %label - / %input{type: "checkbox", "ng-model" => "permissions[permission]"} - / {{ EnterpriseRelationships.permission_presentation(permission) }} %td %select{name: "enterprise_relationship_child_id", "ng-model" => "child_id", "ng-options" => "e.id as e.name for e in Enterprises.all_enterprises"} + %td + %div{"ng-repeat" => "permission in EnterpriseRelationships.all_permissions"} + %label + %input{type: "checkbox", "ng-model" => "permissions[permission]"} + {{ EnterpriseRelationships.permission_presentation(permission) }} + %td + %select{name: "enterprise_relationship_parent_id", "ng-model" => "parent_id", "ng-options" => "e.id as e.name for e in Enterprises.my_enterprises"} %td.actions %input{type: "button", value: "Create", "ng-click" => "create()"} .errors {{ EnterpriseRelationships.create_errors }} diff --git a/app/views/admin/order_cycles/_exchange_form.html.haml b/app/views/admin/order_cycles/_exchange_form.html.haml index 3539892034..8f81b9f03f 100644 --- a/app/views/admin/order_cycles/_exchange_form.html.haml +++ b/app/views/admin/order_cycles/_exchange_form.html.haml @@ -23,4 +23,4 @@ = f.submit 'Add fee', 'ng-click' => 'addExchangeFee($event, exchange)' %td.actions - %a{'ng-click' => 'removeExchange($event, exchange)', :class => "icon-trash no-text"} + %a{'ng-click' => 'removeExchange($event, exchange)', :class => "icon-trash no-text remove-exchange"} diff --git a/app/views/admin/order_cycles/_form.html.haml b/app/views/admin/order_cycles/_form.html.haml index 68dfed9f1a..e859374a3b 100644 --- a/app/views/admin/order_cycles/_form.html.haml +++ b/app/views/admin/order_cycles/_form.html.haml @@ -25,7 +25,7 @@ %tr.products{'ng-show' => 'exchange.showProducts'} = render 'exchange_supplied_products_form' -= select_tag :new_supplier_id, options_from_collection_for_select(Enterprise.is_primary_producer.managed_by(spree_current_user).by_name, :id, :name), {'ng-model' => 'new_supplier_id'} += select_tag :new_supplier_id, options_from_collection_for_select(order_cycle_producer_enterprises, :id, :name), {'ng-model' => 'new_supplier_id'} = f.submit 'Add supplier', 'ng-click' => 'addSupplier($event)' @@ -50,7 +50,7 @@ %tr.products{'ng-show' => 'exchange.showProducts'} = render 'exchange_distributed_products_form' -= select_tag :new_distributor_id, options_from_collection_for_select(Enterprise.is_distributor.managed_by(spree_current_user).by_name, :id, :name), {'ng-model' => 'new_distributor_id'} += select_tag :new_distributor_id, options_from_collection_for_select(order_cycle_hub_enterprises, :id, :name), {'ng-model' => 'new_distributor_id'} = f.submit 'Add distributor', 'ng-click' => 'addDistributor($event)' .actions diff --git a/app/views/admin/order_cycles/show.rep b/app/views/admin/order_cycles/show.rep index 04fc659813..fb71602eb6 100644 --- a/app/views/admin/order_cycles/show.rep +++ b/app/views/admin/order_cycles/show.rep @@ -9,7 +9,7 @@ r.element :order_cycle, @order_cycle do r.element :id end - r.list_of :exchanges, @order_cycle.exchanges.managed_by(spree_current_user).order('id ASC') do |exchange| + r.list_of :exchanges, OpenFoodNetwork::Permissions.new(spree_current_user).order_cycle_exchanges(@order_cycle).order('id ASC') do |exchange| r.element :id r.element :sender_id r.element :receiver_id diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb new file mode 100644 index 0000000000..99c0aa0c15 --- /dev/null +++ b/lib/open_food_network/permissions.rb @@ -0,0 +1,37 @@ +module OpenFoodNetwork + class Permissions + def initialize(user) + @user = user + end + + # Find enterprises that an admin is allowed to add to an order cycle + def order_cycle_enterprises + managed_enterprise_ids = managed_enterprises.pluck :id + permitted_enterprise_ids = related_enterprises_with(:add_to_order_cycle).pluck :id + + Enterprise.where('id IN (?)', managed_enterprise_ids + permitted_enterprise_ids) + end + + # Find the exchanges of an order cycle that an admin can manage + def order_cycle_exchanges(order_cycle) + enterprises = managed_enterprises + related_enterprises_with(:add_to_order_cycle) + order_cycle.exchanges.to_enterprises(enterprises).from_enterprises(enterprises) + end + + + private + + def managed_enterprises + Enterprise.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 + end +end diff --git a/spec/features/admin/enterprise_relationships_spec.rb b/spec/features/admin/enterprise_relationships_spec.rb index 757f8bc2e6..7317f44066 100644 --- a/spec/features/admin/enterprise_relationships_spec.rb +++ b/spec/features/admin/enterprise_relationships_spec.rb @@ -14,9 +14,9 @@ feature %q{ scenario "listing relationships" do # Given some enterprises with relationships e1, e2, e3, e4 = create(:enterprise), create(:enterprise), create(:enterprise), create(:enterprise) - create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [:add_products_to_order_cycle]) + create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [:add_to_order_cycle]) create(:enterprise_relationship, parent: e2, child: e3, permissions_list: [:manage_products]) - create(:enterprise_relationship, parent: e3, child: e4, permissions_list: [:add_products_to_order_cycle, :manage_products]) + create(:enterprise_relationship, parent: e3, child: e4, permissions_list: [:add_to_order_cycle, :manage_products]) # When I go to the relationships page click_link 'Enterprises' @@ -24,10 +24,10 @@ feature %q{ # Then I should see the relationships within('table#enterprise-relationships') do - page.should have_relationship e1, e2, ['can add products to order cycle from'] + page.should have_relationship e1, e2, ['can add to order cycle'] page.should have_relationship e2, e3, ['can manage the products of'] page.should have_relationship e3, e4, - ['can add products to order cycle from', 'can manage the products of'] + ['can add to order cycle', 'can manage the products of'] end end @@ -38,16 +38,17 @@ feature %q{ visit admin_enterprise_relationships_path select 'One', from: 'enterprise_relationship_parent_id' - #check 'can add products to order cycle from' - #check 'can manage the products of' - #uncheck 'can manage the products of' + + check 'can add to order cycle' + check 'can manage the products of' + uncheck 'can manage the products of' select 'Two', from: 'enterprise_relationship_child_id' click_button 'Create' - page.should have_relationship e1, e2 #, ['can add products to order cycle from'] + page.should have_relationship e1, e2, ['can add to order cycle'] er = EnterpriseRelationship.where(parent_id: e1, child_id: e2).first er.should be_present - #er.permissions.map(&:name).should == ['add_products_to_order_cycle'] + er.permissions.map(&:name).should == ['add_to_order_cycle'] end @@ -119,6 +120,6 @@ feature %q{ def have_relationship(parent, child, perms=[]) perms = perms.join(' ') || 'permits' - have_table_row [parent.name, perms, child.name, ''] + have_table_row [child.name, perms, parent.name, ''] end end diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 2423a6dc05..76ee3bdfc2 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -436,26 +436,37 @@ feature %q{ context "as an enterprise user" do - let(:supplier1) { create(:supplier_enterprise, name: 'First Supplier') } - let(:supplier2) { create(:supplier_enterprise, name: 'Another Supplier') } - let(:distributor1) { create(:distributor_enterprise, name: 'First Distributor') } - let(:distributor2) { create(:distributor_enterprise, name: 'Another Distributor') } - let!(:distributor1_fee) { create(:enterprise_fee, enterprise: distributor1, name: 'First Distributor Fee') } - before(:each) do - product = create(:product, supplier: supplier1) - product.distributors << distributor1 + let!(:supplier_managed) { create(:supplier_enterprise, name: 'Managed supplier') } + let!(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Unmanaged supplier') } + let!(:supplier_permitted) { create(:supplier_enterprise, name: 'Permitted supplier') } + let!(:distributor_managed) { create(:distributor_enterprise, name: 'Managed distributor') } + let!(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Unmanaged Distributor') } + let!(:distributor_permitted) { create(:distributor_enterprise, name: 'Permitted distributor') } + let!(:distributor_managed_fee) { create(:enterprise_fee, enterprise: distributor_managed, name: 'Managed distributor fee') } + let!(:supplier_permitted_relationship) do + create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed, + permissions_list: [:add_to_order_cycle]) + end + let!(:distributor_permitted_relationship) do + create(:enterprise_relationship, parent: distributor_permitted, child: distributor_managed, + permissions_list: [:add_to_order_cycle]) + end + + before do + product = create(:product, supplier: supplier_managed) + product.distributors << distributor_managed product.save! @new_user = create_enterprise_user - @new_user.enterprise_roles.build(enterprise: supplier1).save - @new_user.enterprise_roles.build(enterprise: distributor1).save + @new_user.enterprise_roles.build(enterprise: supplier_managed).save + @new_user.enterprise_roles.build(enterprise: distributor_managed).save login_to_admin_as @new_user end scenario "viewing a list of order cycles I am coordinating" do - oc_user_coordinating = create(:simple_order_cycle, { suppliers: [supplier1, supplier2], coordinator: supplier1, distributors: [distributor1, distributor2], name: 'Order Cycle 1' } ) - oc_for_other_user = create(:simple_order_cycle, { coordinator: supplier2, name: 'Order Cycle 2' } ) + oc_user_coordinating = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_unmanaged], name: 'Order Cycle 1' } ) + oc_for_other_user = create(:simple_order_cycle, { coordinator: supplier_unmanaged, name: 'Order Cycle 2' } ) click_link "Order Cycles" @@ -464,8 +475,8 @@ feature %q{ page.should_not have_content oc_for_other_user.name # The order cycle should not show enterprises that I don't manage - page.should_not have_selector 'td.suppliers', text: supplier2.name - page.should_not have_selector 'td.distributors', text: distributor2.name + page.should_not have_selector 'td.suppliers', text: supplier_unmanaged.name + page.should_not have_selector 'td.distributors', text: distributor_unmanaged.name end scenario "creating a new order cycle" do @@ -476,57 +487,81 @@ feature %q{ fill_in 'order_cycle_orders_open_at', with: '2012-11-06 06:00:00' fill_in 'order_cycle_orders_close_at', with: '2012-11-13 17:00:00' - select 'First Supplier', from: 'new_supplier_id' + select 'Managed supplier', from: 'new_supplier_id' + click_button 'Add supplier' + select 'Permitted supplier', from: 'new_supplier_id' click_button 'Add supplier' - select 'First Distributor', from: 'order_cycle_coordinator_id' + select 'Managed distributor', from: 'order_cycle_coordinator_id' click_button 'Add coordinator fee' - select 'First Distributor Fee', from: 'order_cycle_coordinator_fee_0_id' + select 'Managed distributor fee', from: 'order_cycle_coordinator_fee_0_id' - select 'First Distributor', from: 'new_distributor_id' + select 'Managed distributor', from: 'new_distributor_id' + click_button 'Add distributor' + select 'Permitted distributor', from: 'new_distributor_id' click_button 'Add distributor' - # Should only have suppliers / distributors listed which the user can manage - within "#new_supplier_id" do - page.should_not have_content supplier2.name - end - within "#new_distributor_id" do - page.should_not have_content distributor2.name - end - within "#order_cycle_coordinator_id" do - page.should_not have_content distributor2.name - page.should_not have_content supplier1.name - page.should_not have_content supplier2.name + # Should only have suppliers / distributors listed which the user is managing or + # has E2E permission to add products to order cycles + page.should_not have_select 'new_supplier_id', with_options: [supplier_unmanaged.name] + page.should_not have_select 'new_distributor_id', with_options: [distributor_unmanaged.name] + + [distributor_unmanaged.name, supplier_managed.name, supplier_unmanaged.name].each do |enterprise_name| + page.should_not have_select 'order_cycle_coordinator_id', with_options: [enterprise_name] end click_button 'Create' flash_message.should == "Your order cycle has been created." order_cycle = OrderCycle.find_by_name('My order cycle') - order_cycle.coordinator.should == distributor1 + order_cycle.suppliers.sort.should == [supplier_managed, supplier_permitted].sort + order_cycle.coordinator.should == distributor_managed + order_cycle.distributors.sort.should == [distributor_managed, distributor_permitted].sort end - scenario "editing an order cycle" do - oc = create(:simple_order_cycle, { suppliers: [supplier1, supplier2], coordinator: supplier1, distributors: [distributor1, distributor2], name: 'Order Cycle 1' } ) + scenario "editing an order cycle does not affect exchanges we don't manage" do + oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_permitted, distributor_unmanaged], name: 'Order Cycle 1' } ) visit edit_admin_order_cycle_path(oc) - # I should not see exchanges for supplier2 or distributor2 - page.all('tr.supplier').count.should == 1 - page.all('tr.distributor').count.should == 1 + # I should not see exchanges for supplier_unmanaged or distributor_unmanaged + page.all('tr.supplier').count.should == 2 + page.all('tr.distributor').count.should == 2 # When I save, then those exchanges should remain click_button 'Update' page.should have_content "Your order cycle has been updated." oc.reload - oc.suppliers.sort.should == [supplier1, supplier2] - oc.coordinator.should == supplier1 - oc.distributors.sort.should == [distributor1, distributor2] + oc.suppliers.sort.should == [supplier_managed, supplier_permitted, supplier_unmanaged].sort + oc.coordinator.should == supplier_managed + oc.distributors.sort.should == [distributor_managed, distributor_permitted, distributor_unmanaged].sort end + scenario "editing an order cycle" do + oc = create(:simple_order_cycle, { suppliers: [supplier_managed, supplier_permitted, supplier_unmanaged], coordinator: supplier_managed, distributors: [distributor_managed, distributor_permitted, distributor_unmanaged], name: 'Order Cycle 1' } ) + + visit edit_admin_order_cycle_path(oc) + + # When I remove all the exchanges and save + page.find("tr.supplier-#{supplier_managed.id} a.remove-exchange").click + page.find("tr.supplier-#{supplier_permitted.id} a.remove-exchange").click + page.find("tr.distributor-#{distributor_managed.id} a.remove-exchange").click + page.find("tr.distributor-#{distributor_permitted.id} a.remove-exchange").click + click_button 'Update' + + # Then the exchanges should be removed + page.should have_content "Your order cycle has been updated." + + oc.reload + oc.suppliers.should == [supplier_unmanaged] + oc.coordinator.should == supplier_managed + oc.distributors.should == [distributor_unmanaged] + end + + scenario "cloning an order cycle" do - oc = create(:simple_order_cycle) + oc = create(:simple_order_cycle, coordinator: distributor_managed) click_link "Order Cycles" first('a.clone-order-cycle').click diff --git a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee index 4e90f65335..9701377a73 100644 --- a/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/enterprise_relationships_spec.js.coffee @@ -12,5 +12,5 @@ describe "enterprise relationships", -> EnterpriseRelationships = _EnterpriseRelationships_ it "presents permission names", -> - expect(EnterpriseRelationships.permission_presentation("add_products_to_order_cycle")).toEqual "can add products to order cycle from" + expect(EnterpriseRelationships.permission_presentation("add_to_order_cycle")).toEqual "can add to order cycle" expect(EnterpriseRelationships.permission_presentation("manage_products")).toEqual "can manage the products of" diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb new file mode 100644 index 0000000000..b0a004cbf1 --- /dev/null +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -0,0 +1,74 @@ +require 'open_food_network/permissions' + +module OpenFoodNetwork + describe Permissions do + let(:user) { double(:user) } + let(:permissions) { Permissions.new(user) } + let(:permission) { 'one' } + let(:e1) { create(:enterprise) } + let(:e2) { create(:enterprise) } + + describe "finding enterprises that can be added to an order cycle" do + before do + permissions.stub(:managed_enterprises) { Enterprise.where('1=0') } + permissions.stub(:related_enterprises_with) { Enterprise.where('1=0') } + end + + it "returns managed enterprises" do + permissions.stub(:managed_enterprises) { Enterprise.where(id: e1) } + permissions.order_cycle_enterprises.should == [e1] + end + + it "returns permitted enterprises" do + permissions.stub(:related_enterprises_with) { Enterprise.where(id: e2) } + permissions.order_cycle_enterprises.should == [e2] + end + end + + describe "finding exchanges of an order cycle that an admin can manage" do + let(:oc) { create(:simple_order_cycle) } + let!(:ex) { create(:exchange, order_cycle: oc, sender: e1, receiver: e2) } + + before do + permissions.stub(:managed_enterprises) { [] } + permissions.stub(:related_enterprises_with) { [] } + end + + it "returns exchanges involving enterprises managed by the user" do + permissions.stub(:managed_enterprises) { [e1, e2] } + permissions.order_cycle_exchanges(oc).should == [ex] + end + + it "returns exchanges involving enterprises with E2E permission" do + permissions.stub(:related_enterprises_with) { [e1, e2] } + permissions.order_cycle_exchanges(oc).should == [ex] + end + + it "does not return exchanges involving only the sender" do + permissions.stub(:managed_enterprises) { [e1] } + permissions.order_cycle_exchanges(oc).should == [] + end + + it "does not return exchanges involving only the receiver" do + permissions.stub(:managed_enterprises) { [e2] } + permissions.order_cycle_exchanges(oc).should == [] + end + end + + ######################################## + + describe "finding related enterprises with a particular permission" do + let!(:er) { create(:enterprise_relationship, parent: e1, child: e2, permissions_list: [permission]) } + + it "returns the enterprises" do + permissions.stub(:managed_enterprises) { e2 } + permissions.send(:related_enterprises_with, 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 == [] + end + end + end +end diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index 6cdf3db657..3a6c48e891 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -6,10 +6,10 @@ describe EnterpriseRelationship do let(:e2) { create(:enterprise, name: 'B') } let(:e3) { create(:enterprise, name: 'C') } - it "sorts by parent, child enterprise name" do - er1 = create(:enterprise_relationship, parent: e1, child: e3) - er2 = create(:enterprise_relationship, parent: e2, child: e1) - er3 = create(:enterprise_relationship, parent: e1, child: e2) + it "sorts by child, parent enterprise name" do + er1 = create(:enterprise_relationship, parent: e3, child: e1) + er2 = create(:enterprise_relationship, parent: e1, child: e2) + er3 = create(:enterprise_relationship, parent: e2, child: e1) EnterpriseRelationship.by_name.should == [er3, er1, er2] end @@ -43,5 +43,24 @@ describe EnterpriseRelationship do er.permissions.should be_empty end end + + it "finds relationships that grant permissions to some enterprises" do + er1 = create(:enterprise_relationship, parent: e2, child: e1) + er2 = create(:enterprise_relationship, parent: e3, child: e2) + er3 = create(:enterprise_relationship, parent: e1, child: e3) + + EnterpriseRelationship.permitting([e1, e2]).sort.should == [er1, er2] + end + + it "finds relationships that grant a particular permission" do + er1 = create(:enterprise_relationship, parent: e1, child: e2, + permissions_list: ['one', 'two']) + er2 = create(:enterprise_relationship, parent: e2, child: e3, + permissions_list: ['two', 'three']) + er3 = create(:enterprise_relationship, parent: e3, child: e1, + permissions_list: ['three', 'four']) + + EnterpriseRelationship.with_permission('two').sort.should == [er1, er2].sort + end end end From 7f74854a2f72cf668f45c1069ca74d193d8774d3 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 28 Aug 2014 15:06:55 +1000 Subject: [PATCH 37/46] For OC, fetch all enterprises we have access to, including those via E2E relationships --- .../javascripts/admin/order_cycle.js.erb.coffee | 2 +- app/controllers/admin/enterprises_controller.rb | 7 ++++++- app/models/spree/ability_decorator.rb | 1 + .../{index.rabl => for_order_cycle.rabl} | 0 config/routes.rb | 7 +++++-- spec/features/admin/order_cycles_spec.rb | 16 ++++++++++++---- spec/javascripts/unit/order_cycle_spec.js.coffee | 2 +- spec/models/spree/ability_spec.rb | 2 +- 8 files changed, 27 insertions(+), 10 deletions(-) rename app/views/admin/enterprises/{index.rabl => for_order_cycle.rabl} (100%) diff --git a/app/assets/javascripts/admin/order_cycle.js.erb.coffee b/app/assets/javascripts/admin/order_cycle.js.erb.coffee index aadf66af60..b8068681ca 100644 --- a/app/assets/javascripts/admin/order_cycle.js.erb.coffee +++ b/app/assets/javascripts/admin/order_cycle.js.erb.coffee @@ -330,7 +330,7 @@ angular.module('order_cycle', ['ngResource']) }]) .factory('Enterprise', ['$resource', ($resource) -> - Enterprise = $resource('/admin/enterprises/:enterprise_id.json', {}, {'index': {method: 'GET', isArray: true}}) + Enterprise = $resource('/admin/enterprises/for_order_cycle/:enterprise_id.json', {}, {'index': {method: 'GET', isArray: true}}) { Enterprise: Enterprise diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 53e58d110b..85fe6831fb 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -6,6 +6,11 @@ module Admin create.after :grant_management helper 'spree/products' + include OrderCyclesHelper + + def for_order_cycle + @collection = order_cycle_permitted_enterprises + end def bulk_update @@ -53,7 +58,7 @@ module Admin end def collection_actions - [:index, :bulk_update] + [:index, :for_order_cycle, :bulk_update] end def load_methods_and_fees diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index b29aa6d5b3..68d98bd568 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -76,6 +76,7 @@ class AbilityDecorator can [:admin, :index, :read, :edit, :update, :bulk_update, :clone], OrderCycle do |order_cycle| user.enterprises.include? order_cycle.coordinator end + can [:for_order_cycle], Enterprise can [:index, :create], EnterpriseFee can [:admin, :read, :edit, :bulk_update, :destroy], EnterpriseFee do |enterprise_fee| diff --git a/app/views/admin/enterprises/index.rabl b/app/views/admin/enterprises/for_order_cycle.rabl similarity index 100% rename from app/views/admin/enterprises/index.rabl rename to app/views/admin/enterprises/for_order_cycle.rabl diff --git a/config/routes.rb b/config/routes.rb index 36213b2622..5230e798cc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -33,12 +33,15 @@ Openfoodnetwork::Application.routes.draw do namespace :admin do resources :order_cycles do - post :bulk_update, :on => :collection, :as => :bulk_update + post :bulk_update, on: :collection, as: :bulk_update get :clone, on: :member end resources :enterprises do - post :bulk_update, :on => :collection, :as => :bulk_update + collection do + get :for_order_cycle + post :bulk_update, as: :bulk_update + end resources :producer_properties do post :update_positions, on: :collection diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index 76ee3bdfc2..febd966109 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -451,12 +451,10 @@ feature %q{ create(:enterprise_relationship, parent: distributor_permitted, child: distributor_managed, permissions_list: [:add_to_order_cycle]) end + let!(:product_managed) { create(:product, supplier: supplier_managed) } + let!(:product_permitted) { create(:product, supplier: supplier_permitted) } before do - product = create(:product, supplier: supplier_managed) - product.distributors << distributor_managed - product.save! - @new_user = create_enterprise_user @new_user.enterprise_roles.build(enterprise: supplier_managed).save @new_user.enterprise_roles.build(enterprise: distributor_managed).save @@ -492,6 +490,9 @@ feature %q{ select 'Permitted supplier', from: 'new_supplier_id' click_button 'Add supplier' + select_incoming_variant supplier_managed, 0, product_managed.master + select_incoming_variant supplier_permitted, 1, product_permitted.master + select 'Managed distributor', from: 'order_cycle_coordinator_id' click_button 'Add coordinator fee' select 'Managed distributor fee', from: 'order_cycle_coordinator_fee_0_id' @@ -574,4 +575,11 @@ feature %q{ end + + private + + def select_incoming_variant(supplier, exchange_no, variant) + page.find("table.exchanges tr.supplier-#{supplier.id} td.products input").click + check "order_cycle_incoming_exchange_#{exchange_no}_variants_#{variant.id}" + end end diff --git a/spec/javascripts/unit/order_cycle_spec.js.coffee b/spec/javascripts/unit/order_cycle_spec.js.coffee index 172b2cdb1a..0d5127314a 100644 --- a/spec/javascripts/unit/order_cycle_spec.js.coffee +++ b/spec/javascripts/unit/order_cycle_spec.js.coffee @@ -327,7 +327,7 @@ describe 'OrderCycle services', -> inject ($injector, _$httpBackend_)-> Enterprise = $injector.get('Enterprise') $httpBackend = _$httpBackend_ - $httpBackend.whenGET('/admin/enterprises.json').respond [ + $httpBackend.whenGET('/admin/enterprises/for_order_cycle.json').respond [ {id: 1, name: 'One', supplied_products: [1, 2]} {id: 2, name: 'Two', supplied_products: [3, 4]} {id: 3, name: 'Three', supplied_products: [5, 6]} diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index a2d1fb5bdf..9e5ae49780 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -264,7 +264,7 @@ module Spree end it 'should have the ability administrate and create enterpises' do - should have_ability([:admin, :index, :create], for: Enterprise) + should have_ability([:admin, :index, :for_order_cycle, :create], for: Enterprise) end end end From 62e6cacfd0d7db7e043c9b6aa77cf834d37e2c85 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 Aug 2014 17:37:56 +1000 Subject: [PATCH 38/46] Rename spec to match view name change --- .../{index.rabl_spec.rb => for_order_cycle.rabl_spec.rb} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename spec/views/admin/enterprises/{index.rabl_spec.rb => for_order_cycle.rabl_spec.rb} (79%) diff --git a/spec/views/admin/enterprises/index.rabl_spec.rb b/spec/views/admin/enterprises/for_order_cycle.rabl_spec.rb similarity index 79% rename from spec/views/admin/enterprises/index.rabl_spec.rb rename to spec/views/admin/enterprises/for_order_cycle.rabl_spec.rb index 91463c2212..1585d08e43 100644 --- a/spec/views/admin/enterprises/index.rabl_spec.rb +++ b/spec/views/admin/enterprises/for_order_cycle.rabl_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' -describe "admin/enterprises/index.rabl" do +describe "admin/enterprises/for_order_cycle.rabl" do let(:enterprise) { create(:distributor_enterprise) } let!(:product) { create(:simple_product, supplier: enterprise) } let!(:deleted_product) { create(:simple_product, supplier: enterprise, deleted_at: 1.day.ago) } - let(:render) { Rabl.render([enterprise], 'admin/enterprises/index', view_path: 'app/views', scope: RablHelper::FakeContext.instance) } + let(:render) { Rabl.render([enterprise], 'admin/enterprises/for_order_cycle', view_path: 'app/views', scope: RablHelper::FakeContext.instance) } describe "supplied products" do it "does not render deleted products" do From 66f20a6b8a8acf3d6e4c52ad1f205f478b2d11d5 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 09:17:38 +1000 Subject: [PATCH 39/46] Name test enterprises semantically --- .../admin/bulk_product_update_spec.rb | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index ccddba5508..d1dfc2b129 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -729,20 +729,20 @@ feature %q{ end context "as an enterprise manager" do - let(:s1) { create(:supplier_enterprise, name: 'First Supplier') } - let(:s2) { create(:supplier_enterprise, name: 'Another Supplier') } - let(:s3) { create(:supplier_enterprise, name: 'Yet Another Supplier') } - let(:d1) { create(:distributor_enterprise, name: 'First Distributor') } - let(:d2) { create(:distributor_enterprise, name: 'Another Distributor') } - let!(:product_supplied) { create(:product, supplier: s1, price: 10.0, on_hand: 6) } - let!(:product_not_supplied) { create(:product, supplier: s3) } - let(:product_supplied_inactive) { create(:product, supplier: s1, price: 10.0, on_hand: 6, available_on: 1.week.from_now) } + let(:supplier_managed1) { create(:supplier_enterprise, name: 'Supplier Managed 1') } + let(:supplier_managed2) { create(:supplier_enterprise, name: 'Supplier Managed 2') } + let(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Supplier Unmanaged') } + let(:distributor_managed) { create(:distributor_enterprise, name: 'Distributor Managed') } + let(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Distributor Unmanaged') } + let!(:product_supplied) { create(:product, supplier: supplier_managed1, price: 10.0, on_hand: 6) } + let!(:product_not_supplied) { create(:product, supplier: supplier_unmanaged) } + let(:product_supplied_inactive) { create(:product, supplier: supplier_managed1, price: 10.0, on_hand: 6, available_on: 1.week.from_now) } - before(:each) do + before do @enterprise_user = create_enterprise_user - @enterprise_user.enterprise_roles.build(enterprise: s1).save - @enterprise_user.enterprise_roles.build(enterprise: s2).save - @enterprise_user.enterprise_roles.build(enterprise: d1).save + @enterprise_user.enterprise_roles.build(enterprise: supplier_managed1).save + @enterprise_user.enterprise_roles.build(enterprise: supplier_managed2).save + @enterprise_user.enterprise_roles.build(enterprise: distributor_managed).save login_to_admin_as @enterprise_user end @@ -757,8 +757,8 @@ feature %q{ it "shows only suppliers that I manage" do visit '/admin/products/bulk_edit' - expect(page).to have_select 'producer', with_options: [s1.name, s2.name], selected: s1.name - expect(page).to have_no_select 'producer', with_options: [s3.name] + expect(page).to have_select 'producer', with_options: [supplier_managed1.name, supplier_managed2.name], selected: supplier_managed1.name + expect(page).to have_no_select 'producer', with_options: [supplier_unmanaged.name] end it "shows inactive products that I supply" do @@ -777,13 +777,13 @@ feature %q{ first("div#columns_dropdown div.menu div.menu_item", text: "Available On").click expect(page).to have_field "product_name", with: p.name - expect(page).to have_select "producer", selected: s1.name + expect(page).to have_select "producer", selected: supplier_managed1.name expect(page).to have_field "available_on", with: p.available_on.strftime("%F %T") expect(page).to have_field "price", with: "10.0" expect(page).to have_field "on_hand", with: "6" fill_in "product_name", with: "Big Bag Of Potatoes" - select(s2.name, :from => 'producer') + select(supplier_managed2.name, :from => 'producer') fill_in "available_on", with: (Date.today-3).strftime("%F %T") fill_in "price", with: "20" select "Weight (kg)", from: "variant_unit_with_scale" @@ -795,7 +795,7 @@ feature %q{ p.reload expect(p.name).to eq "Big Bag Of Potatoes" - expect(p.supplier).to eq s2 + expect(p.supplier).to eq supplier_managed2 expect(p.variant_unit).to eq "weight" expect(p.variant_unit_scale).to eq 1000 # Kg expect(p.available_on).to eq 3.days.ago.beginning_of_day From e0645dfbd91e12d16e2833df985332bcf9b11245 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 09:42:50 +1000 Subject: [PATCH 40/46] Fetch managed products via OpenFoodNetwork::Permissions --- .../spree/api/products_controller_decorator.rb | 8 +++++++- lib/open_food_network/permissions.rb | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/api/products_controller_decorator.rb b/app/controllers/spree/api/products_controller_decorator.rb index 7e81e1f789..65765e03ca 100644 --- a/app/controllers/spree/api/products_controller_decorator.rb +++ b/app/controllers/spree/api/products_controller_decorator.rb @@ -1,3 +1,5 @@ +require 'open_food_network/permissions' + Spree::Api::ProductsController.class_eval do def managed authorize! :admin, Spree::Product @@ -8,7 +10,11 @@ Spree::Api::ProductsController.class_eval do end def bulk_products - @products = product_scope.ransack(params[:q]).result.managed_by(current_api_user).page(params[:page]).per(params[:per_page]) + @products = OpenFoodNetwork::Permissions.new(current_api_user).managed_products. + merge(product_scope). + ransack(params[:q]).result. + page(params[:page]).per(params[:per_page]) + render text: { products: ActiveModel::ArraySerializer.new(@products, each_serializer: Spree::Api::ProductSerializer), pages: @products.num_pages }.to_json end diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index 99c0aa0c15..fbd85d933b 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -18,6 +18,10 @@ module OpenFoodNetwork order_cycle.exchanges.to_enterprises(enterprises).from_enterprises(enterprises) end + def managed_products + Spree::Product.managed_by(@user) + end + private From cfb31b46e48fb765976b27c04b99ffcd8ceba997 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 10:13:59 +1000 Subject: [PATCH 41/46] Bulk product edit lists managed products --- lib/open_food_network/permissions.rb | 12 ++++++- .../admin/bulk_product_update_spec.rb | 26 +++++++++++----- .../lib/open_food_network/permissions_spec.rb | 31 +++++++++++++++++++ 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index fbd85d933b..f35a7db602 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -19,7 +19,9 @@ module OpenFoodNetwork end def managed_products - Spree::Product.managed_by(@user) + managed_enterprise_products_ids = managed_enterprise_products.pluck :id + permitted_enterprise_products_ids = related_enterprise_products.pluck :id + Spree::Product.where('id IN (?)', managed_enterprise_products_ids + permitted_enterprise_products_ids) end @@ -37,5 +39,13 @@ module OpenFoodNetwork Enterprise.where('id IN (?)', parent_ids) end + + def managed_enterprise_products + Spree::Product.managed_by(@user) + end + + def related_enterprise_products + Spree::Product.where('supplier_id IN (?)', related_enterprises_with(:manage_products)) + end end end diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index d1dfc2b129..90f83bb26d 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -732,12 +732,21 @@ feature %q{ let(:supplier_managed1) { create(:supplier_enterprise, name: 'Supplier Managed 1') } let(:supplier_managed2) { create(:supplier_enterprise, name: 'Supplier Managed 2') } let(:supplier_unmanaged) { create(:supplier_enterprise, name: 'Supplier Unmanaged') } + let(:supplier_permitted) { create(:supplier_enterprise, name: 'Supplier Permitted') } let(:distributor_managed) { create(:distributor_enterprise, name: 'Distributor Managed') } let(:distributor_unmanaged) { create(:distributor_enterprise, name: 'Distributor Unmanaged') } let!(:product_supplied) { create(:product, supplier: supplier_managed1, price: 10.0, on_hand: 6) } let!(:product_not_supplied) { create(:product, supplier: supplier_unmanaged) } + let!(:product_supplied_permitted) { create(:product, name: 'Product Permitted', supplier: supplier_permitted, price: 10.0, on_hand: 6) } let(:product_supplied_inactive) { create(:product, supplier: supplier_managed1, price: 10.0, on_hand: 6, available_on: 1.week.from_now) } + let!(:supplier_permitted_relationship) do + create(:enterprise_relationship, parent: supplier_permitted, child: supplier_managed1, + permissions_list: [:manage_products]) + end + + use_short_wait + before do @enterprise_user = create_enterprise_user @enterprise_user.enterprise_roles.build(enterprise: supplier_managed1).save @@ -751,6 +760,7 @@ feature %q{ visit '/admin/products/bulk_edit' expect(page).to have_field 'product_name', with: product_supplied.name + expect(page).to have_field 'product_name', with: product_supplied_permitted.name expect(page).to have_no_field 'product_name', with: product_not_supplied.name end @@ -782,13 +792,15 @@ feature %q{ expect(page).to have_field "price", with: "10.0" expect(page).to have_field "on_hand", with: "6" - fill_in "product_name", with: "Big Bag Of Potatoes" - select(supplier_managed2.name, :from => 'producer') - fill_in "available_on", with: (Date.today-3).strftime("%F %T") - fill_in "price", with: "20" - select "Weight (kg)", from: "variant_unit_with_scale" - fill_in "on_hand", with: "18" - fill_in "display_as", with: "Big Bag" + within("tr#p_#{product_supplied.id}") do + fill_in "product_name", with: "Big Bag Of Potatoes" + select(supplier_managed2.name, :from => 'producer') + fill_in "available_on", with: (Date.today-3).strftime("%F %T") + fill_in "price", with: "20" + select "Weight (kg)", from: "variant_unit_with_scale" + fill_in "on_hand", with: "18" + fill_in "display_as", with: "Big Bag" + end click_button 'Save Changes' expect(page.find("#update-status-message")).to have_content "Changes saved." diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index b0a004cbf1..bbab0d7175 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -55,6 +55,26 @@ module OpenFoodNetwork end end + describe "finding managed products" do + let!(:p1) { create(:simple_product) } + let!(:p2) { create(:simple_product) } + + before do + permissions.stub(:managed_enterprise_products) { Spree::Product.where('1=0') } + permissions.stub(:related_enterprise_products) { Spree::Product.where('1=0') } + end + + it "returns products produced by managed enterprises" do + permissions.stub(:managed_enterprise_products) { Spree::Product.where(id: p1) } + permissions.managed_products.should == [p1] + end + + it "returns products produced by permitted enterprises" do + permissions.stub(:related_enterprise_products) { Spree::Product.where(id: p2) } + permissions.managed_products.should == [p2] + end + end + ######################################## describe "finding related enterprises with a particular permission" do @@ -70,5 +90,16 @@ module OpenFoodNetwork permissions.send(:related_enterprises_with, permission).should == [] end end + + describe "finding the supplied products of related enterprises" do + let!(:e) { create(:enterprise) } + let!(:p) { create(:simple_product, supplier: e) } + + it "returns supplied products" do + permissions.should_receive(:related_enterprises_with).with(:manage_products) { [e] } + + permissions.send(:related_enterprise_products).should == [p] + end + end end end From 4d8d74dec7d230660ef9bce6e94da56272de1f4b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 10:36:59 +1000 Subject: [PATCH 42/46] Find enterprises that we manage products for --- lib/open_food_network/permissions.rb | 17 +++++-- .../lib/open_food_network/permissions_spec.rb | 49 ++++++++++++++----- 2 files changed, 51 insertions(+), 15 deletions(-) diff --git a/lib/open_food_network/permissions.rb b/lib/open_food_network/permissions.rb index f35a7db602..3d5df1e804 100644 --- a/lib/open_food_network/permissions.rb +++ b/lib/open_food_network/permissions.rb @@ -6,10 +6,7 @@ module OpenFoodNetwork # Find enterprises that an admin is allowed to add to an order cycle def order_cycle_enterprises - managed_enterprise_ids = managed_enterprises.pluck :id - permitted_enterprise_ids = related_enterprises_with(:add_to_order_cycle).pluck :id - - Enterprise.where('id IN (?)', managed_enterprise_ids + permitted_enterprise_ids) + managed_and_related_enterprises_with :add_to_order_cycle end # Find the exchanges of an order cycle that an admin can manage @@ -24,6 +21,10 @@ module OpenFoodNetwork Spree::Product.where('id IN (?)', managed_enterprise_products_ids + permitted_enterprise_products_ids) end + def managed_product_enterprises + managed_and_related_enterprises_with :manage_products + end + private @@ -40,6 +41,14 @@ module OpenFoodNetwork Enterprise.where('id IN (?)', parent_ids) end + def managed_and_related_enterprises_with(permission) + managed_enterprise_ids = managed_enterprises.pluck :id + permitted_enterprise_ids = related_enterprises_with(permission).pluck :id + + Enterprise.where('id IN (?)', managed_enterprise_ids + permitted_enterprise_ids) + end + + def managed_enterprise_products Spree::Product.managed_by(@user) end diff --git a/spec/lib/open_food_network/permissions_spec.rb b/spec/lib/open_food_network/permissions_spec.rb index bbab0d7175..bcd1f8b742 100644 --- a/spec/lib/open_food_network/permissions_spec.rb +++ b/spec/lib/open_food_network/permissions_spec.rb @@ -9,19 +9,15 @@ module OpenFoodNetwork let(:e2) { create(:enterprise) } describe "finding enterprises that can be added to an order cycle" do - before do - permissions.stub(:managed_enterprises) { Enterprise.where('1=0') } - permissions.stub(:related_enterprises_with) { Enterprise.where('1=0') } - end + let(:e) { double(:enterprise) } - it "returns managed enterprises" do - permissions.stub(:managed_enterprises) { Enterprise.where(id: e1) } - permissions.order_cycle_enterprises.should == [e1] - end + it "returns managed and related enterprises with add_to_order_cycle permission" do + permissions. + should_receive(:managed_and_related_enterprises_with). + with(:add_to_order_cycle). + and_return([e]) - it "returns permitted enterprises" do - permissions.stub(:related_enterprises_with) { Enterprise.where(id: e2) } - permissions.order_cycle_enterprises.should == [e2] + permissions.order_cycle_enterprises.should == [e] end end @@ -75,6 +71,19 @@ module OpenFoodNetwork end end + describe "finding enterprises that we manage products for" do + let(:e) { double(:enterprise) } + + it "returns managed and related enterprises with manage_products permission" do + permissions. + should_receive(:managed_and_related_enterprises_with). + with(:manage_products). + and_return([e]) + + permissions.managed_product_enterprises.should == [e] + end + end + ######################################## describe "finding related enterprises with a particular permission" do @@ -91,6 +100,24 @@ module OpenFoodNetwork 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') } + end + + it "returns managed enterprises" do + permissions.should_receive(:managed_enterprises) { Enterprise.where(id: e1) } + permissions.send(:managed_and_related_enterprises_with, 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] + end + end + describe "finding the supplied products of related enterprises" do let!(:e) { create(:enterprise) } let!(:p) { create(:simple_product, supplier: e) } From c81503d95f3939e20fc335b6458e73cb5e6b98cb Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 10:44:09 +1000 Subject: [PATCH 43/46] Include producers I have permission to in BPE producers choice --- app/controllers/spree/admin/products_controller_decorator.rb | 2 +- spec/features/admin/bulk_product_update_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 9f4fdbe46b..7fc9984241 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -85,7 +85,7 @@ Spree::Admin::ProductsController.class_eval do def load_bpe_data current_user.generate_spree_api_key! unless spree_current_user.spree_api_key @spree_api_key = spree_current_user.spree_api_key - @producers = Enterprise.managed_by(spree_current_user).is_primary_producer.order(:name) + @producers = OpenFoodNetwork::Permissions.new(spree_current_user).managed_product_enterprises.is_primary_producer.by_name @taxons = Spree::Taxon.order(:name) end end diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index 90f83bb26d..c4dbba32fd 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -764,10 +764,10 @@ feature %q{ expect(page).to have_no_field 'product_name', with: product_not_supplied.name end - it "shows only suppliers that I manage" do + it "shows only suppliers that I manage or have permission to" do visit '/admin/products/bulk_edit' - expect(page).to have_select 'producer', with_options: [supplier_managed1.name, supplier_managed2.name], selected: supplier_managed1.name + expect(page).to have_select 'producer', with_options: [supplier_managed1.name, supplier_managed2.name, supplier_permitted.name], selected: supplier_managed1.name expect(page).to have_no_select 'producer', with_options: [supplier_unmanaged.name] end From 94683f1eaa153e1287665902f95ee92fbc5aca3d Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 11:15:00 +1000 Subject: [PATCH 44/46] Check authorisation for bulk update products --- .../admin/products_controller_decorator.rb | 3 +++ .../spree/admin/products_controller_spec.rb | 21 +++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 7fc9984241..a5b38126e5 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -30,6 +30,9 @@ Spree::Admin::ProductsController.class_eval do "#{string}q[#{filter[:property][:db_column]}_#{filter[:predicate][:predicate]}]=#{filter[:value]};" end + # Ensure we're authorised to update all products + product_set.collection.each { |p| authorize! :update, p } + if product_set.save redirect_to "/api/products/bulk_products?page=1;per_page=500;#{bulk_index_query}" else diff --git a/spec/controllers/spree/admin/products_controller_spec.rb b/spec/controllers/spree/admin/products_controller_spec.rb index f05be2bb69..9746ffa0e2 100644 --- a/spec/controllers/spree/admin/products_controller_spec.rb +++ b/spec/controllers/spree/admin/products_controller_spec.rb @@ -1,11 +1,28 @@ require 'spec_helper' describe Spree::Admin::ProductsController do - context "Creating a new product" do + describe "updating a product we do not have access to" do + let(:s_managed) { create(:enterprise) } + let(:s_unmanaged) { create(:enterprise) } + let(:p) { create(:simple_product, supplier: s_unmanaged, name: 'Peas') } + before do - login_as_admin + login_as_enterprise_user [s_managed] + spree_post :bulk_update, {"products" => [{"id" => p.id, "name" => "Pine nuts"}]} end + it "denies access" do + response.should redirect_to "http://test.host/unauthorized" + end + + it "does not update any product" do + p.reload.name.should_not == "Pine nuts" + end + end + + context "creating a new product" do + before { login_as_admin } + it "redirects to bulk_edit when the user hits 'create'" do s = create(:supplier_enterprise) t = create(:taxon) From e72c3d861b5b8a579163282bd151a88e8b91238c Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 11:49:09 +1000 Subject: [PATCH 45/46] Enterprise manager can edit products from enterprises it has manage_products permission on --- app/models/spree/ability_decorator.rb | 4 ++-- spec/features/admin/bulk_product_update_spec.rb | 14 +++++++------- spec/models/spree/ability_spec.rb | 17 ++++++++++++++--- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 68d98bd568..9f9c96cb15 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -43,12 +43,12 @@ class AbilityDecorator # Enterprise User can only access products that they are a supplier for can [:create], Spree::Product can [:admin, :read, :update, :product_distributions, :bulk_edit, :bulk_update, :clone, :destroy], Spree::Product do |product| - user.enterprises.include? product.supplier + OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.include? product.supplier end can [:create], Spree::Variant can [:admin, :index, :read, :edit, :update, :search, :destroy], Spree::Variant do |variant| - user.enterprises.include? variant.product.supplier + OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.include? variant.product.supplier end can [:admin, :index, :read, :create, :edit, :update_positions, :destroy], Spree::ProductProperty diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index c4dbba32fd..9f530f860c 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -780,19 +780,19 @@ feature %q{ end it "allows me to update a product" do - p = product_supplied + p = product_supplied_permitted visit '/admin/products/bulk_edit' first("div#columns_dropdown", :text => "COLUMNS").click first("div#columns_dropdown div.menu div.menu_item", text: "Available On").click - expect(page).to have_field "product_name", with: p.name - expect(page).to have_select "producer", selected: supplier_managed1.name - expect(page).to have_field "available_on", with: p.available_on.strftime("%F %T") - expect(page).to have_field "price", with: "10.0" - expect(page).to have_field "on_hand", with: "6" + within "tr#p_#{p.id}" do + expect(page).to have_field "product_name", with: p.name + expect(page).to have_select "producer", selected: supplier_permitted.name + expect(page).to have_field "available_on", with: p.available_on.strftime("%F %T") + expect(page).to have_field "price", with: "10.0" + expect(page).to have_field "on_hand", with: "6" - within("tr#p_#{product_supplied.id}") do fill_in "product_name", with: "Big Bag Of Potatoes" select(supplier_managed2.name, :from => 'producer') fill_in "available_on", with: (Date.today-3).strftime("%F %T") diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 9e5ae49780..cf17d7fe3a 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -51,14 +51,17 @@ module Spree # create enterprises let(:s1) { create(:supplier_enterprise) } let(:s2) { create(:supplier_enterprise) } + let(:s_related) { create(:supplier_enterprise) } let(:d1) { create(:distributor_enterprise) } let(:d2) { create(:distributor_enterprise) } let(:p1) { create(:product, supplier: s1, distributors:[d1, d2]) } let(:p2) { create(:product, supplier: s2, distributors:[d1, d2]) } + let(:p_related) { create(:product, supplier: s_related) } let(:er1) { create(:enterprise_relationship, parent: s1, child: d1) } let(:er2) { create(:enterprise_relationship, parent: d1, child: s1) } + let(:er_p) { create(:enterprise_relationship, parent: s_related, child: s1, permissions_list: [:manage_products]) } subject { user } let(:user) { nil } @@ -74,12 +77,20 @@ module Spree let(:order) {create(:order)} - it "should be able to read/write their enterprises' products" do + it "should be able to read/write their enterprises' products and variants" do should have_ability([:admin, :read, :update, :product_distributions, :bulk_edit, :bulk_update, :clone, :destroy], for: p1) + should have_ability([:admin, :index, :read, :edit, :update, :search, :destroy], for: p1.master) end - it "should not be able to read/write other enterprises' products" do + it "should be able to read/write related enterprises' products and variants with manage_products permission" do + er_p + should have_ability([:admin, :read, :update, :product_distributions, :bulk_edit, :bulk_update, :clone, :destroy], for: p_related) + should have_ability([:admin, :index, :read, :edit, :update, :search, :destroy], for: p_related.master) + end + + it "should not be able to read/write other enterprises' products and variants" do should_not have_ability([:admin, :read, :update, :product_distributions, :bulk_edit, :bulk_update, :clone, :destroy], for: p2) + should_not have_ability([:admin, :index, :read, :edit, :update, :search, :destroy], for: p2.master) end it "should not be able to access admin actions on orders" do @@ -247,7 +258,7 @@ module Spree end end - context 'Enterprise manager' do + context 'enterprise manager' do let (:user) do user = create(:user) user.spree_roles = [] From 2cd5afbf9ccbc68fa86f6635110d197c4e242e23 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Mon, 1 Sep 2014 16:21:23 +1000 Subject: [PATCH 46/46] Set product.largeImage in JS, use for product modal --- .../darkswarm/services/products.js.coffee | 1 + .../javascripts/templates/product_modal.html.haml | 2 +- .../unit/darkswarm/services/product_spec.js.coffee | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/darkswarm/services/products.js.coffee b/app/assets/javascripts/darkswarm/services/products.js.coffee index 5819c7b661..9da3187f82 100644 --- a/app/assets/javascripts/darkswarm/services/products.js.coffee +++ b/app/assets/javascripts/darkswarm/services/products.js.coffee @@ -48,3 +48,4 @@ Darkswarm.factory 'Products', ($resource, Enterprises, Dereferencer, Taxons, Car product.primaryImage = product.images[0]?.small_url if product.images product.primaryImageOrMissing = product.primaryImage || "/assets/noimage/small.png" + product.largeImage = product.images[0]?.large_url if product.images diff --git a/app/assets/javascripts/templates/product_modal.html.haml b/app/assets/javascripts/templates/product_modal.html.haml index 31f51c6655..82cda0a371 100644 --- a/app/assets/javascripts/templates/product_modal.html.haml +++ b/app/assets/javascripts/templates/product_modal.html.haml @@ -1,6 +1,6 @@ .row .columns.small-12.large-6 - %img.product-img{"ng-src" => "{{product.primaryImage}}", "ng-if" => "product.primaryImage"} + %img.product-img{"ng-src" => "{{product.largeImage}}", "ng-if" => "product.largeImage"} .columns.small-12.large-6.product-header %h2 / %render-svg{path: "{{product.primary_taxon.icon}}"} diff --git a/spec/javascripts/unit/darkswarm/services/product_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/product_spec.js.coffee index d7c5a20b91..27e3f5aff6 100644 --- a/spec/javascripts/unit/darkswarm/services/product_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/product_spec.js.coffee @@ -7,6 +7,7 @@ describe 'Products service', -> CurrentHubMock = {} currentOrder = null product = null + productWithImage = null beforeEach -> product = @@ -16,6 +17,14 @@ describe 'Products service', -> price: 11 master: {} variants: [] + productWithImage = + supplier: + id: 9 + master: {} + variants: [] + images: [ + large_url: 'foo.png' + ] currentOrder = line_items: [] @@ -62,6 +71,11 @@ describe 'Products service', -> expect(Products.products[0].primaryImage).toBeUndefined() expect(Products.products[0].primaryImageOrMissing).toEqual "/assets/noimage/small.png" + it "sets largeImage", -> + $httpBackend.expectGET("/shop/products").respond([productWithImage]) + $httpBackend.flush() + expect(Products.products[0].largeImage).toEqual("foo.png") + describe "determining the price to display for a product", -> it "displays the product price when the product does not have variants", -> $httpBackend.expectGET("/shop/products").respond([product])