Fixes #29433 - seeding with scope

db/seeds.d/150-bookmarks.rb

@@ -8,7 +8,7 @@
     { :name => "disabled", :query => 'status.enabled = false', :controller => "hosts" },
     { :name => "ok hosts", :query => 'last_report > "35 minutes ago" and status.enabled = true and status.applied = 0 and status.failed = 0 and status.pending = 0', :controller => "hosts" },
   ].each do |input|
-    next if Bookmark.where(:controller => input[:controller], :name => input[:name]).first
+    next if Bookmark.where(:controller => input[:controller], :name => input[:name]).exists?
     next if SeedHelper.audit_modified? Bookmark, input[:name], :controller => input[:controller]
     b = Bookmark.create({ :public => true }.merge(input))
     raise "Unable to create bookmark: #{format_errors b}" if b.nil? || b.errors.any?

lib/seed_helper.rb

@@ -8,32 +8,34 @@ class << self
     # that it can be slow if there's high number of audits for the specified type
     def audit_modified?(type, name, attributes = {})
       audits = Audit.where(:auditable_type => type.base_class.name, :auditable_name => name)
-      audits = audits_from_attributes(audits, attributes) if attributes.present?
 
-      return true if audits.where(:action => :destroy).present?
+      return true if filter_destroy_audits(audits.where(:action => :destroy), attributes).present?
       audits.where(:action => :update).each do |audit|
-        return true if audit_changed(audit, name)
+        return true if audit_changed(audit, name) && audited_matches_attributes?(audit, attributes)
       end
       false
     end
 
-    def audit_changed(audit, name)
-      audit.audited_changes['name'].is_a?(Array) && audit.audited_changes['name'].first == name
-    end
-
-    def audits_from_attributes(audits, attributes)
-      audits.where(:id => interesting_audits(audits, attributes).map(&:id))
-    end
-
-    def interesting_audits(audits, attributes)
+    def filter_destroy_audits(audits, attributes)
+      return audits unless attributes.present?
       audits.select do |audit|
         attributes.all? do |attribute, value|
-          changed_attribute = audit.audited_changes[attribute]
-          (audit.action == 'update') ? changed_attribute.first == value : changed_attribute == value
+          audit.audited_changes[attribute] == value
         end
       end
     end
 
+    def audited_matches_attributes?(audit, attributes)
+      return true unless attributes.present?
+      matching_attributes_on = audit.auditable
+      matching_attributes_on ||= Audit.find_by(:auditable_type => audit.auditable_type, :auditable_id => audit.auditable_id, :action => :destroy)&.audited_changes
+      matching_attributes_on.nil? || attributes.all? { |attr, val| matching_attributes_on[attr.to_s] == val }
+    end
+
+    def audit_changed(audit, name)
+      audit.audited_changes['name'].is_a?(Array) && audit.audited_changes['name'].first == name
+    end
+
     def create_filters(role, collection)
       collection.group_by(&:resource_type).each do |resource, permissions|
         filter      = Filter.new

test/factories/bookmark.rb

@@ -2,6 +2,7 @@
   factory :bookmark do
     sequence(:name) { |n| "bookmark_#{n}" }
     query { "bar" }
+    controller { 'hosts' }
     # rubocop:disable Layout/EmptyLinesAroundAccessModifier
     public { false }
     # rubocop:enable Layout/EmptyLinesAroundAccessModifier

test/unit/seed_helper_test.rb

@@ -75,12 +75,48 @@ class SeedHelperTest < ActiveSupport::TestCase
     assert_includes role.filters.pluck(:search), 'name = example.com'
   end
 
-  test "should recognize object was modified" do
-    medium = Medium.last
-    medium_name = medium.name
-    refute SeedHelper.audit_modified?(Medium, medium.name)
-    medium.update(:name => "renamed medium")
-    assert SeedHelper.audit_modified?(Medium, medium_name)
+  describe '#audit_modified?' do
+    it "should recognize object was modified" do
+      medium = Medium.last
+      medium_name = medium.name
+      refute SeedHelper.audit_modified?(Medium, medium.name)
+      medium.update(:name => "renamed medium")
+      assert SeedHelper.audit_modified?(Medium, medium_name)
+    end
+
+    context 'with attributes filtering' do
+      let(:bookmark) { FactoryBot.create(:bookmark, :name => 'two', :public => true, :controller => 'config_reports') }
+
+      it "recognizes irrelevant change" do
+        refute SeedHelper.audit_modified?(Bookmark, bookmark.name, :controller => bookmark.controller)
+        bookmark.update(:query => "name = barbar")
+        bookmarks(:two).update(:name => 'modified irelevant')
+        refute SeedHelper.audit_modified?(Medium, bookmark.name, :controller => bookmark.controller)
+      end
+
+      it "recognizes relevant changes in complex history" do
+        old_name = bookmark.name
+        refute SeedHelper.audit_modified?(Bookmark, bookmark.name, :controller => bookmark.controller)
+        bookmark.update(:query => "name = barbar")
+        bookmark.update(:name => 'modified')
+        bookmark.update(:name => 'modified2')
+        bookmark.update(:query => "name = bar2")
+        assert SeedHelper.audit_modified?(Bookmark, old_name, :controller => bookmark.controller)
+        bookmarks(:two).destroy
+        bookmark.destroy
+        assert SeedHelper.audit_modified?(Bookmark, old_name, :controller => bookmark.controller)
+      end
+
+      it "recognizes irrelevant changes in complex history" do
+        refute SeedHelper.audit_modified?(Bookmark, bookmark.name, :controller => bookmark.controller)
+        bookmarks(:two)
+        bookmarks(:two).update(:name => 'modified')
+        bookmarks(:two).update(:name => 'modified2')
+        bookmarks(:two).destroy
+        bookmark.update(:query => "name = barbar")
+        refute SeedHelper.audit_modified?(Bookmark, bookmark.name, :controller => bookmark.controller)
+      end
+    end
   end
 
   describe '.import_raw_template' do