diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index a146fc4cc4..ccca96ab26 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -16,7 +16,7 @@ class Enterprise < ActiveRecord::Base has_many :enterprise_fees has_many :enterprise_roles, :dependent => :destroy has_many :users, through: :enterprise_roles - belongs_to :owner, class_name: 'Spree::User', foreign_key: :owner_id + belongs_to :owner, class_name: 'Spree::User', foreign_key: :owner_id, inverse_of: :owned_enterprises has_and_belongs_to_many :payment_methods, join_table: 'distributors_payment_methods', class_name: 'Spree::PaymentMethod', foreign_key: 'distributor_id' has_many :distributor_shipping_methods, foreign_key: :distributor_id has_many :shipping_methods, through: :distributor_shipping_methods @@ -47,7 +47,8 @@ class Enterprise < ActiveRecord::Base validates :name, presence: true validates :type, presence: true, inclusion: {in: TYPES} validates :address, presence: true, associated: true - validates :owner, presence: true + validates_presence_of :owner + validate :enforce_ownership_limit, if: lambda { owner_id_changed? } before_validation :ensure_owner_is_manager, if: lambda { owner_id_changed? } before_validation :set_unused_address_fields @@ -241,4 +242,10 @@ class Enterprise < ActiveRecord::Base def ensure_owner_is_manager users << owner unless users.include?(owner) || owner.admin? end + + def enforce_ownership_limit + unless owner.can_own_more_enterprises? + errors.add(:owner, "^You are not permitted to own own any more enterprises (limit is #{owner.enterprise_limit}).") + end + end end diff --git a/app/models/spree/user_decorator.rb b/app/models/spree/user_decorator.rb index 9890d0ff7b..2a177f84a0 100644 --- a/app/models/spree/user_decorator.rb +++ b/app/models/spree/user_decorator.rb @@ -1,7 +1,7 @@ Spree.user_class.class_eval do has_many :enterprise_roles, :dependent => :destroy has_many :enterprises, through: :enterprise_roles - has_many :owned_enterprises, class_name: 'Enterprise', foreign_key: :owner_id + has_many :owned_enterprises, class_name: 'Enterprise', foreign_key: :owner_id, inverse_of: :owner has_one :cart accepts_nested_attributes_for :enterprise_roles, :allow_destroy => true @@ -9,7 +9,7 @@ Spree.user_class.class_eval do attr_accessible :enterprise_ids, :enterprise_roles_attributes after_create :send_signup_confirmation - validate :owned_enterprises_count + validate :limit_owned_enterprises def build_enterprise_roles Enterprise.all.each do |enterprise| @@ -23,11 +23,15 @@ Spree.user_class.class_eval do Spree::UserMailer.signup_confirmation(self).deliver end + def can_own_more_enterprises? + owned_enterprises(:reload).size < enterprise_limit + end + private - def owned_enterprises_count + def limit_owned_enterprises if owned_enterprises.size > enterprise_limit - errors.add(:owned_enterprises, "^The nominated user is not permitted to own own any more enterprises.") + errors.add(:owned_enterprises, "^The nominated user is not permitted to own own any more enterprises (limit is #{enterprise_limit}).") end end end diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb index ee67dc1a73..249528149d 100644 --- a/spec/features/admin/enterprises_spec.rb +++ b/spec/features/admin/enterprises_spec.rb @@ -257,13 +257,13 @@ feature %q{ 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(:enterprise_user) { create_enterprise_user } before(:each) do - @new_user = create_enterprise_user - @new_user.enterprise_roles.build(enterprise: supplier1).save - @new_user.enterprise_roles.build(enterprise: distributor1).save + enterprise_user.enterprise_roles.build(enterprise: supplier1).save + enterprise_user.enterprise_roles.build(enterprise: distributor1).save - login_to_admin_as @new_user + login_to_admin_as enterprise_user end scenario "can view enterprises I have permission to" do @@ -290,23 +290,42 @@ feature %q{ expect(page).to_not have_content "distributor2.name" end - scenario "creating an enterprise" do - # When I create an enterprise - click_link 'Enterprises' - click_link 'New Enterprise' - fill_in 'enterprise_name', with: 'zzz' - fill_in 'enterprise_address_attributes_address1', with: 'z' - fill_in 'enterprise_address_attributes_city', with: 'z' - fill_in 'enterprise_address_attributes_zipcode', with: 'z' - click_button 'Create' + context "creating an enterprise" do + before do + # When I create an enterprise + click_link 'Enterprises' + click_link 'New Enterprise' + fill_in 'enterprise_name', with: 'zzz' + fill_in 'enterprise_address_attributes_address1', with: 'z' + fill_in 'enterprise_address_attributes_city', with: 'z' + fill_in 'enterprise_address_attributes_zipcode', with: 'z' + end - # Then it should be created - page.should have_content 'Enterprise "zzz" has been successfully created!' - enterprise = Enterprise.last - enterprise.name.should == 'zzz' + scenario "without violating rules" do + click_button 'Create' - # And I should be managing it - Enterprise.managed_by(@new_user).should include enterprise + # Then it should be created + page.should have_content 'Enterprise "zzz" has been successfully created!' + enterprise = Enterprise.last + enterprise.name.should == 'zzz' + + # And I should be managing it + Enterprise.managed_by(enterprise_user).should include enterprise + end + + context "overstepping my owned enterprises limit" do + before do + create(:enterprise, owner: enterprise_user) + end + + it "shows me an error message" do + click_button 'Create' + + # Then it should show me an error + expect(page).to_not have_content 'Enterprise "zzz" has been successfully created!' + expect(page).to have_content "You are not permitted to own own any more enterprises (limit is 1)." + end + end end scenario "editing enterprises I have permission to" do diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index c6418def09..f6b3c13d2d 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe Enterprise do + include AuthenticationWorkflow describe "associations" do it { should belong_to(:owner) } @@ -54,9 +55,9 @@ describe Enterprise do end describe "ownership" do - let(:u1) { create(:user) } - let(:u2) { create(:user) } - let(:e) { create(:enterprise, owner: u1 ) } + let(:u1) { create_enterprise_user } + let(:u2) { create_enterprise_user } + let!(:e) { create(:enterprise, owner: u1 ) } it "adds new owner to list of managers" do expect(e.owner).to eq u1 @@ -68,6 +69,16 @@ describe Enterprise do expect(e.owner).to eq u2 expect(e.users).to include u1, u2 end + + it "validates ownership limit" do + expect(u1.enterprise_limit).to be 1 + expect(u1.owned_enterprises(:reload)).to eq [e] + e2 = create(:enterprise, owner: u2 ) + expect{ + e2.owner = u1 + e2.save! + }.to raise_error ActiveRecord::RecordInvalid, "Validation failed: You are not permitted to own own any more enterprises (limit is 1)." + end end end diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index e985d9bde8..f049c64b2f 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -3,23 +3,23 @@ describe Spree.user_class do it { should have_many(:owned_enterprises) } describe "enterprise ownership" do - let(:u1) { create(:user) } - let(:u2) { create(:user) } - let(:e1) { create(:enterprise, owner: u1) } - let(:e2) { create(:enterprise, owner: u1) } + let(:u1) { create(:user, enterprise_limit: 2) } + let(:u2) { create(:user, enterprise_limit: 1) } + let!(:e1) { create(:enterprise, owner: u1) } + let!(:e2) { create(:enterprise, owner: u1) } - it "provides access to owned enteprises" do - expect(u1.owned_enterprises).to include e1, e2 + it "provides access to owned enterprises" do + expect(u1.owned_enterprises(:reload)).to include e1, e2 end it "enforces the limit on the number of enterprise owned" do - expect(u2.owned_enterprises).to eq [] + expect(u2.owned_enterprises(:reload)).to eq [] u2.owned_enterprises << e1 - u2.owned_enterprises << e2 + expect(u2.save!).to_not raise_error expect { + u2.owned_enterprises << e2 u2.save! - }.to raise_error ActiveRecord::RecordInvalid, "Validation failed: The nominated user is not permitted to own own any more enterprises." - + }.to raise_error ActiveRecord::RecordInvalid, "Validation failed: The nominated user is not permitted to own own any more enterprises (limit is 1)." end end end diff --git a/spec/support/request/authentication_workflow.rb b/spec/support/request/authentication_workflow.rb index 49b1512cac..a2ee597f27 100644 --- a/spec/support/request/authentication_workflow.rb +++ b/spec/support/request/authentication_workflow.rb @@ -38,7 +38,7 @@ module AuthenticationWorkflow end def create_enterprise_user(enterprises = []) - new_user = create(:user, email: 'enterprise@hub.com', password: 'blahblah', :password_confirmation => 'blahblah', ) + new_user = create(:user, password: 'blahblah', :password_confirmation => 'blahblah') new_user.spree_roles = [] # for some reason unbeknown to me, this new user gets admin permissions by default. for enterprise in enterprises do new_user.enterprise_roles.build(enterprise: enterprise).save