From 3e1b9f9ee92704b7a620d8d11e937374575dfe09 Mon Sep 17 00:00:00 2001 From: Joe Rafaniello Date: Fri, 29 Sep 2017 14:00:32 -0400 Subject: [PATCH] Merge pull request #16039 from gtanzillo/delete-tag-assignments Delete tag assignments when deleting a tag that is referenced in an assignment (cherry picked from commit fc67deb5c3a1eca38574e9001ec52f933da0b112) https://bugzilla.redhat.com/show_bug.cgi?id=1497748 --- 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 9133dce6468..856560533ec 100644 --- a/app/models/classification.rb +++ b/app/models/classification.rb @@ -531,7 +531,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 @@ -542,7 +549,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 8c3146bcb68..d719643b118 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 @@ -190,7 +199,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 bd91fb7485f..cebb3ba4fac 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"