Commit 995f9c29 authored by Dmitriy Zaporozhets (DZ)'s avatar Dmitriy Zaporozhets (DZ) Committed by Adam Hegyi

Add pagination to integrated error tracking

parent c56d6c30
......@@ -15,8 +15,7 @@ module ErrorTracking
collection = by_status(collection)
collection = sort(collection)
# Limit collection until pagination implemented.
limit(collection)
collection.keyset_paginate(cursor: params[:cursor], per_page: limit)
end
private
......@@ -39,9 +38,9 @@ module ErrorTracking
params[:sort] ? collection.sort_by_attribute(params[:sort]) : collection.order_id_desc
end
def limit(collection)
def limit
# Restrict the maximum limit at 100 records.
collection.limit([(params[:limit] || 20).to_i, 100].min)
[(params[:limit] || 20).to_i, 100].min
end
end
end
......@@ -76,16 +76,21 @@ module ErrorTracking
filter_opts = {
status: opts[:issue_status],
sort: opts[:sort],
limit: opts[:limit]
limit: opts[:limit],
cursor: opts[:cursor]
}
errors = ErrorTracking::ErrorsFinder.new(current_user, project, filter_opts).execute
pagination = {}
pagination[:next] = { cursor: errors.cursor_for_next_page } if errors.has_next_page?
pagination[:previous] = { cursor: errors.cursor_for_previous_page } if errors.has_previous_page?
# We use the same response format as project_error_tracking_setting
# method below for compatibility with existing code.
{
issues: errors.map(&:to_sentry_error),
pagination: {}
pagination: pagination
}
else
project_error_tracking_setting.list_sentry_issues(**opts)
......
# frozen_string_literal: true
class ImproveIndexForErrorTracking < Gitlab::Database::Migration[1.0]
disable_ddl_transaction!
def up
add_concurrent_index :error_tracking_errors, %i(project_id status last_seen_at id),
order: { last_seen_at: :desc, id: :desc },
name: 'index_et_errors_on_project_id_and_status_last_seen_at_id_desc'
add_concurrent_index :error_tracking_errors, %i(project_id status first_seen_at id),
order: { first_seen_at: :desc, id: :desc },
name: 'index_et_errors_on_project_id_and_status_first_seen_at_id_desc'
add_concurrent_index :error_tracking_errors, %i(project_id status events_count id),
order: { events_count: :desc, id: :desc },
name: 'index_et_errors_on_project_id_and_status_events_count_id_desc'
remove_concurrent_index :error_tracking_errors, [:project_id, :status, :last_seen_at], name: 'index_et_errors_on_project_id_and_status_and_last_seen_at'
remove_concurrent_index :error_tracking_errors, [:project_id, :status, :first_seen_at], name: 'index_et_errors_on_project_id_and_status_and_first_seen_at'
remove_concurrent_index :error_tracking_errors, [:project_id, :status, :events_count], name: 'index_et_errors_on_project_id_and_status_and_events_count'
end
def down
add_concurrent_index :error_tracking_errors, [:project_id, :status, :last_seen_at], name: 'index_et_errors_on_project_id_and_status_and_last_seen_at'
add_concurrent_index :error_tracking_errors, [:project_id, :status, :first_seen_at], name: 'index_et_errors_on_project_id_and_status_and_first_seen_at'
add_concurrent_index :error_tracking_errors, [:project_id, :status, :events_count], name: 'index_et_errors_on_project_id_and_status_and_events_count'
remove_concurrent_index :error_tracking_errors, [:project_id, :status, :last_seen_at, :id], name: 'index_et_errors_on_project_id_and_status_last_seen_at_id_desc'
remove_concurrent_index :error_tracking_errors, [:project_id, :status, :first_seen_at, :id], name: 'index_et_errors_on_project_id_and_status_first_seen_at_id_desc'
remove_concurrent_index :error_tracking_errors, [:project_id, :status, :events_count, :id], name: 'index_et_errors_on_project_id_and_status_events_count_id_desc'
end
end
41ea0971cd62ba43bf98c1901169e7bb8fcebe68025d947f26b0ccf6806c976e
\ No newline at end of file
......@@ -25031,13 +25031,13 @@ CREATE UNIQUE INDEX index_escalation_rules_on_all_attributes ON incident_managem
CREATE INDEX index_escalation_rules_on_user ON incident_management_escalation_rules USING btree (user_id);
CREATE INDEX index_et_errors_on_project_id_and_status_and_events_count ON error_tracking_errors USING btree (project_id, status, events_count);
CREATE INDEX index_et_errors_on_project_id_and_status_and_id ON error_tracking_errors USING btree (project_id, status, id);
CREATE INDEX index_et_errors_on_project_id_and_status_and_first_seen_at ON error_tracking_errors USING btree (project_id, status, first_seen_at);
CREATE INDEX index_et_errors_on_project_id_and_status_events_count_id_desc ON error_tracking_errors USING btree (project_id, status, events_count DESC, id DESC);
CREATE INDEX index_et_errors_on_project_id_and_status_and_id ON error_tracking_errors USING btree (project_id, status, id);
CREATE INDEX index_et_errors_on_project_id_and_status_first_seen_at_id_desc ON error_tracking_errors USING btree (project_id, status, first_seen_at DESC, id DESC);
CREATE INDEX index_et_errors_on_project_id_and_status_and_last_seen_at ON error_tracking_errors USING btree (project_id, status, last_seen_at);
CREATE INDEX index_et_errors_on_project_id_and_status_last_seen_at_id_desc ON error_tracking_errors USING btree (project_id, status, last_seen_at DESC, id DESC);
CREATE INDEX index_events_on_action ON events USING btree (action);
......@@ -29,14 +29,25 @@ RSpec.describe ErrorTracking::ErrorsFinder do
context 'with sort parameter' do
let(:params) { { status: 'unresolved', sort: 'first_seen' } }
it { is_expected.to eq([error, error_yesterday]) }
it { expect(subject.to_a).to eq([error, error_yesterday]) }
end
context 'with limit parameter' do
context 'pagination' do
let(:params) { { limit: '1', sort: 'first_seen' } }
# Sort by first_seen is DESC by default, so the most recent error is `error`
it { is_expected.to contain_exactly(error) }
it { expect(subject.has_next_page?).to be_truthy }
it 'returns next page by cursor' do
params_with_cursor = params.merge(cursor: subject.cursor_for_next_page)
errors = described_class.new(user, project, params_with_cursor).execute
expect(errors).to contain_exactly(error_resolved)
expect(errors.has_next_page?).to be_truthy
expect(errors.has_previous_page?).to be_truthy
end
end
end
end
......@@ -5,56 +5,71 @@ require 'spec_helper'
RSpec.describe ErrorTracking::ListIssuesService do
include_context 'sentry error tracking context'
let(:params) { { search_term: 'something', sort: 'last_seen', cursor: 'some-cursor' } }
let(:list_sentry_issues_args) do
{
issue_status: 'unresolved',
limit: 20,
search_term: 'something',
sort: 'last_seen',
cursor: 'some-cursor'
}
end
let(:params) { {} }
subject { described_class.new(project, user, params) }
describe '#execute' do
context 'with authorized user' do
let(:issues) { [] }
context 'Sentry backend' do
let(:params) { { search_term: 'something', sort: 'last_seen', cursor: 'some-cursor' } }
let(:list_sentry_issues_args) do
{
issue_status: 'unresolved',
limit: 20,
search_term: 'something',
sort: 'last_seen',
cursor: 'some-cursor'
}
end
context 'with authorized user' do
let(:issues) { [] }
described_class::ISSUE_STATUS_VALUES.each do |status|
it "returns the issues with #{status} issue_status" do
params[:issue_status] = status
list_sentry_issues_args[:issue_status] = status
expect_list_sentry_issues_with(list_sentry_issues_args)
expect(result).to eq(status: :success, pagination: {}, issues: issues)
end
end
described_class::ISSUE_STATUS_VALUES.each do |status|
it "returns the issues with #{status} issue_status" do
params[:issue_status] = status
list_sentry_issues_args[:issue_status] = status
it 'returns the issues with no issue_status' do
expect_list_sentry_issues_with(list_sentry_issues_args)
expect(result).to eq(status: :success, pagination: {}, issues: issues)
end
end
it 'returns the issues with no issue_status' do
expect_list_sentry_issues_with(list_sentry_issues_args)
it 'returns bad request for an issue_status not on the whitelist' do
params[:issue_status] = 'assigned'
expect(error_tracking_setting).not_to receive(:list_sentry_issues)
expect(result).to eq(message: "Bad Request: Invalid issue_status", status: :error, http_status: :bad_request)
end
expect(result).to eq(status: :success, pagination: {}, issues: issues)
include_examples 'error tracking service data not ready', :list_sentry_issues
include_examples 'error tracking service sentry error handling', :list_sentry_issues
include_examples 'error tracking service http status handling', :list_sentry_issues
end
it 'returns bad request for an issue_status not on the whitelist' do
params[:issue_status] = 'assigned'
include_examples 'error tracking service unauthorized user'
include_examples 'error tracking service disabled'
expect(error_tracking_setting).not_to receive(:list_sentry_issues)
expect(result).to eq(message: "Bad Request: Invalid issue_status", status: :error, http_status: :bad_request)
def expect_list_sentry_issues_with(list_sentry_issues_args)
expect(error_tracking_setting)
.to receive(:list_sentry_issues)
.with(list_sentry_issues_args)
.and_return(issues: [], pagination: {})
end
include_examples 'error tracking service data not ready', :list_sentry_issues
include_examples 'error tracking service sentry error handling', :list_sentry_issues
include_examples 'error tracking service http status handling', :list_sentry_issues
end
include_examples 'error tracking service unauthorized user'
include_examples 'error tracking service disabled'
context 'GitLab backend' do
let_it_be(:error1) { create(:error_tracking_error, name: 'foo', project: project) }
let_it_be(:error2) { create(:error_tracking_error, name: 'bar', project: project) }
context 'integrated error tracking' do
let_it_be(:error) { create(:error_tracking_error, project: project) }
let(:params) { { limit: '1' } }
before do
error_tracking_setting.update!(integrated: true)
......@@ -63,7 +78,9 @@ RSpec.describe ErrorTracking::ListIssuesService do
it 'returns the error in expected format' do
expect(result[:status]).to eq(:success)
expect(result[:issues].size).to eq(1)
expect(result[:issues].first.to_json).to eq(error.to_sentry_error.to_json)
expect(result[:issues].first.to_json).to eq(error2.to_sentry_error.to_json)
expect(result[:pagination][:next][:cursor]).to be_present
expect(result[:pagination][:previous]).to be_nil
end
end
end
......@@ -76,10 +93,3 @@ RSpec.describe ErrorTracking::ListIssuesService do
end
end
end
def expect_list_sentry_issues_with(list_sentry_issues_args)
expect(error_tracking_setting)
.to receive(:list_sentry_issues)
.with(list_sentry_issues_args)
.and_return(issues: [], pagination: {})
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment