Browse Source

Performance: Existing Notification type History lookup should be done on DB level.

Rolf Schmidt 5 years ago
parent
commit
b468134e93
3 changed files with 8 additions and 52 deletions
  1. 1 16
      app/models/history.rb
  2. 7 9
      app/models/transaction/notification.rb
  3. 0 27
      spec/models/history_spec.rb

+ 1 - 16
app/models/history.rb

@@ -146,20 +146,9 @@ returns
     assets: assets,
   }
 
-return all history entries of an object and it's assets and extended with an condition (e. g. to only retrive new history entries)
-
-  history = History.list('Ticket', 123, nil, true, ['created_at > ?', [Time.zone.now - 2.days]])
-
-returns
-
-  history = {
-    list: list,
-    assets: assets,
-  }
-
 =end
 
-  def self.list(requested_object, requested_object_id, related_history_object = nil, assets = nil, condition = nil)
+  def self.list(requested_object, requested_object_id, related_history_object = nil, assets = nil)
     histories = History.where(
       history_object_id: object_lookup(requested_object).id,
       o_id:              requested_object_id
@@ -174,10 +163,6 @@ returns
       )
     end
 
-    if condition.present?
-      histories = histories.where(condition)
-    end
-
     histories = histories.order(:created_at)
 
     list = histories.map(&:attributes).each do |data|

+ 7 - 9
app/models/transaction/notification.rb

@@ -111,15 +111,13 @@ class Transaction::Notification
         if !identifier || identifier == ''
           identifier = user.login
         end
-        already_notified = false
-        History.list('Ticket', ticket.id, nil, nil, ['created_at > ?', [Time.zone.now - 2.days]]).each do |history|
-          next if history['type'] != 'notification'
-          next if !history['value_to'].match?(/\(#{Regexp.escape(@item[:type])}:/)
-          next if !history['value_to'].match?(/#{Regexp.escape(identifier)}\(/)
-          next if !history['created_at'].today?
-
-          already_notified = true
-        end
+
+        already_notified = History.where(
+          history_type_id:   History.type_lookup('notification').id,
+          history_object_id: History.object_lookup('Ticket').id,
+          o_id:              ticket.id
+        ).where('created_at > ?', Time.zone.now.beginning_of_day).where('value_to LIKE ?', "%#{identifier}(#{@item[:type]}:%").exists?
+
         next if already_notified
       end
 

+ 0 - 27
spec/models/history_spec.rb

@@ -188,32 +188,5 @@ RSpec.describe History, type: :model do
         end
       end
     end
-
-    context 'when given an object with histories' do
-      context 'and called without "condition" argument' do
-        let!(:object) { create(:user) }
-
-        before do
-          travel 3.days
-          object.update(email: 'foo@example.com')
-        end
-
-        context 'or "assets" flag' do
-          let(:list) { described_class.list(object.class.name, object.id, nil, nil, ['created_at > ?', [Time.zone.now - 2.days]]) }
-
-          it 'returns an array of attribute hashes for those histories' do
-            expect(list).to match_array(
-              [
-                hash_including(
-                  'type'     => 'updated',
-                  'o_id'     => object.id,
-                  'value_to' => 'foo@example.com',
-                )
-              ]
-            )
-          end
-        end
-      end
-    end
   end
 end