7 files changed, 59 insertions, 4 deletions
diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb
index fb788c47ef1..6944857bd33 100644
--- a/app/controllers/admin/hooks_controller.rb
+++ b/app/controllers/admin/hooks_controller.rb
@@ -52,8 +52,7 @@ class Admin::HooksController < Admin::ApplicationController
   end
 
   def hook_logs
-    @hook_logs ||=
-      Kaminari.paginate_array(hook.web_hook_logs.order(created_at: :desc)).page(params[:page])
+    @hook_logs ||= hook.web_hook_logs.recent.page(params[:page])
   end
 
   def hook_params
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index dd7aa1a67b9..6800d742b0a 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -58,8 +58,7 @@ class Projects::HooksController < Projects::ApplicationController
   end
 
   def hook_logs
-    @hook_logs ||=
-      Kaminari.paginate_array(hook.web_hook_logs.order(created_at: :desc)).page(params[:page])
+    @hook_logs ||= hook.web_hook_logs.recent.page(params[:page])
   end
 
   def hook_params
diff --git a/app/models/hooks/web_hook_log.rb b/app/models/hooks/web_hook_log.rb
index e72c125fb69..59a1f2aed69 100644
--- a/app/models/hooks/web_hook_log.rb
+++ b/app/models/hooks/web_hook_log.rb
@@ -7,6 +7,11 @@ class WebHookLog < ActiveRecord::Base
 
   validates :web_hook, presence: true
 
+  def self.recent
+    where('created_at >= ?', 2.days.ago.beginning_of_day)
+      .order(created_at: :desc)
+  end
+
   def success?
     response_status =~ /^2/
   end
diff --git a/changelogs/unreleased/web-hooks-log-pagination.yml b/changelogs/unreleased/web-hooks-log-pagination.yml
new file mode 100644
index 00000000000..fd9e4f9ca13
--- /dev/null
+++ b/changelogs/unreleased/web-hooks-log-pagination.yml
@@ -0,0 +1,5 @@
+---
+title: Fixed pagination of web hook logs
+merge_request:
+author:
+type: performance
diff --git a/db/migrate/20180628124813_alter_web_hook_logs_indexes.rb b/db/migrate/20180628124813_alter_web_hook_logs_indexes.rb
new file mode 100644
index 00000000000..1878e76811d
--- /dev/null
+++ b/db/migrate/20180628124813_alter_web_hook_logs_indexes.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AlterWebHookLogsIndexes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  # "created_at" comes first so the Sidekiq worker pruning old webhook logs can
+  # use a composite index index.
+  #
+  # We leave the old standalone index on "web_hook_id" in place so future code
+  # that doesn't care about "created_at" can still use that index.
+  COLUMNS_TO_INDEX = %i[created_at web_hook_id]
+
+  def up
+    add_concurrent_index(:web_hook_logs, COLUMNS_TO_INDEX)
+  end
+
+  def down
+    remove_concurrent_index(:web_hook_logs, COLUMNS_TO_INDEX)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 384a1ec6d37..5d917abdbb5 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -2145,6 +2145,7 @@ ActiveRecord::Schema.define(version: 20180629191052) do
     t.datetime "updated_at", null: false
   end
 
+  add_index "web_hook_logs", ["created_at", "web_hook_id"], name: "index_web_hook_logs_on_created_at_and_web_hook_id", using: :btree
   add_index "web_hook_logs", ["web_hook_id"], name: "index_web_hook_logs_on_web_hook_id", using: :btree
 
   create_table "web_hooks", force: :cascade do |t|
diff --git a/spec/models/hooks/web_hook_log_spec.rb b/spec/models/hooks/web_hook_log_spec.rb
index 19bc88b1333..744a6ccae8b 100644
--- a/spec/models/hooks/web_hook_log_spec.rb
+++ b/spec/models/hooks/web_hook_log_spec.rb
@@ -9,6 +9,24 @@ describe WebHookLog do
 
   it { is_expected.to validate_presence_of(:web_hook) }
 
+  describe '.recent' do
+    let(:hook) { create(:project_hook) }
+
+    it 'does not return web hook logs that are too old' do
+      create(:web_hook_log, web_hook: hook, created_at: 91.days.ago)
+
+      expect(described_class.recent.size).to be_zero
+    end
+
+    it 'returns the web hook logs in descending order' do
+      hook1 = create(:web_hook_log, web_hook: hook, created_at: 2.hours.ago)
+      hook2 = create(:web_hook_log, web_hook: hook, created_at: 1.hour.ago)
+      hooks = described_class.recent.to_a
+
+      expect(hooks).to eq([hook2, hook1])
+    end
+  end
+
   describe '#success?' do
     let(:web_hook_log) { build(:web_hook_log, response_status: status) }