Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete tag assignments when deleting a tag that is referenced in an assignment #16039

Merged
merged 1 commit into from
Sep 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions app/models/classification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
11 changes: 10 additions & 1 deletion app/models/mixins/assignment_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
56 changes: 49 additions & 7 deletions spec/models/classification_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want this just to show that it deletes the correct tag but not the others (on the next two lines)
expect(Tag.exists?(:name => assignment_tag)).to be false

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
Expand Down
18 changes: 18 additions & 0 deletions spec/models/mixins/assignment_mixin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down