Commit 60730fd8 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'lm-remove-counts-and-redirect' into 'master'

Remove count for pending/running/finished pipelines in tabs

See merge request gitlab-org/gitlab!35693
parents d5dfd781 c17127d4
...@@ -19,6 +19,9 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -19,6 +19,9 @@ class Projects::PipelinesController < Projects::ApplicationController
end end
before_action :ensure_pipeline, only: [:show] before_action :ensure_pipeline, only: [:show]
# Will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/225596
before_action :redirect_for_legacy_scope_filter, only: [:index], if: -> { request.format.html? }
around_action :allow_gitaly_ref_name_caching, only: [:index, :show] around_action :allow_gitaly_ref_name_caching, only: [:index, :show]
track_unique_visits :charts, target_id: 'p_analytics_pipelines' track_unique_visits :charts, target_id: 'p_analytics_pipelines'
...@@ -34,9 +37,6 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -34,9 +37,6 @@ class Projects::PipelinesController < Projects::ApplicationController
.page(params[:page]) .page(params[:page])
.per(30) .per(30)
@running_count = limited_pipelines_count(project, 'running')
@pending_count = limited_pipelines_count(project, 'pending')
@finished_count = limited_pipelines_count(project, 'finished')
@pipelines_count = limited_pipelines_count(project) @pipelines_count = limited_pipelines_count(project)
respond_to do |format| respond_to do |format|
...@@ -47,10 +47,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -47,10 +47,7 @@ class Projects::PipelinesController < Projects::ApplicationController
render json: { render json: {
pipelines: serialize_pipelines, pipelines: serialize_pipelines,
count: { count: {
all: @pipelines_count, all: @pipelines_count
running: @running_count,
pending: @pending_count,
finished: @finished_count
} }
} }
end end
...@@ -229,6 +226,12 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -229,6 +226,12 @@ class Projects::PipelinesController < Projects::ApplicationController
render_404 unless pipeline render_404 unless pipeline
end end
def redirect_for_legacy_scope_filter
return unless %w[running pending].include?(params[:scope])
redirect_to url_for(safe_params.except(:scope).merge(status: safe_params[:scope])), status: :moved_permanently
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def pipeline def pipeline
@pipeline ||= if params[:id].blank? && params[:latest] @pipeline ||= if params[:id].blank? && params[:latest]
......
---
title: Remove count for pending/running/finished pipelines in tabs
merge_request: 35693
author:
type: changed
...@@ -37,9 +37,6 @@ RSpec.describe Projects::PipelinesController do ...@@ -37,9 +37,6 @@ RSpec.describe Projects::PipelinesController do
expect(json_response).to include('pipelines') expect(json_response).to include('pipelines')
expect(json_response['pipelines'].count).to eq 6 expect(json_response['pipelines'].count).to eq 6
expect(json_response['count']['all']).to eq '6' expect(json_response['count']['all']).to eq '6'
expect(json_response['count']['running']).to eq '2'
expect(json_response['count']['pending']).to eq '1'
expect(json_response['count']['finished']).to eq '3'
json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages| json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages|
expect(stages.count).to eq 3 expect(stages.count).to eq 3
...@@ -122,13 +119,15 @@ RSpec.describe Projects::PipelinesController do ...@@ -122,13 +119,15 @@ RSpec.describe Projects::PipelinesController do
end end
end end
context 'filter by scope' do context 'when user tries to access legacy scope via URL' do
it 'returns matched pipelines' do it 'redirects to all pipelines with that status instead' do
get_pipelines_index_json(scope: 'running') get_pipelines_index_html(scope: 'running')
check_pipeline_response(returned: 2, all: 6, running: 2, pending: 1, finished: 3) expect(response).to redirect_to(project_pipelines_path(project, status: 'running', format: :html))
end end
end
context 'filter by scope' do
context 'scope is branches or tags' do context 'scope is branches or tags' do
before do before do
create(:ci_pipeline, :failed, project: project, ref: 'v1.0.0', tag: true) create(:ci_pipeline, :failed, project: project, ref: 'v1.0.0', tag: true)
...@@ -140,7 +139,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -140,7 +139,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns matched pipelines' do it 'returns matched pipelines' do
get_pipelines_index_json(scope: 'branches') get_pipelines_index_json(scope: 'branches')
check_pipeline_response(returned: 2, all: 9, running: 2, pending: 1, finished: 6) check_pipeline_response(returned: 2, all: 9)
end end
end end
...@@ -148,7 +147,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -148,7 +147,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns matched pipelines' do it 'returns matched pipelines' do
get_pipelines_index_json(scope: 'tags') get_pipelines_index_json(scope: 'tags')
check_pipeline_response(returned: 1, all: 9, running: 2, pending: 1, finished: 6) check_pipeline_response(returned: 1, all: 9)
end end
end end
end end
...@@ -161,7 +160,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -161,7 +160,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns matched pipelines' do it 'returns matched pipelines' do
get_pipelines_index_json(username: user.username) get_pipelines_index_json(username: user.username)
check_pipeline_response(returned: 1, all: 1, running: 1, pending: 0, finished: 0) check_pipeline_response(returned: 1, all: 1)
end end
end end
...@@ -169,7 +168,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -169,7 +168,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns empty' do it 'returns empty' do
get_pipelines_index_json(username: 'invalid-username') get_pipelines_index_json(username: 'invalid-username')
check_pipeline_response(returned: 0, all: 0, running: 0, pending: 0, finished: 0) check_pipeline_response(returned: 0, all: 0)
end end
end end
end end
...@@ -181,7 +180,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -181,7 +180,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns matched pipelines' do it 'returns matched pipelines' do
get_pipelines_index_json(ref: 'branch-1') get_pipelines_index_json(ref: 'branch-1')
check_pipeline_response(returned: 1, all: 1, running: 1, pending: 0, finished: 0) check_pipeline_response(returned: 1, all: 1)
end end
end end
...@@ -189,7 +188,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -189,7 +188,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns empty list' do it 'returns empty list' do
get_pipelines_index_json(ref: 'invalid-ref') get_pipelines_index_json(ref: 'invalid-ref')
check_pipeline_response(returned: 0, all: 0, running: 0, pending: 0, finished: 0) check_pipeline_response(returned: 0, all: 0)
end end
end end
end end
...@@ -199,15 +198,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -199,15 +198,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns matched pipelines' do it 'returns matched pipelines' do
get_pipelines_index_json(status: 'success') get_pipelines_index_json(status: 'success')
check_pipeline_response(returned: 1, all: 1, running: 0, pending: 0, finished: 1) check_pipeline_response(returned: 1, all: 1)
end
context 'when filter by unrelated scope' do
it 'returns empty list' do
get_pipelines_index_json(status: 'success', scope: 'running')
check_pipeline_response(returned: 0, all: 1, running: 0, pending: 0, finished: 1)
end
end end
end end
...@@ -215,7 +206,7 @@ RSpec.describe Projects::PipelinesController do ...@@ -215,7 +206,7 @@ RSpec.describe Projects::PipelinesController do
it 'returns empty list' do it 'returns empty list' do
get_pipelines_index_json(status: 'manual') get_pipelines_index_json(status: 'manual')
check_pipeline_response(returned: 0, all: 0, running: 0, pending: 0, finished: 0) check_pipeline_response(returned: 0, all: 0)
end end
end end
...@@ -223,11 +214,19 @@ RSpec.describe Projects::PipelinesController do ...@@ -223,11 +214,19 @@ RSpec.describe Projects::PipelinesController do
it 'returns all list' do it 'returns all list' do
get_pipelines_index_json(status: 'invalid-status') get_pipelines_index_json(status: 'invalid-status')
check_pipeline_response(returned: 6, all: 6, running: 2, pending: 1, finished: 3) check_pipeline_response(returned: 6, all: 6)
end end
end end
end end
def get_pipelines_index_html(params = {})
get :index, params: {
namespace_id: project.namespace,
project_id: project
}.merge(params),
format: :html
end
def get_pipelines_index_json(params = {}) def get_pipelines_index_json(params = {})
get :index, params: { get :index, params: {
namespace_id: project.namespace, namespace_id: project.namespace,
...@@ -284,15 +283,12 @@ RSpec.describe Projects::PipelinesController do ...@@ -284,15 +283,12 @@ RSpec.describe Projects::PipelinesController do
) )
end end
def check_pipeline_response(returned:, all:, running:, pending:, finished:) def check_pipeline_response(returned:, all:)
aggregate_failures do aggregate_failures do
expect(response).to match_response_schema('pipeline') expect(response).to match_response_schema('pipeline')
expect(json_response['pipelines'].count).to eq returned expect(json_response['pipelines'].count).to eq returned
expect(json_response['count']['all'].to_i).to eq all expect(json_response['count']['all'].to_i).to eq all
expect(json_response['count']['running'].to_i).to eq running
expect(json_response['count']['pending'].to_i).to eq pending
expect(json_response['count']['finished'].to_i).to eq finished
end end
end end
end 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