Browse Source

Maintenance: Does skip the job creation if no changes happened and fixes missing search index update (#4306).

Rolf Schmidt 2 years ago
parent
commit
886cd320c8

+ 3 - 21
app/models/concerns/has_search_index_backend.rb

@@ -4,9 +4,7 @@ module HasSearchIndexBackend
   extend ActiveSupport::Concern
 
   included do
-    after_create  :search_index_update
-    after_update  :search_index_update
-    after_touch   :search_index_update_touch
+    after_commit  :search_index_update, if: :persisted?
     after_destroy :search_index_destroy
   end
 
@@ -25,26 +23,10 @@ update search index, if configured - will be executed automatically
     # start background job to transfer data to search index
     return true if !SearchIndexBackend.enabled?
 
-    SearchIndexAssociationsJob.perform_later(self.class.to_s, id)
-    true
-  end
-
-=begin
-
-update search index, if configured - will be executed automatically
-
-  model = Model.find(123)
-  model.search_index_update_touch
-
-=end
-
-  def search_index_update_touch
-    return true if ignore_search_indexing?(:update)
-
-    # start background job to transfer data to search index
-    return true if !SearchIndexBackend.enabled?
+    return true if previous_changes.blank?
 
     SearchIndexJob.perform_later(self.class.to_s, id)
+    SearchIndexAssociationsJob.perform_later(self.class.to_s, id)
     true
   end
 

+ 2 - 0
app/models/ticket/enqueues_user_ticket_counter_job.rb

@@ -20,6 +20,8 @@ module Ticket::EnqueuesUserTicketCounterJob
 
     return true if !customer_id
 
+    return true if previous_changes.blank?
+
     # send background job
     TicketUserTicketCounterJob.perform_later(
       customer_id,

+ 2 - 0
app/models/ticket/touches_associations.rb

@@ -17,6 +17,8 @@ module Ticket::TouchesAssociations
     # return if we run import mode
     return true if Setting.get('import_mode')
 
+    return true if saved_changes.blank?
+
     # touch old customer if changed
     customer_id_changed = saved_changes['customer_id']
     if customer_id_changed && customer_id_changed[0] != customer_id_changed[1] && customer_id_changed[0]

+ 19 - 6
spec/models/ticket/has_search_index_backend_spec.rb

@@ -3,12 +3,6 @@
 require 'rails_helper'
 
 RSpec.describe 'HasSearchIndexBackend', type: :model, searchindex: true, performs_jobs: true do
-  before do
-    article && organization
-
-    searchindex_model_reload([::Ticket, ::Organization])
-  end
-
   describe 'Updating referenced data between ticket and organizations' do
     let(:organization) { create(:organization, name: 'Tomato42') }
     let(:user)         { create(:customer, organization: organization) }
@@ -25,6 +19,12 @@ RSpec.describe 'HasSearchIndexBackend', type: :model, searchindex: true, perform
       article
     end
 
+    before do
+      article && organization
+
+      searchindex_model_reload([::Ticket, ::Organization])
+    end
+
     it 'finds added tickets' do
       result = SearchIndexBackend.search('organization.name:Tomato42', 'Ticket', sort_by: ['updated_at'], order_by: ['desc'])
       expect(result).to eq([{ id: ticket.id.to_s, type: 'Ticket' }])
@@ -81,4 +81,17 @@ RSpec.describe 'HasSearchIndexBackend', type: :model, searchindex: true, perform
       expect(Ticket).not_to be_search_index_attribute_relevant('updated_by_id')
     end
   end
+
+  describe 'Updating group settings causes huge numbers of delayed jobs #4306' do
+    let(:ticket) { create(:ticket) }
+
+    before do
+      ticket
+      Delayed::Job.destroy_all
+    end
+
+    it 'does not create any jobs if nothing has changed' do
+      expect { ticket.update(title: ticket.title) }.not_to change(Delayed::Job, :count)
+    end
+  end
 end

+ 19 - 6
spec/models/user/has_search_index_backend_spec.rb

@@ -3,16 +3,16 @@
 require 'rails_helper'
 
 RSpec.describe 'HasSearchIndexBackend', type: :model, searchindex: true do
-  before do
-    user && organization
-
-    searchindex_model_reload([::User, ::Organization])
-  end
-
   describe 'Updating referenced data between user and organizations' do
     let(:organization) { create(:organization, name: 'Tomato42') }
     let(:user)         { create(:customer, organization: organization) }
 
+    before do
+      user && organization
+
+      searchindex_model_reload([::User, ::Organization])
+    end
+
     it 'finds added users' do
       result = SearchIndexBackend.search('organization.name:Tomato42', 'User', sort_by: ['updated_at'], order_by: ['desc'])
       expect(result).to eq([{ id: user.id.to_s, type: 'User' }])
@@ -40,4 +40,17 @@ RSpec.describe 'HasSearchIndexBackend', type: :model, searchindex: true do
       expect(organization).to be_search_index_indexable_bulk_updates(User)
     end
   end
+
+  describe 'Updating group settings causes huge numbers of delayed jobs #4306' do
+    let(:user) { create(:user) }
+
+    before do
+      user
+      Delayed::Job.destroy_all
+    end
+
+    it 'does not create any jobs if nothing has changed' do
+      expect { user.update(firstname: user.firstname) }.not_to change(Delayed::Job, :count)
+    end
+  end
 end