From b468134e93c8356dc9c2297e76b6f130878ddfb8 Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@znuny.com>
Date: Fri, 17 Jan 2020 17:08:43 +0100
Subject: [PATCH] Performance: Existing Notification type History lookup should
 be done on DB level.

---
 app/models/history.rb                  | 17 +---------------
 app/models/transaction/notification.rb | 14 ++++++-------
 spec/models/history_spec.rb            | 27 --------------------------
 3 files changed, 7 insertions(+), 51 deletions(-)

diff --git a/app/models/history.rb b/app/models/history.rb
index dee341269..662a555e6 100644
--- a/app/models/history.rb
+++ b/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|
diff --git a/app/models/transaction/notification.rb b/app/models/transaction/notification.rb
index f6593a36f..d045ee5f6 100644
--- a/app/models/transaction/notification.rb
+++ b/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
 
diff --git a/spec/models/history_spec.rb b/spec/models/history_spec.rb
index 97a1ee57c..0f60f3189 100644
--- a/spec/models/history_spec.rb
+++ b/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