diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 1f87b8737ba..7af3d180a35 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -6,7 +6,8 @@ class EnterpriseRelationship < ActiveRecord::Base validates_presence_of :parent_id, :child_id validates_uniqueness_of :child_id, scope: :parent_id, message: I18n.t('validation_msg_relationship_already_established') - after_save :apply_variant_override_permissions + after_save :update_permissions_of_child_variant_overrides + before_destroy :revoke_all_child_variant_overrides scope :with_enterprises, joins('LEFT JOIN enterprises AS parent_enterprises ON parent_enterprises.id = enterprise_relationships.parent_id'). @@ -26,7 +27,6 @@ class EnterpriseRelationship < ActiveRecord::Base scope :by_name, with_enterprises.order('child_enterprises.name, parent_enterprises.name') - # Load an array of the relatives of each enterprise (ie. any enterprise related to it in # either direction). This array is split into distributors and producers, and has the format: # {enterprise_id => {distributors: [id, ...], producers: [id, ...]} } @@ -76,14 +76,24 @@ def has_permission?(name) private - def apply_variant_override_permissions - variant_overrides = VariantOverride.unscoped.for_hubs(child) - .joins(variant: :product).where("spree_products.supplier_id IN (?)", parent) - + def update_permissions_of_child_variant_overrides if has_permission?(:create_variant_overrides) - variant_overrides.update_all(permission_revoked_at: nil) + allow_all_child_variant_overrides else - variant_overrides.update_all(permission_revoked_at: Time.now) + revoke_all_child_variant_overrides end end + + def allow_all_child_variant_overrides + child_variant_overrides.update_all(permission_revoked_at: nil) + end + + def revoke_all_child_variant_overrides + child_variant_overrides.update_all(permission_revoked_at: Time.now) + end + + def child_variant_overrides + VariantOverride.unscoped.for_hubs(child) + .joins(variant: :product).where("spree_products.supplier_id IN (?)", parent) + end end diff --git a/db/migrate/20181020103501_revoke_variant_overrideswithout_permissions.rb b/db/migrate/20181020103501_revoke_variant_overrideswithout_permissions.rb new file mode 100644 index 00000000000..d8140c9bf39 --- /dev/null +++ b/db/migrate/20181020103501_revoke_variant_overrideswithout_permissions.rb @@ -0,0 +1,17 @@ +class RevokeVariantOverrideswithoutPermissions < ActiveRecord::Migration + def up + # This process was executed when the permission_revoked_at colum was created (see AddPermissionRevokedAtToVariantOverrides) + # It needs to be repeated due to #2739 + variant_override_hubs = Enterprise.where(id: VariantOverride.select(:hub_id).uniq) + + variant_override_hubs.find_each do |hub| + permitting_producer_ids = hub.relationships_as_child + .with_permission(:create_variant_overrides).pluck(:parent_id) + + variant_overrides_with_revoked_permissions = VariantOverride.for_hubs(hub) + .joins(variant: :product).where("spree_products.supplier_id NOT IN (?)", permitting_producer_ids) + + variant_overrides_with_revoked_permissions.update_all(permission_revoked_at: Time.now) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index c29fa95b9e6..3e60568a4a3 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 => 20181010093850) do +ActiveRecord::Schema.define(:version => 20181020103501) do create_table "account_invoices", :force => true do |t| t.integer "user_id", :null => false diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index 5dbdf1f3be3..650e4f1fa39 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -140,7 +140,7 @@ end describe "callbacks" do - context "applying variant override permissions" do + context "updating variant override permissions" do let(:hub) { create(:distributor_enterprise) } let(:producer) { create(:supplier_enterprise) } let(:some_other_producer) { create(:supplier_enterprise) } @@ -152,6 +152,17 @@ 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 "revoking variant override permissions" do + context "when the enterprise relationship is destroyed" do + before { er.destroy } + it "should set permission_revoked_at to the current time for all variant overrides of the relationship" do + expect(vo1.reload.permission_revoked_at).to_not be_nil + expect(vo2.reload.permission_revoked_at).to_not be_nil + expect(vo2.reload.permission_revoked_at).to_not be_nil + end + end + end + 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