From 4bf27982f4e5a23825fb19d49bf58f229080e564 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 25 Feb 2016 18:32:10 +1100 Subject: [PATCH] Applying variant override permissions when they are added or removed Also remove variant overrides with revoked permissions from the default scope --- app/models/enterprise_relationship.rb | 24 +++- app/models/variant_override.rb | 2 + spec/models/enterprise_relationship_spec.rb | 119 ++++++++++++++++++-- spec/models/variant_override_spec.rb | 6 + 4 files changed, 141 insertions(+), 10 deletions(-) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 8d279b2a20..da0996fdc5 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -6,6 +6,8 @@ class EnterpriseRelationship < ActiveRecord::Base validates_presence_of :parent_id, :child_id validates_uniqueness_of :child_id, scope: :parent_id, message: "^That relationship is already established." + after_save :apply_variant_override_permissions + 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') @@ -61,10 +63,28 @@ class EnterpriseRelationship < ActiveRecord::Base def permissions_list=(perms) - perms.andand.each { |name| permissions.build name: name } + if perms.nil? + permissions.destroy_all + else + permissions.where('name NOT IN (?)', perms).destroy_all + perms.map { |name| permissions.find_or_initialize_by_name name } + end end def has_permission?(name) - permissions.map(&:name).map(&:to_sym).include? name.to_sym + permissions(:reload).map(&:name).map(&:to_sym).include? name.to_sym + end + + private + + def apply_variant_override_permissions + variant_overrides = VariantOverride.unscoped.for_hubs(child) + .joins(variant: :product).where("spree_products.supplier_id IN (?)", parent) + + if has_permission?(:create_variant_overrides) + variant_overrides.update_all(permission_revoked_at: nil) + else + variant_overrides.update_all(permission_revoked_at: Time.now) + end end end diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index 21820ce0db..baede3e62d 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -6,6 +6,8 @@ class VariantOverride < ActiveRecord::Base # Default stock can be nil, indicating stock should not be reset or zero, meaning reset to zero. Need to ensure this can be set by the user. validates :default_stock, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true + default_scope where(permission_revoked_at: nil) + scope :for_hubs, lambda { |hubs| where(hub_id: hubs) } diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index fcdfa8b250..ca21cc7dfc 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -31,16 +31,51 @@ describe EnterpriseRelationship do 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).should match_array ['one', 'two'] + context "creating a new list of permissions" do + it "creates a new permission for each item in the list" do + er = EnterpriseRelationship.create! parent: e1, child: e2, permissions_list: ['one', 'two'] + er.reload + er.permissions.map(&:name).should match_array ['one', 'two'] + 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 - 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 + context "updating an existing list of permissions" do + let(:er) { create(:enterprise_relationship, parent: e1, child: e2, permissions_list: ["one", "two", "three"]) } + it "creates a new permission for each item in the list that has no existing permission" do + er.permissions_list = ['four'] + er.save! + er.reload + er.permissions.map(&:name).should include 'four' + end + + it "does not duplicate existing permissions" do + er.permissions_list = ["one", "two", "three"] + er.save! + er.reload + er.permissions.map(&:name).count.should == 3 + er.permissions.map(&:name).should match_array ["one", "two", "three"] + end + + it "removes permissions that are not in the list" do + er.permissions_list = ['one', 'three'] + er.save! + er.reload + er.permissions.map(&:name).should include 'one', 'three' + er.permissions.map(&:name).should_not include 'two' + end + + it "does removes all permissions when the list provided is nil" do + er.permissions_list = nil + er.save! + er.reload + er.permissions.should be_empty + end end end @@ -103,4 +138,72 @@ describe EnterpriseRelationship do EnterpriseRelationship.relatives[e2.id][:producers].should == Set.new([e1.id]) end end + + describe "callbacks" do + context "applying variant override permissions" do + let(:hub) { create(:distributor_enterprise) } + let(:producer) { create(:supplier_enterprise) } + let(:some_other_producer) { create(:supplier_enterprise) } + + context "when variant_override permission is present" do + let!(:er) { create(:enterprise_relationship, child: hub, parent: producer, permissions_list: [:add_to_order_cycles, :create_variant_overrides] )} + let!(:some_other_er) { create(:enterprise_relationship, child: hub, parent: some_other_producer, permissions_list: [:add_to_order_cycles, :create_variant_overrides] )} + let!(:vo1) { create(:variant_override, hub: hub, variant: create(:variant, product: create(:product, supplier: producer))) } + let!(:vo2) { create(:variant_override, hub: hub, variant: create(:variant, product: create(:product, supplier: producer))) } + let!(:vo3) { create(:variant_override, hub: hub, variant: create(:variant, product: create(:product, supplier: some_other_producer))) } + + context "and is then removed" do + before { er.permissions_list = [:add_to_order_cycles]; er.save! } + it "should set permission_revoked_at to the current time for all relevant variant overrides" do + expect(vo1.reload.permission_revoked_at).to_not be_nil + expect(vo2.reload.permission_revoked_at).to_not be_nil + end + + it "should not affect other variant overrides" do + expect(vo3.reload.permission_revoked_at).to be_nil + end + end + + context "and then some other permission is removed" do + before { er.permissions_list = [:create_variant_overrides]; er.save! } + + it "should have no effect on existing variant_overrides" do + expect(vo1.reload.permission_revoked_at).to be_nil + expect(vo2.reload.permission_revoked_at).to be_nil + expect(vo3.reload.permission_revoked_at).to be_nil + end + end + end + + context "when variant_override permission is not present" do + let!(:er) { create(:enterprise_relationship, child: hub, parent: producer, permissions_list: [:add_to_order_cycles] )} + let!(:some_other_er) { create(:enterprise_relationship, child: hub, parent: some_other_producer, permissions_list: [:add_to_order_cycles] )} + let!(:vo1) { create(:variant_override, hub: hub, variant: create(:variant, product: create(:product, supplier: producer)), permission_revoked_at: Time.now) } + let!(:vo2) { create(:variant_override, hub: hub, variant: create(:variant, product: create(:product, supplier: producer)), permission_revoked_at: Time.now) } + let!(:vo3) { create(:variant_override, hub: hub, variant: create(:variant, product: create(:product, supplier: some_other_producer)), permission_revoked_at: Time.now) } + + context "and is then added" do + before { er.permissions_list = [:add_to_order_cycles, :create_variant_overrides]; er.save! } + it "should set permission_revoked_at to nil for all relevant variant overrides" do + expect(vo1.reload.permission_revoked_at).to be_nil + expect(vo2.reload.permission_revoked_at).to be_nil + end + + it "should not affect other variant overrides" do + expect(vo3.reload.permission_revoked_at).to_not be_nil + end + end + + context "and then some other permission is added" do + before { er.permissions_list = [:add_to_order_cycles, :manage_products]; er.save! } + + it "should have no effect on existing variant_overrides" do + expect(vo1.reload.permission_revoked_at).to_not be_nil + expect(vo2.reload.permission_revoked_at).to_not be_nil + expect(vo3.reload.permission_revoked_at).to_not be_nil + end + end + end + end + end end diff --git a/spec/models/variant_override_spec.rb b/spec/models/variant_override_spec.rb index ed2f2c00f4..9efc53b521 100644 --- a/spec/models/variant_override_spec.rb +++ b/spec/models/variant_override_spec.rb @@ -10,6 +10,12 @@ describe VariantOverride do let(:v) { create(:variant) } let!(:vo1) { create(:variant_override, hub: hub1, variant: v) } let!(:vo2) { create(:variant_override, hub: hub2, variant: v) } + let!(:vo3) { create(:variant_override, hub: hub1, variant: v, permission_revoked_at: Time.now) } + + it "ignores variant_overrides with revoked_permissions by default" do + expect(VariantOverride.all).to_not include vo3 + expect(VariantOverride.unscoped).to include vo3 + end it "finds variant overrides for a set of hubs" do VariantOverride.for_hubs([hub1, hub2]).should match_array [vo1, vo2]