From dba7bf9ac937db7bfe5c002dfa37365389ae6562 Mon Sep 17 00:00:00 2001 From: Gregg Tanzillo Date: Mon, 25 Sep 2017 17:55:24 -0400 Subject: [PATCH] Delete tag assignments when deleting a tag that is referenced in an assignment Eg. - Tag to be deleted - /managed/environment/any1 - Referenced in assignment by the tag /chargeback_rate/assigned_to/vm/tag/managed/environment/any1. Which means that a chargeback rate (tagged with the latter) is assigned to any Vm tagged with environment/any1. Should also be deleted https://bugzilla.redhat.com/show_bug.cgi?id=1483365 --- app/models/classification.rb | 16 +++++- app/models/mixins/assignment_mixin.rb | 11 +++- spec/models/classification_spec.rb | 56 ++++++++++++++++++--- spec/models/mixins/assignment_mixin_spec.rb | 18 +++++++ 4 files changed, 91 insertions(+), 10 deletions(-) diff --git a/app/models/classification.rb b/app/models/classification.rb index 7457d8c3a15..c18a96815e1 100644 --- a/app/models/classification.rb +++ b/app/models/classification.rb @@ -521,7 +521,14 @@ def save_tag end def delete_all_entries - entries.each(&:delete_tag_and_taggings) + entries.each do |e| + e.delete_assignments + e.delete_tag_and_taggings + end + end + + def delete_assignments + AssignmentMixin.all_assignments(tag.name).destroy_all end def delete_tag_and_taggings @@ -532,7 +539,12 @@ def delete_tag_and_taggings end def delete_tags_and_entries - delete_all_entries if category? + if category? + delete_all_entries + else # entry + delete_assignments + end + delete_tag_and_taggings end end diff --git a/app/models/mixins/assignment_mixin.rb b/app/models/mixins/assignment_mixin.rb index 7567e95f0d5..0d80aaee44d 100644 --- a/app/models/mixins/assignment_mixin.rb +++ b/app/models/mixins/assignment_mixin.rb @@ -3,6 +3,15 @@ module AssignmentMixin extend ActiveSupport::Concern ESCAPED_PREFIX = "escaped".freeze + NAMESPACE_SUFFIX = "assigned_to".freeze + + def all_assignments(tag = nil) + scope = Tag.where(["name LIKE ?", "%/#{AssignmentMixin::NAMESPACE_SUFFIX}/%"]) + scope = scope.where(["name LIKE ?", "%#{tag}"]) if tag.present? + + scope + end + module_function :all_assignments included do #:nodoc: acts_as_miq_taggable @@ -188,7 +197,7 @@ def get_assigned_for_target(target, options = {}) end def namespace - "/#{base_model.name.underscore}/assigned_to" + "/#{base_model.name.underscore}/#{NAMESPACE_SUFFIX}" end end # module ClassMethods end # module AssignmentMixin diff --git a/spec/models/classification_spec.rb b/spec/models/classification_spec.rb index ef72f331d6a..076a866dbb6 100644 --- a/spec/models/classification_spec.rb +++ b/spec/models/classification_spec.rb @@ -61,14 +61,56 @@ FactoryGirl.create(:classification_tag, :name => "multi_entry_2", :parent => parent) end - it "#destroy" do - cat = Classification.find_by_name("test_category") - expect(cat).to_not be_nil - entries = cat.entries - expect(entries.length).to eq(2) + context "#destroy" do + it "a category deletes all entries" do + cat = Classification.find_by_name("test_category") + expect(cat).to_not be_nil + entries = cat.entries + expect(entries.length).to eq(2) + + cat.destroy + entries.each { |e| expect(Classification.find_by(:id => e.id)).to be_nil } + end + + it "a category deletes assignments referenced by its entries" do + cat = Classification.find_by_name("test_category") + assignment_tag = "/chargeback_rate/assigned_to/vm/tag/managed/test_category/test_entry" + + Tag.create!(:name => assignment_tag) + expect(Tag.exists?(:name => assignment_tag)).to be true + + cat.destroy + + expect(Tag.exists?(:name => assignment_tag)).to be false + end - cat.destroy - entries.each { |e| expect(Classification.find_by(:id => e.id)).to be_nil } + it "a category does not delete assignments that are close but do not match its tag" do + cat = Classification.find_by_name("test_category") + assignment_tag = "/chargeback_rate/assigned_to/vm/tag/managed/test_category/test_entry" + another_tag1 = Tag.create(:name => "/policy_set/assigned_to/vm/tag/managed/test_category/test_entry1") + another_tag2 = Tag.create(:name => "/chargeback_rate/assigned_to/vm/tag/managed/test_category1/test_entry") + + Tag.create!(:name => assignment_tag) + + cat.destroy + + expect(Tag.exists?(:name => assignment_tag)).to be false + expect(Tag.exists?(:name => another_tag1.name)).to be true + expect(Tag.exists?(:name => another_tag2.name)).to be true + end + + it "an entry deletes assignments where its tag is referenced" do + cat = Classification.find_by_name("test_category") + ent = cat.entries.find { |e| e.name == "test_entry" } + assignment_tag = "/chargeback_rate/assigned_to/vm/tag/managed/test_category/test_entry" + + Tag.create!(:name => assignment_tag) + expect(Tag.exists?(:name => assignment_tag)).to be true + + ent.destroy + + expect(Tag.exists?(:name => assignment_tag)).to be false + end end it "should test setup data" do diff --git a/spec/models/mixins/assignment_mixin_spec.rb b/spec/models/mixins/assignment_mixin_spec.rb index 77331bbd5a8..4c6a8c50595 100644 --- a/spec/models/mixins/assignment_mixin_spec.rb +++ b/spec/models/mixins/assignment_mixin_spec.rb @@ -25,6 +25,24 @@ end end + describe ".all_assignments" do + it "returns only tags representing assignments" do + t1 = Tag.create(:name => "/chargeback_rate/assigned_to/vm/tag/managed/environment/any1") + Tag.create(:name => "/something/with_the same_tag/vm/tag/managed/environment/any1") + + expect(described_class.all_assignments.all).to eq([t1]) + end + + it "returns only tags representing assignments that match tag in argument" do + Tag.create(:name => "/chargeback_rate/assigned_to/vm/tag/managed/environment/any1") + t2 = Tag.create(:name => "/chargeback_rate/assigned_to/vm/tag/managed/environment/any2") + + expect( + described_class.all_assignments("/chargeback_rate/assigned_to/vm/tag/managed/environment/any2").all + ).to eq([t2]) + end + end + private # creates a tag e.g. "/managed/environment/test"