From 58105030682d6dc4f213f2ffb07cf0bd708291b6 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Fri, 26 Jul 2013 11:47:25 +1000 Subject: [PATCH 01/11] Move test data seeding into separate rake task. Add new role to seeds file. --- db/seeds.rb | 83 ++++------------------------------------------ lib/tasks/dev.rake | 76 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 76 deletions(-) create mode 100644 lib/tasks/dev.rake diff --git a/db/seeds.rb b/db/seeds.rb index 85fc9e4d86..ea43787661 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,10 +1,6 @@ # This file should contain all the record creation needed to seed the database with its default values. # The data can then be loaded with the rake db:seed (or created alongside the db with db:setup). -require File.expand_path('../../spec/factories', __FILE__) -require File.expand_path('../../spec/support/spree/init', __FILE__) - - # -- Spree unless Spree::Country.find_by_name 'Australia' puts "[db:seed] Seeding Spree" @@ -12,12 +8,9 @@ unless Spree::Country.find_by_name 'Australia' Spree::Auth::Engine.load_seed if defined?(Spree::Auth) end - # -- States -# States are created from factories instead of fixtures because models loaded from fixtures -# are not available to the app in the remainder of the seeding process (probably because of -# activerecord caching). unless Spree::State.find_by_name 'Victoria' + country = Spree::Country.find_by_name('Australia') puts "[db:seed] Seeding states" [ @@ -30,74 +23,12 @@ unless Spree::State.find_by_name 'Victoria' ['Victoria', 'Vic'], ['Western Australia', 'WA'] ].each do |state| - - # country_id 12 == Australia. See db/default/spree/countries.yaml - FactoryGirl.create(:state, :name => state[0], :abbr => state[1], :country_id => 12) + Spree::State.create!({"name"=>state[0], "abbr"=>state[1], :country=>country}, :without_protection => true) end end - -# -- Shipping / payment information -unless Spree::Zone.find_by_name 'Australia' - puts "[db:seed] Seeding shipping / payment information" - zone = FactoryGirl.create(:zone, :name => 'Australia', :zone_members => []) - country = Spree::Country.find_by_name('Australia') - Spree::ZoneMember.create(:zone => zone, :zoneable => country) - FactoryGirl.create(:shipping_method, :zone => zone) - FactoryGirl.create(:payment_method, :environment => 'development') -end - - -# -- Taxonomies -unless Spree::Taxonomy.find_by_name 'Products' - puts "[db:seed] Seeding taxonomies" - taxonomy = Spree::Taxonomy.find_by_name('Products') || FactoryGirl.create(:taxonomy, :name => 'Products') - taxonomy_root = taxonomy.root - - ['Vegetables', 'Fruit', 'Oils', 'Preserves and Sauces', 'Dairy', 'Meat and Fish'].each do |taxon_name| - FactoryGirl.create(:taxon, :name => taxon_name, :parent_id => taxonomy_root.id) - end -end - - -# -- Enterprises -unless Enterprise.count > 0 - puts "[db:seed] Seeding enterprises" - - 3.times { FactoryGirl.create(:supplier_enterprise) } - 3.times { FactoryGirl.create(:distributor_enterprise) } -end - - -# -- Products -unless Spree::Product.count > 0 - puts "[db:seed] Seeding products" - - prod1 = FactoryGirl.create(:product, - :name => 'Garlic', :price => 20.00, - :supplier => Enterprise.is_primary_producer[0], - :taxons => [Spree::Taxon.find_by_name('Vegetables')]) - - ProductDistribution.create(:product => prod1, - :distributor => Enterprise.is_distributor[0], - :shipping_method => Spree::ShippingMethod.first) - - - prod2 = FactoryGirl.create(:product, - :name => 'Fuji Apple', :price => 5.00, - :supplier => Enterprise.is_primary_producer[1], - :taxons => [Spree::Taxon.find_by_name('Fruit')]) - - ProductDistribution.create(:product => prod2, - :distributor => Enterprise.is_distributor[1], - :shipping_method => Spree::ShippingMethod.first) - - prod3 = FactoryGirl.create(:product, - :name => 'Beef - 5kg Trays', :price => 50.00, - :supplier => Enterprise.is_primary_producer[2], - :taxons => [Spree::Taxon.find_by_name('Meat and Fish')]) - - ProductDistribution.create(:product => prod3, - :distributor => Enterprise.is_distributor[2], - :shipping_method => Spree::ShippingMethod.first) -end +# -- Roles +unless Spree::Role.find_by_name 'enterprise' + puts "seeding roles" + Spree::Role.create!(:name => "enterprise") +end \ No newline at end of file diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake new file mode 100644 index 0000000000..abbf1214de --- /dev/null +++ b/lib/tasks/dev.rake @@ -0,0 +1,76 @@ + +namespace :openfoodweb do + + namespace :dev do + + desc 'load sample data' + task :load_sample_data => :environment do + require File.expand_path('../../../spec/factories', __FILE__) + + # -- Shipping / payment information + unless Spree::Zone.find_by_name 'Australia' + puts "[db:seed] Seeding shipping / payment information" + zone = FactoryGirl.create(:zone, :name => 'Australia', :zone_members => []) + country = Spree::Country.find_by_name('Australia') + Spree::ZoneMember.create(:zone => zone, :zoneable => country) + FactoryGirl.create(:shipping_method, :zone => zone) + FactoryGirl.create(:payment_method, :environment => 'development') + end + + + # -- Taxonomies + unless Spree::Taxonomy.find_by_name 'Products' + puts "[db:seed] Seeding taxonomies" + taxonomy = Spree::Taxonomy.find_by_name('Products') || FactoryGirl.create(:taxonomy, :name => 'Products') + taxonomy_root = taxonomy.root + + ['Vegetables', 'Fruit', 'Oils', 'Preserves and Sauces', 'Dairy', 'Meat and Fish'].each do |taxon_name| + FactoryGirl.create(:taxon, :name => taxon_name, :parent_id => taxonomy_root.id) + end + end + + + # -- Enterprises + unless Enterprise.count > 0 + puts "[db:seed] Seeding enterprises" + + 3.times { FactoryGirl.create(:supplier_enterprise) } + 3.times { FactoryGirl.create(:distributor_enterprise) } + end + + + # -- Products + unless Spree::Product.count > 0 + puts "[db:seed] Seeding products" + + prod1 = FactoryGirl.create(:product, + :name => 'Garlic', :price => 20.00, + :supplier => Enterprise.is_primary_producer[0], + :taxons => [Spree::Taxon.find_by_name('Vegetables')]) + + ProductDistribution.create(:product => prod1, + :distributor => Enterprise.is_distributor[0], + :shipping_method => Spree::ShippingMethod.first) + + + prod2 = FactoryGirl.create(:product, + :name => 'Fuji Apple', :price => 5.00, + :supplier => Enterprise.is_primary_producer[1], + :taxons => [Spree::Taxon.find_by_name('Fruit')]) + + ProductDistribution.create(:product => prod2, + :distributor => Enterprise.is_distributor[1], + :shipping_method => Spree::ShippingMethod.first) + + prod3 = FactoryGirl.create(:product, + :name => 'Beef - 5kg Trays', :price => 50.00, + :supplier => Enterprise.is_primary_producer[2], + :taxons => [Spree::Taxon.find_by_name('Meat and Fish')]) + + ProductDistribution.create(:product => prod3, + :distributor => Enterprise.is_distributor[2], + :shipping_method => Spree::ShippingMethod.first) + end + end + end +end From 4f5679aac38327eff7af0741f2265c1e0122ce20 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Wed, 31 Jul 2013 16:00:28 +1000 Subject: [PATCH 02/11] Add EnterpriseRole model and link up to users and enterprises. --- app/models/enterprise.rb | 2 ++ app/models/enterprise_role.rb | 4 ++++ app/models/spree/user_decorator.rb | 4 ++++ .../20130729030515_add_enterprise_role.rb | 8 ++++++++ db/schema.rb | 18 ++++++++++-------- lib/tasks/dev.rake | 1 + spec/support/spree/init.rb | 2 -- 7 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 app/models/enterprise_role.rb create mode 100644 app/models/spree/user_decorator.rb create mode 100644 db/migrate/20130729030515_add_enterprise_role.rb diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 868ec7be52..16a35609f8 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -4,6 +4,8 @@ class Enterprise < ActiveRecord::Base belongs_to :address, :class_name => 'Spree::Address' has_many :product_distributions, :foreign_key => 'distributor_id', :dependent => :destroy has_many :distributed_products, :through => :product_distributions, :source => :product + has_many :enterprise_roles + has_many :users, through: :enterprise_roles accepts_nested_attributes_for :address diff --git a/app/models/enterprise_role.rb b/app/models/enterprise_role.rb new file mode 100644 index 0000000000..94b926f84f --- /dev/null +++ b/app/models/enterprise_role.rb @@ -0,0 +1,4 @@ +class EnterpriseRole < ActiveRecord::Base + belongs_to :user, :class_name => Spree.user_class + belongs_to :enterprise +end diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb new file mode 100644 index 0000000000..58870b787b --- /dev/null +++ b/app/models/spree/user_decorator.rb @@ -0,0 +1,4 @@ +Spree.user_class.class_eval do + has_many :enterprise_roles, :dependent => :destroy + has_many :enterprises, through: :enterprise_roles +end diff --git a/db/migrate/20130729030515_add_enterprise_role.rb b/db/migrate/20130729030515_add_enterprise_role.rb new file mode 100644 index 0000000000..25a1deeeac --- /dev/null +++ b/db/migrate/20130729030515_add_enterprise_role.rb @@ -0,0 +1,8 @@ +class AddEnterpriseRole < ActiveRecord::Migration + def change + create_table :enterprise_roles do |t| + t.references :user, index: true + t.references :enterprise, index: true + end + end +end diff --git a/db/schema.rb b/db/schema.rb index d7c132b227..eb4022ce3f 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 => 20130729021924) do +ActiveRecord::Schema.define(:version => 20130729030515) do create_table "cms_blocks", :force => true do |t| t.integer "page_id", :null => false @@ -130,11 +130,6 @@ ActiveRecord::Schema.define(:version => 20130729021924) do add_index "cms_snippets", ["site_id", "identifier"], :name => "index_cms_snippets_on_site_id_and_identifier", :unique => true add_index "cms_snippets", ["site_id", "position"], :name => "index_cms_snippets_on_site_id_and_position" - create_table "coordinator_fees", :id => false, :force => true do |t| - t.integer "order_cycle_id" - t.integer "enterprise_fee_id" - end - create_table "enterprise_fees", :force => true do |t| t.integer "enterprise_id" t.string "fee_type" @@ -143,6 +138,11 @@ ActiveRecord::Schema.define(:version => 20130729021924) do t.datetime "updated_at", :null => false end + create_table "enterprise_roles", :force => true do |t| + t.integer "user_id" + t.integer "enterprise_id" + end + create_table "enterprises", :force => true do |t| t.string "name" t.string "description" @@ -194,8 +194,10 @@ ActiveRecord::Schema.define(:version => 20130729021924) do t.datetime "orders_open_at" t.datetime "orders_close_at" t.integer "coordinator_id" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.integer "coordinator_admin_fee_id" + t.integer "coordinator_sales_fee_id" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "product_distributions", :force => true do |t| diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index abbf1214de..576fe8fca9 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -6,6 +6,7 @@ namespace :openfoodweb do desc 'load sample data' task :load_sample_data => :environment do require File.expand_path('../../../spec/factories', __FILE__) + require File.expand_path('../../../spec/support/spree/init', __FILE__) # -- Shipping / payment information unless Spree::Zone.find_by_name 'Australia' diff --git a/spec/support/spree/init.rb b/spec/support/spree/init.rb index 99a4dafdc4..471b7cf2c9 100644 --- a/spec/support/spree/init.rb +++ b/spec/support/spree/init.rb @@ -1,5 +1,3 @@ -require 'factory_girl_rails' - # Initialise shipping method when created without one, like this: # create(:product, :distributors => [...]) # In this case, we don't care what the shipping method is, but we need one for validations to pass. From 65617e0e77fb4e37ac85f05a878303ee170c6a12 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Thu, 1 Aug 2013 10:16:20 +1000 Subject: [PATCH 03/11] Add enterprise roles for a user and wire up interface. --- app/models/spree/user_decorator.rb | 12 ++++ .../users_add_enterprise_management.rb | 6 ++ .../admin/users/_enterprises_form.html.haml | 14 ++++ spec/features/admin/enterprise_user_spec.rb | 65 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 app/overrides/users_add_enterprise_management.rb create mode 100644 app/views/spree/admin/users/_enterprises_form.html.haml create mode 100644 spec/features/admin/enterprise_user_spec.rb diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 58870b787b..bbd4c30177 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -1,4 +1,16 @@ Spree.user_class.class_eval do has_many :enterprise_roles, :dependent => :destroy has_many :enterprises, through: :enterprise_roles + + accepts_nested_attributes_for :enterprise_roles, :allow_destroy => true + + attr_accessible :enterprise_ids, :enterprise_roles_attributes + + def build_enterprise_roles + Enterprise.all.each do |enterprise| + unless self.enterprise_roles.find_by_enterprise_id enterprise.id + self.enterprise_roles.build(:enterprise => enterprise) + end + end + end end diff --git a/app/overrides/users_add_enterprise_management.rb b/app/overrides/users_add_enterprise_management.rb new file mode 100644 index 0000000000..4ca1dd9e49 --- /dev/null +++ b/app/overrides/users_add_enterprise_management.rb @@ -0,0 +1,6 @@ +Deface::Override.new(:virtual_path => "spree/admin/users/_form", + :insert_after => "[data-hook='admin_user_form_fields']", + :partial => "spree/admin/users/enterprises_form", + :name => "add_enterprises_to_user" + ) + diff --git a/app/views/spree/admin/users/_enterprises_form.html.haml b/app/views/spree/admin/users/_enterprises_form.html.haml new file mode 100644 index 0000000000..411f7657d0 --- /dev/null +++ b/app/views/spree/admin/users/_enterprises_form.html.haml @@ -0,0 +1,14 @@ +%fieldset + %legend 'Manage Enterprises' + = f.field_container :enterprise_roles do + - f.object.build_enterprise_roles + %table + = f.fields_for :enterprise_roles do |enterprise_form| + %tr + %td + = hidden_field_tag "#{enterprise_form.object_name}[_destroy]", 1, :id => nil + = check_box_tag "#{enterprise_form.object_name}[_destroy]", 0, !enterprise_form.object.new_record? + %td + = label_tag "#{enterprise_form.object_name}[_destroy]", enterprise_form.object.enterprise.name + = enterprise_form.hidden_field :enterprise_id + diff --git a/spec/features/admin/enterprise_user_spec.rb b/spec/features/admin/enterprise_user_spec.rb new file mode 100644 index 0000000000..281e32e47c --- /dev/null +++ b/spec/features/admin/enterprise_user_spec.rb @@ -0,0 +1,65 @@ +require "spec_helper" + +feature %q{ + As a Super User + I want to setup users to manage an enterprise +} do + include AuthenticationWorkflow + include WebHelper + + background do + @new_user = create(:user, :email => 'enterprise@hub.com') + @enterprise1 = create(:enterprise, name: 'Enterprise 1') + @enterprise2 = create(:enterprise, name: 'Enterprise 2') + @enterprise3 = create(:enterprise, name: 'Enterprise 3') + @enterprise4 = create(:enterprise, name: 'Enterprise 4') + end + + context "creating an Enterprise User" do + context 'with no enterprises' do + scenario "assigning a user to an Enterprise" do + login_to_admin_section + + click_link 'Users' + click_link @new_user.email + click_link 'Edit' + + check @enterprise2.name + + click_button 'Update' + + @new_user.enterprises.count.should == 1 + @new_user.enterprises.first.name.should == @enterprise2.name + end + + end + + context 'with existing enterprises' do + + before(:each) do + @new_user.enterprise_roles.build(enterprise: @enterprise1).save + @new_user.enterprise_roles.build(enterprise: @enterprise3).save + end + + scenario "removing and add enterprises for a user" do + login_to_admin_section + + click_link 'Users' + click_link @new_user.email + click_link 'Edit' + + uncheck @enterprise3.name # remove + check @enterprise4.name # add + + click_button 'Update' + + @new_user.enterprises.count.should == 2 + @new_user.enterprises.should include(@enterprise1) + @new_user.enterprises.should include(@enterprise4) + end + + end + + + end +end From 287bd57a4ee844bad81f5abf96a7396e9719d71e Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Thu, 1 Aug 2013 10:31:17 +1000 Subject: [PATCH 04/11] Fix loading of initial data to ensure there is an itemwise shipping method. --- README.markdown | 4 ++++ lib/tasks/dev.rake | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/README.markdown b/README.markdown index 83b9bc189b..dc7c009fe3 100644 --- a/README.markdown +++ b/README.markdown @@ -44,6 +44,10 @@ Create the development and test databases, using the settings specified in `conf rake db:schema:load db:seed +Load some default data for your environment + + rake openfoodweb:dev:load_sample_data + At long last, your dreams of spinning up a development server can be realised: rails server diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index 576fe8fca9..bb53266596 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -30,7 +30,6 @@ namespace :openfoodweb do end end - # -- Enterprises unless Enterprise.count > 0 puts "[db:seed] Seeding enterprises" @@ -39,6 +38,9 @@ namespace :openfoodweb do 3.times { FactoryGirl.create(:distributor_enterprise) } end + unless Spree::ShippingMethod.all.any? { |sm| sm.calculator.is_a? OpenFoodWeb::Calculator::Itemwise } + FactoryGirl.create(:itemwise_shipping_method) + end # -- Products unless Spree::Product.count > 0 From 46474ea4ccfd40eeb7239d5b1f28d41d4cd00278 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Thu, 1 Aug 2013 15:01:47 +1000 Subject: [PATCH 05/11] Prevent order cycle validations on orders when feature toggled off. --- app/models/spree/order_decorator.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/spree/order_decorator.rb b/app/models/spree/order_decorator.rb index c22f5f938a..b1dba6cec3 100644 --- a/app/models/spree/order_decorator.rb +++ b/app/models/spree/order_decorator.rb @@ -15,7 +15,11 @@ Spree::Order.class_eval do def products_available_from_new_distribution # Check that the line_items in the current order are available from a newly selected distribution - errors.add(:base, "Distributor or order cycle cannot supply the products in your cart") unless DistributionChangeValidator.new(self).can_change_to_distribution?(distributor, order_cycle) + if OpenFoodWeb::FeatureToggle.enabled? :order_cycles + errors.add(:base, "Distributor or order cycle cannot supply the products in your cart") unless DistributionChangeValidator.new(self).can_change_to_distribution?(distributor, order_cycle) + else + errors.add(:distributor_id, "cannot supply the products in your cart") unless DistributionChangeValidator.new(self).can_change_to_distributor?(distributor) + end end def set_order_cycle!(order_cycle) From 5bccd38b5e5459e846f37ed05034cd5ba0e1ca37 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Thu, 1 Aug 2013 15:37:42 +1000 Subject: [PATCH 06/11] Fix tests - don't have feature toggle overrides on when running tests! --- spec/models/order_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb index 8e0e6c54f7..f9dca32470 100644 --- a/spec/models/order_spec.rb +++ b/spec/models/order_spec.rb @@ -137,7 +137,7 @@ describe Spree::Order do subject.distributor = test_enterprise subject.should_not be_valid - subject.errors.messages.should == {base: ["Distributor or order cycle cannot supply the products in your cart"]} + subject.errors.messages.should == {distributor_id: ["cannot supply the products in your cart"]} end end end From 0bb4f676e1d4370702955fb945f8deafcc846ec1 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Fri, 2 Aug 2013 11:41:16 +1000 Subject: [PATCH 07/11] Add permissions to enterprise users so they can manage their own products. --- .../admin/products_controller_decorator.rb | 20 ++++-- app/models/spree/ability_decorator.rb | 23 +++++++ .../admin/products/_supplier_form.html.haml | 11 +-- .../admin/users/_enterprises_form.html.haml | 1 - db/schema.rb | 13 ++-- db/seeds.rb | 6 -- spec/features/admin/enterprise_user_spec.rb | 69 ++++++++++++++----- .../request/authentication_workflow.rb | 15 +++- 8 files changed, 118 insertions(+), 40 deletions(-) create mode 100644 app/models/spree/ability_decorator.rb diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index bbbf8a3a5b..71ec1462ea 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -5,8 +5,10 @@ Spree::Admin::ProductsController.class_eval do respond_to :json, :only => :clone + before_filter :filter_out_products_for_enterprise_users, :only => :index + #respond_override :clone => { :json => {:success => lambda { redirect_to bulk_index_admin_products_url+"?q[id_eq]=#{@new.id}" } } } - + def bulk_update collection_hash = Hash[params[:_json].each_with_index.map { |p,i| [i,p] }] product_set = Spree::ProductSet.new({:collection_attributes => collection_hash}) @@ -17,16 +19,24 @@ Spree::Admin::ProductsController.class_eval do render :nothing => true end end - + protected def location_after_save - if URI(request.referer).path == '/admin/products/bulk_edit' + if URI(request.referer).path == '/admin/products/bulk_edit' bulk_edit_admin_products_url - else + else location_after_save_original end end - + + def filter_out_products_for_enterprise_users + unless spree_current_user.has_spree_role?('admin') + @collection.select! do |product| + product.supplier.users.include? spree_current_user + end + end + end + private def load_spree_api_key diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb new file mode 100644 index 0000000000..3d10e81923 --- /dev/null +++ b/app/models/spree/ability_decorator.rb @@ -0,0 +1,23 @@ + +class AbilityDecorator + include CanCan::Ability + def initialize(user) + if user.enterprises.count > 0 + can [:admin, :read, :update, :bulk_edit], Spree::Product do |product| + user.enterprises.include? product.supplier + end + + can [:create], Spree::Product + can [:admin, :index, :read, :create, :edit], Spree::Variant + can [:admin, :index, :read, :create, :edit], Spree::ProductProperty + can [:admin, :index, :read, :create, :edit], Spree::Image + + can [:admin, :index, :read, :search], Spree::Taxon + can [:admin, :index, :read, :create, :edit], Spree::Classification + + can [:admin, :index, :read], Spree::Order + end + end +end + +Spree::Ability.register_ability(AbilityDecorator) diff --git a/app/views/spree/admin/products/_supplier_form.html.haml b/app/views/spree/admin/products/_supplier_form.html.haml index 0de2720a65..0cecc1a57b 100644 --- a/app/views/spree/admin/products/_supplier_form.html.haml +++ b/app/views/spree/admin/products/_supplier_form.html.haml @@ -1,5 +1,6 @@ -= f.field_container :supplier do - = f.label :supplier - %br - = f.collection_select(:supplier_id, Enterprise.is_primary_producer, :id, :name, {:include_blank => true}, {:class => "select2"}) - = f.error_message_on :supplier +- if spree_current_user.has_spree_role?('admin') + = f.field_container :supplier do + = f.label :supplier + %br + = f.collection_select(:supplier_id, Enterprise.is_primary_producer, :id, :name, {:include_blank => true}, {:class => "select2"}) + = f.error_message_on :supplier diff --git a/app/views/spree/admin/users/_enterprises_form.html.haml b/app/views/spree/admin/users/_enterprises_form.html.haml index 411f7657d0..03cecd41d5 100644 --- a/app/views/spree/admin/users/_enterprises_form.html.haml +++ b/app/views/spree/admin/users/_enterprises_form.html.haml @@ -11,4 +11,3 @@ %td = label_tag "#{enterprise_form.object_name}[_destroy]", enterprise_form.object.enterprise.name = enterprise_form.hidden_field :enterprise_id - diff --git a/db/schema.rb b/db/schema.rb index eb4022ce3f..cbcb36496e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -130,6 +130,11 @@ ActiveRecord::Schema.define(:version => 20130729030515) do add_index "cms_snippets", ["site_id", "identifier"], :name => "index_cms_snippets_on_site_id_and_identifier", :unique => true add_index "cms_snippets", ["site_id", "position"], :name => "index_cms_snippets_on_site_id_and_position" + create_table "coordinator_fees", :id => false, :force => true do |t| + t.integer "order_cycle_id" + t.integer "enterprise_fee_id" + end + create_table "enterprise_fees", :force => true do |t| t.integer "enterprise_id" t.string "fee_type" @@ -194,10 +199,8 @@ ActiveRecord::Schema.define(:version => 20130729030515) do t.datetime "orders_open_at" t.datetime "orders_close_at" t.integer "coordinator_id" - t.integer "coordinator_admin_fee_id" - t.integer "coordinator_sales_fee_id" - t.datetime "created_at", :null => false - t.datetime "updated_at", :null => false + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "product_distributions", :force => true do |t| @@ -428,9 +431,9 @@ ActiveRecord::Schema.define(:version => 20130729030515) 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" end add_index "spree_orders", ["number"], :name => "index_orders_on_number" diff --git a/db/seeds.rb b/db/seeds.rb index ea43787661..82f95651bd 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -26,9 +26,3 @@ unless Spree::State.find_by_name 'Victoria' Spree::State.create!({"name"=>state[0], "abbr"=>state[1], :country=>country}, :without_protection => true) end end - -# -- Roles -unless Spree::Role.find_by_name 'enterprise' - puts "seeding roles" - Spree::Role.create!(:name => "enterprise") -end \ No newline at end of file diff --git a/spec/features/admin/enterprise_user_spec.rb b/spec/features/admin/enterprise_user_spec.rb index 281e32e47c..e26be25c1e 100644 --- a/spec/features/admin/enterprise_user_spec.rb +++ b/spec/features/admin/enterprise_user_spec.rb @@ -7,29 +7,27 @@ feature %q{ include AuthenticationWorkflow include WebHelper - background do - @new_user = create(:user, :email => 'enterprise@hub.com') - @enterprise1 = create(:enterprise, name: 'Enterprise 1') - @enterprise2 = create(:enterprise, name: 'Enterprise 2') - @enterprise3 = create(:enterprise, name: 'Enterprise 3') - @enterprise4 = create(:enterprise, name: 'Enterprise 4') + before(:each) do + @new_user = create_enterprise_user + @supplier1 = create(:supplier_enterprise, name: 'Supplier 1') + @supplier2 = create(:supplier_enterprise, name: 'Supplier 2') + @distributor1 = create(:distributor_enterprise, name: 'Distributor 3') + @distributor2 = create(:distributor_enterprise, name: 'Distributor 4') end context "creating an Enterprise User" do context 'with no enterprises' do scenario "assigning a user to an Enterprise" do login_to_admin_section - click_link 'Users' click_link @new_user.email click_link 'Edit' - check @enterprise2.name + check @supplier2.name click_button 'Update' - @new_user.enterprises.count.should == 1 - @new_user.enterprises.first.name.should == @enterprise2.name + @new_user.enterprises.first.name.should == @supplier2.name end end @@ -37,8 +35,8 @@ feature %q{ context 'with existing enterprises' do before(:each) do - @new_user.enterprise_roles.build(enterprise: @enterprise1).save - @new_user.enterprise_roles.build(enterprise: @enterprise3).save + @new_user.enterprise_roles.build(enterprise: @supplier1).save + @new_user.enterprise_roles.build(enterprise: @distributor1).save end scenario "removing and add enterprises for a user" do @@ -48,18 +46,57 @@ feature %q{ click_link @new_user.email click_link 'Edit' - uncheck @enterprise3.name # remove - check @enterprise4.name # add + uncheck @distributor1.name # remove + check @distributor2.name # add click_button 'Update' @new_user.enterprises.count.should == 2 - @new_user.enterprises.should include(@enterprise1) - @new_user.enterprises.should include(@enterprise4) + @new_user.enterprises.should include(@supplier1) + @new_user.enterprises.should include(@distributor2) end end + end + + context "Product management" do + + context 'products I supply' do + before(:each) do + @new_user.enterprise_roles.build(enterprise: @supplier1).save + product1 = create(:product, name: 'Green eggs', supplier: @supplier1) + product2 = create(:product, name: 'Ham', supplier: @supplier2) + login_to_admin_as @new_user + end + + scenario "manage products that I supply" do + visit 'admin/products' + + within '#listing_products' do + page.should have_content 'Green eggs' + page.should_not have_content 'Ham' + end + end + end end + + context "System management lockdown" do + + before(:each) do + @new_user.enterprise_roles.build(enterprise: @supplier1).save + login_to_admin_as @new_user + end + + scenario "should not be able to see system configuration" do + visit 'admin/general_settings/edit' + page.should have_content 'Authorization Failure' + end + + scenario "should not be able to see user management" do + visit 'admin/users' + page.should have_content 'Authorization Failure' + end + end end diff --git a/spec/support/request/authentication_workflow.rb b/spec/support/request/authentication_workflow.rb index 63781e8b50..319890134c 100644 --- a/spec/support/request/authentication_workflow.rb +++ b/spec/support/request/authentication_workflow.rb @@ -11,9 +11,20 @@ module AuthenticationWorkflow admin_user.spree_roles << admin_role + login_to_admin_as admin_user + end + + def create_enterprise_user + new_user = create(:user, email: 'enterprise@hub.com', password: 'blahblah', :password_confirmation => 'blahblah', ) + new_user.spree_roles = [] # for some reason unbeknown to me, this new user gets admin permissions by default. + new_user.save + new_user + end + + def login_to_admin_as user visit spree.admin_path - fill_in 'spree_user_email', :with => 'admin@ofw.org' - fill_in 'spree_user_password', :with => 'passw0rd' + fill_in 'spree_user_email', :with => user.email + fill_in 'spree_user_password', :with => user.password click_button 'Login' end From 269294612d99744f6c1aec22def2978f822dfb1f Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Fri, 2 Aug 2013 14:01:45 +1000 Subject: [PATCH 08/11] Add product tests for an enterprise user. --- spec/features/admin/product_spec.rb | 57 +++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/spec/features/admin/product_spec.rb b/spec/features/admin/product_spec.rb index 9bcacd13d3..58f73ba9b4 100644 --- a/spec/features/admin/product_spec.rb +++ b/spec/features/admin/product_spec.rb @@ -59,5 +59,62 @@ feature %q{ product.group_buy_unit_size.should == 10.0 end + + describe 'As an enterprise user' do + + before(:each) do + @new_user = create_enterprise_user + @supplier1 = create(:supplier_enterprise, name: 'Another Supplier') + @new_user.enterprise_roles.build(enterprise: @supplier1).save + @new_user.enterprise_roles.build(enterprise: @distributors[0]).save + + login_to_admin_as @new_user + end + + scenario "create new product" do + click_link 'Products' + click_link 'New Product' + + fill_in 'product_name', :with => 'A new product !!!' + fill_in 'product_price', :with => '19.99' + + # check suppliers are only ones we have access to ??????????????????????? + page.should have_selector('#product_supplier_id') + select 'Another Supplier', :from => 'product_supplier_id' + + # check that distributors are only the ones we have access to ????????????????? + check @distributors[0].name + select 'My shipping method', :from => 'product_product_distributions_attributes_0_shipping_method_id' + + click_button 'Create' + + flash_message.should == 'Product "A new product !!!" has been successfully created!' + product = Spree::Product.find_by_name('A new product !!!') + product.supplier.should == @supplier1 + product.distributors.should == [@distributors[0]] + end + + describe 'with existing product' do + before(:each) do + @product = create(:product, supplier: @supplier1) + end + + scenario "can edit" do + click_link 'Products' + + click_link @product.name + page.should_not have_selector('#product_supplier_id') + + click_link "Images" + page.should_not have_content 'Authorization Failure' + + click_link "Variants" + page.should_not have_content 'Authorization Failure' + + click_link "Product Properties" + page.should_not have_content 'Authorization Failure' + end + end + end end end From 9d32a5775abf7c744a0dd62b3025bddaa14319f9 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Fri, 2 Aug 2013 16:08:10 +1000 Subject: [PATCH 09/11] Enterprise user admin: only allow suppliers and distributors that we manage to be selected when creating and editing products. --- .../admin/products_controller_decorator.rb | 2 +- app/models/enterprise.rb | 7 +++++ app/models/spree/product_decorator.rb | 4 +-- .../products/_distributors_form.html.haml | 2 +- .../_supplier_and_group_buy_for_new.html.haml | 2 +- .../admin/products/_supplier_form.html.haml | 2 +- spec/features/admin/product_spec.rb | 21 +++++++++---- spec/models/enterprises_spec.rb | 30 +++++++++++++++++-- 8 files changed, 55 insertions(+), 15 deletions(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 71ec1462ea..9c5fabbba1 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -32,7 +32,7 @@ Spree::Admin::ProductsController.class_eval do def filter_out_products_for_enterprise_users unless spree_current_user.has_spree_role?('admin') @collection.select! do |product| - product.supplier.users.include? spree_current_user + !product.supplier.nil? and product.supplier.users.include? spree_current_user end end end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 16a35609f8..cb6de01b35 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -50,6 +50,13 @@ class Enterprise < ActiveRecord::Base with_distributed_products_outer.with_order_cycles_and_exchange_variants_outer. where('product_distributions.product_id = ? OR spree_variants.product_id = ?', product, product). select('DISTINCT enterprises.*') + } + scope :managed_by, lambda { |user| + if user.has_spree_role?('admin') + all + else + joins(:enterprise_roles).where('enterprise_roles.user_id = ?', user.id) + end } diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index fb3d5c472f..cf015b0460 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -87,8 +87,8 @@ Spree::Product.class_eval do # Build a product distribution for each distributor - def build_product_distributions - Enterprise.is_distributor.each do |distributor| + def build_product_distributions_for_user user + Enterprise.is_distributor.managed_by(user).each do |distributor| unless self.product_distributions.find_by_distributor_id distributor.id self.product_distributions.build(:distributor => distributor) end diff --git a/app/views/spree/admin/products/_distributors_form.html.haml b/app/views/spree/admin/products/_distributors_form.html.haml index 3131383d35..e5c867c6e8 100644 --- a/app/views/spree/admin/products/_distributors_form.html.haml +++ b/app/views/spree/admin/products/_distributors_form.html.haml @@ -1,6 +1,6 @@ %h2 Distributors = f.field_container :product_distributions do - - f.object.build_product_distributions + - f.object.build_product_distributions_for_user spree_current_user %table = f.fields_for :product_distributions do |pd_form| %tr diff --git a/app/views/spree/admin/products/_supplier_and_group_buy_for_new.html.haml b/app/views/spree/admin/products/_supplier_and_group_buy_for_new.html.haml index 8cf8569fbc..c0615a753b 100644 --- a/app/views/spree/admin/products/_supplier_and_group_buy_for_new.html.haml +++ b/app/views/spree/admin/products/_supplier_and_group_buy_for_new.html.haml @@ -2,7 +2,7 @@ .alpha.six.columns = f.field_container :supplier do = f.label :supplier - = f.collection_select(:supplier_id, Enterprise.is_primary_producer, :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"}) + = f.collection_select(:supplier_id, Enterprise.is_primary_producer.managed_by(spree_current_user), :id, :name, {:include_blank => true}, {:class => "select2 fullwidth"}) = f.error_message_on :supplier .four.columns = f.field_container :group_buy do diff --git a/app/views/spree/admin/products/_supplier_form.html.haml b/app/views/spree/admin/products/_supplier_form.html.haml index 0cecc1a57b..cfc69a3a7f 100644 --- a/app/views/spree/admin/products/_supplier_form.html.haml +++ b/app/views/spree/admin/products/_supplier_form.html.haml @@ -2,5 +2,5 @@ = f.field_container :supplier do = f.label :supplier %br - = f.collection_select(:supplier_id, Enterprise.is_primary_producer, :id, :name, {:include_blank => true}, {:class => "select2"}) + = f.collection_select(:supplier_id, Enterprise.is_primary_producer.managed_by(spree_current_user), :id, :name, {:include_blank => true}, {:class => "select2"}) = f.error_message_on :supplier diff --git a/spec/features/admin/product_spec.rb b/spec/features/admin/product_spec.rb index 58f73ba9b4..d1bc4f6334 100644 --- a/spec/features/admin/product_spec.rb +++ b/spec/features/admin/product_spec.rb @@ -64,8 +64,8 @@ feature %q{ before(:each) do @new_user = create_enterprise_user - @supplier1 = create(:supplier_enterprise, name: 'Another Supplier') - @new_user.enterprise_roles.build(enterprise: @supplier1).save + @supplier2 = create(:supplier_enterprise, name: 'Another Supplier') + @new_user.enterprise_roles.build(enterprise: @supplier2).save @new_user.enterprise_roles.build(enterprise: @distributors[0]).save login_to_admin_as @new_user @@ -78,25 +78,34 @@ feature %q{ fill_in 'product_name', :with => 'A new product !!!' fill_in 'product_price', :with => '19.99' - # check suppliers are only ones we have access to ??????????????????????? page.should have_selector('#product_supplier_id') select 'Another Supplier', :from => 'product_supplier_id' - # check that distributors are only the ones we have access to ????????????????? + # Should only have suppliers listed which the user can manage + within "#product_supplier_id" do + page.should_not have_content @supplier.name + end + check @distributors[0].name select 'My shipping method', :from => 'product_product_distributions_attributes_0_shipping_method_id' + # Should only have distributors listed which the user can manage + within "#product_product_distributions_field" do + page.should_not have_content @distributors[1].name + page.should_not have_content @distributors[2].name + end + click_button 'Create' flash_message.should == 'Product "A new product !!!" has been successfully created!' product = Spree::Product.find_by_name('A new product !!!') - product.supplier.should == @supplier1 + product.supplier.should == @supplier2 product.distributors.should == [@distributors[0]] end describe 'with existing product' do before(:each) do - @product = create(:product, supplier: @supplier1) + @product = create(:product, supplier: @supplier2) end scenario "can edit" do diff --git a/spec/models/enterprises_spec.rb b/spec/models/enterprises_spec.rb index 1a49815ca4..f43e20983e 100644 --- a/spec/models/enterprises_spec.rb +++ b/spec/models/enterprises_spec.rb @@ -58,6 +58,30 @@ describe Enterprise do create(:simple_order_cycle, suppliers: [s], distributors: [d], variants: [p.master], orders_open_at: 1.week.from_now, orders_close_at: 2.weeks.from_now) Enterprise.active_distributors.should be_empty end + + it "shows only enterprises for given user" do + user = create(:user) + user.spree_roles = [] + e1 = create(:enterprise) + e2 = create(:enterprise) + e1.enterprise_roles.build(user: user).save + + enterprises = Enterprise.managed_by user + enterprises.count.should == 1 + enterprises.should include e1 + end + + it "shows all enterprises for admin user" do + user = create(:admin_user) + e1 = create(:enterprise) + e2 = create(:enterprise) + e1.enterprise_roles.build(user: user).save + + enterprises = Enterprise.managed_by user + enterprises.count.should == 2 + enterprises.should include e1 + enterprises.should include e2 + end end describe "with_distributed_active_products_on_hand" do @@ -102,9 +126,9 @@ describe Enterprise do create(:product, :supplier => d1, :on_hand => 5) create(:product, :supplier => d1, :on_hand => 5) create(:product, :supplier => d2, :on_hand => 5) - create(:product, :supplier => d3, :on_hand => 5, :available_on => 1.week.from_now) - create(:product, :supplier => d4, :on_hand => 0) - create(:product, :supplier => d5).delete + create(:product, :supplier => d3, :on_hand => 5, :available_on => 1.week.from_now) + create(:product, :supplier => d4, :on_hand => 0) + create(:product, :supplier => d5).delete Enterprise.with_supplied_active_products_on_hand.sort.should == [d1, d2] Enterprise.with_supplied_active_products_on_hand.distinct_count.should == 2 From e75021d7bd2b23320ba9883776bfec5d9fd5f9e1 Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Fri, 2 Aug 2013 18:13:42 +1000 Subject: [PATCH 10/11] Fix loading of products for an enterprise in the admin section. NOTE: this is a major hack - had to copy the current fetch data from the spree product_controller and modify it to get the roles to take affect. There must be a better way. --- .../admin/products_controller_decorator.rb | 29 ++++++++++++++----- app/models/enterprise.rb | 2 +- app/models/spree/product_decorator.rb | 7 +++++ spec/models/enterprises_spec.rb | 1 - spec/models/product_spec.rb | 28 ++++++++++++++++++ 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 9c5fabbba1..9af68d05fa 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -5,8 +5,6 @@ Spree::Admin::ProductsController.class_eval do respond_to :json, :only => :clone - before_filter :filter_out_products_for_enterprise_users, :only => :index - #respond_override :clone => { :json => {:success => lambda { redirect_to bulk_index_admin_products_url+"?q[id_eq]=#{@new.id}" } } } def bulk_update @@ -29,12 +27,29 @@ Spree::Admin::ProductsController.class_eval do end end - def filter_out_products_for_enterprise_users - unless spree_current_user.has_spree_role?('admin') - @collection.select! do |product| - !product.supplier.nil? and product.supplier.users.include? spree_current_user - end + def collection + # This method is copied directly from the spree product controller, except where we narrow the search below with the managed_by search to support + # enterprise users. + # TODO: There has to be a better way!!! + return @collection if @collection.present? + params[:q] ||= {} + params[:q][:deleted_at_null] ||= "1" + + params[:q][:s] ||= "name asc" + + @search = super.ransack(params[:q]) + @collection = @search.result. + managed_by(spree_current_user). # this line is added to the original spree code!!!!! + group_by_products_id. + includes(product_includes). + page(params[:page]). + per(Spree::Config[:admin_products_per_page]) + + if params[:q][:s].include?("master_default_price_amount") + # PostgreSQL compatibility + @collection = @collection.group("spree_prices.amount") end + @collection end private diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index cb6de01b35..bdcf9c9201 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -53,7 +53,7 @@ class Enterprise < ActiveRecord::Base } scope :managed_by, lambda { |user| if user.has_spree_role?('admin') - all + scoped else joins(:enterprise_roles).where('enterprise_roles.user_id = ?', user.id) end diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index cf015b0460..f0405baed6 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -57,6 +57,13 @@ Spree::Product.class_eval do scope :in_order_cycle, lambda { |order_cycle| with_order_cycles_inner. where('exchanges.sender_id = order_cycles.coordinator_id'). where('order_cycles.id = ?', order_cycle) } + scope :managed_by, lambda { |user| + if user.has_spree_role?('admin') + scoped + else + where('supplier_id IN (?)', user.enterprises.map {|enterprise| enterprise.id }) + end + } # -- Methods diff --git a/spec/models/enterprises_spec.rb b/spec/models/enterprises_spec.rb index f43e20983e..1a6789d622 100644 --- a/spec/models/enterprises_spec.rb +++ b/spec/models/enterprises_spec.rb @@ -75,7 +75,6 @@ describe Enterprise do user = create(:admin_user) e1 = create(:enterprise) e2 = create(:enterprise) - e1.enterprise_roles.build(user: user).save enterprises = Enterprise.managed_by user enterprises.count.should == 2 diff --git a/spec/models/product_spec.rb b/spec/models/product_spec.rb index 2a755e565a..8704dce6be 100644 --- a/spec/models/product_spec.rb +++ b/spec/models/product_spec.rb @@ -154,6 +154,34 @@ module Spree Product.in_order_cycle(oc1).should == [p1] end end + + describe 'access roles' do + before(:each) do + @e1 = create(:enterprise) + @e2 = create(:enterprise) + @p1 = create(:product, supplier: @e1) + @p2 = create(:product, supplier: @e2) + end + + it "shows only products for given user" do + user = create(:user) + user.spree_roles = [] + @e1.enterprise_roles.build(user: user).save + + product = Product.managed_by user + product.count.should == 1 + product.should include @p1 + end + + it "shows all products for admin user" do + user = create(:admin_user) + + product = Product.managed_by user + product.count.should == 2 + product.should include @p1 + product.should include @p2 + end + end end describe "finders" do From b0d959648902ea7e4d46076d981311da50ac2fef Mon Sep 17 00:00:00 2001 From: Andrew Spinks Date: Mon, 5 Aug 2013 10:00:47 +1000 Subject: [PATCH 11/11] Fix bug with available on translation - seemed to be getting overridden. --- config/locales/en.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 56eba0e134..36aca9c55c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2,10 +2,8 @@ # See https://github.com/svenfuchs/rails-i18n/tree/master/rails%2Flocale for starting points. en: - spree: - devise: failure: - invalid: | - Invalid email or password. + invalid: | + Invalid email or password. Were you a guest last time? Perhaps you need to create an account or reset your password. \ No newline at end of file