Commit 87439b5d authored by Payton Burdette's avatar Payton Burdette Committed by Gabriel Mazetto

Refactor pipeline warnings

Group pipeline warnings, make
them collapisible using summary
element. Add cap for how many
warning can be displayed
parent f91bef1c
......@@ -2,16 +2,35 @@
module Ci
module PipelinesHelper
include Gitlab::Ci::Warnings
def pipeline_warnings(pipeline)
return unless pipeline.warning_messages.any?
content_tag(:div, class: 'alert alert-warning') do
content_tag(:h4, 'Warning:') <<
content_tag(:div) do
pipeline.warning_messages.each do |warning|
concat(markdown(warning.content))
end
end
total_warnings = pipeline.warning_messages.length
message = warning_header(total_warnings)
content_tag(:div, class: 'bs-callout bs-callout-warning') do
content_tag(:details) do
concat content_tag(:summary, message, class: 'gl-mb-2')
warning_markdown(pipeline) { |markdown| concat markdown }
end
end
end
def warning_header(count)
message = _("%{total_warnings} warning(s) found:") % { total_warnings: count }
return message unless count > MAX_LIMIT
_("%{message} showing first %{warnings_displayed}") % { message: message, warnings_displayed: MAX_LIMIT }
end
private
def warning_markdown(pipeline)
pipeline.warning_messages(limit: MAX_LIMIT).each do |warning|
yield markdown(warning.content)
end
end
end
......
......@@ -671,8 +671,10 @@ module Ci
messages.select(&:error?)
end
def warning_messages
messages.select(&:warning?)
def warning_messages(limit: nil)
messages.select(&:warning?).tap do |warnings|
break warnings.take(limit) if limit
end
end
# Manually set the notes for a Ci::Pipeline
......
- if warnings
- warnings.each do |warning|
- total_warnings = warnings.length
- message = warning_header(total_warnings)
- if warnings.any?
.bs-callout.bs-callout-warning
%p
%b= _("Warning:")
= markdown(warning)
%details
%summary.gl-mb-2= message
- warnings.each do |warning|
= markdown(warning)
- total_warnings = warnings.length
- message = warning_header(total_warnings)
- if warnings.any?
- warnings.map(&:content).each do |warning|
.bs-callout.bs-callout-warning
%p
%b= _("Warning:")
.bs-callout.bs-callout-warning
%details
%summary.gl-mb-2= message
- warnings.map(&:content).each do |warning|
= markdown(warning)
......@@ -19,7 +19,7 @@
- lint_link_start = '<a href="%{url}">'.html_safe % { url: lint_link_url }
= s_('You can also test your %{gitlab_ci_yml} in %{lint_link_start}CI Lint%{lint_link_end}').html_safe % { gitlab_ci_yml: '.gitlab-ci.yml', lint_link_start: lint_link_start, lint_link_end: '</a>'.html_safe }
= render "projects/pipelines/pipeline_warnings", warnings: @pipeline.warning_messages
= render "projects/pipelines/pipeline_warnings", warnings: @pipeline.warning_messages(limit: Gitlab::Ci::Warnings::MAX_LIMIT)
= render "projects/pipelines/with_tabs", pipeline: @pipeline, pipeline_has_errors: pipeline_has_errors
.js-pipeline-details-vue{ data: { endpoint: project_pipeline_path(@project, @pipeline, format: :json) } }
---
title: Group pipeline warnings and make them collapsible
merge_request: 39634
author:
type: changed
......@@ -40,7 +40,7 @@ module Gitlab
Result.new(
jobs: dry_run_convert_to_jobs(pipeline.stages),
errors: pipeline.error_messages.map(&:content),
warnings: pipeline.warning_messages.map(&:content)
warnings: pipeline.warning_messages(limit: ::Gitlab::Ci::Warnings::MAX_LIMIT).map(&:content)
)
end
......@@ -54,7 +54,7 @@ module Gitlab
Result.new(
jobs: static_validation_convert_to_jobs(result.config&.stages, result.config&.builds),
errors: result.errors,
warnings: result.warnings
warnings: result.warnings.take(::Gitlab::Ci::Warnings::MAX_LIMIT) # rubocop: disable CodeReuse/ActiveRecord
)
end
......
# frozen_string_literal: true
module Gitlab::Ci::Warnings
MAX_LIMIT = 25
end
......@@ -548,6 +548,9 @@ msgstr ""
msgid "%{mergeLength}/%{usersLength} can merge"
msgstr ""
msgid "%{message} showing first %{warnings_displayed}"
msgstr ""
msgid "%{milestone_name} (Past due)"
msgstr ""
......@@ -791,6 +794,9 @@ msgstr ""
msgid "%{totalWeight} total weight"
msgstr ""
msgid "%{total_warnings} warning(s) found:"
msgstr ""
msgid "%{total} open issue weight"
msgstr ""
......
......@@ -22,8 +22,8 @@ RSpec.describe Ci::PipelinesHelper do
let(:warning_messages) { [double(content: 'Warning 1'), double(content: 'Warning 2')] }
it 'returns a warning callout box' do
expect(subject).to have_css 'div.alert-warning'
expect(subject).to include 'Warning:'
expect(subject).to have_css 'div.bs-callout-warning'
expect(subject).to include '2 warning(s) found:'
end
it 'lists the the warnings' do
......@@ -32,4 +32,24 @@ RSpec.describe Ci::PipelinesHelper do
end
end
end
describe 'warning_header' do
subject { helper.warning_header(count) }
context 'when warnings are more than max cap' do
let(:count) { 30 }
it 'returns 30 warning(s) found: showing first 25' do
expect(subject).to eq('30 warning(s) found: showing first 25')
end
end
context 'when warnings are less than max cap' do
let(:count) { 15 }
it 'returns 15 warning(s) found' do
expect(subject).to eq('15 warning(s) found:')
end
end
end
end
......@@ -92,6 +92,31 @@ RSpec.describe Gitlab::Ci::Lint do
end
end
context 'when content has more warnings than max limit' do
# content will result in 2 warnings
let(:content) do
<<~YAML
rspec:
script: rspec
rules:
- when: always
rspec2:
script: rspec
rules:
- when: always
YAML
end
before do
stub_const('Gitlab::Ci::Warnings::MAX_LIMIT', 1)
end
it 'returns a result with warnings' do
expect(subject).to be_valid
expect(subject.warnings.size).to eq(1)
end
end
context 'when content has errors and warnings' do
let(:content) do
<<~YAML
......
......@@ -87,7 +87,7 @@ RSpec.describe 'projects/ci/lints/show' do
it 'shows warning messages' do
render
expect(rendered).to have_content('Warning:')
expect(rendered).to have_content('2 warning(s) found:')
expect(rendered).to have_content('Warning 1')
expect(rendered).to have_content('Warning 2')
end
......@@ -116,7 +116,7 @@ RSpec.describe 'projects/ci/lints/show' do
it 'shows warning messages' do
render
expect(rendered).to have_content('Warning:')
expect(rendered).to have_content('2 warning(s) found:')
expect(rendered).to have_content('Warning 1')
expect(rendered).to have_content('Warning 2')
end
......
......@@ -26,7 +26,7 @@ RSpec.describe 'projects/pipelines/new' do
it 'displays the warnings' do
render
expect(rendered).to have_css('div.alert-warning')
expect(rendered).to have_css('div.bs-callout-warning')
expect(rendered).to have_content('warning 1')
expect(rendered).to have_content('warning 2')
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