Merge branch 'ci-lint-view-error-messages' into 'master'

Warning messages for pipeline configurations

See merge request gitlab-org/gitlab!38734

app/helpers/ci/pipelines_helper.rb

+# frozen_string_literal: true
+
+module Ci
+  module PipelinesHelper
+    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
+      end
+    end
+  end
+end

app/views/projects/ci/lints/_create.html.haml

@@ -4,6 +4,8 @@
       %b= _("Status:")
       = _("syntax is correct")
 
+  = render "projects/ci/lints/lint_warnings", warnings: @warnings
+
   .table-holder
     %table.table.table-bordered
       %thead
@@ -47,3 +49,5 @@
   %pre
     - @errors.each do |message|
       %p= message
+
+  = render "projects/ci/lints/lint_warnings", warnings: @warnings

app/views/projects/ci/lints/_lint_warnings.html.haml

+- if warnings
+  - warnings.each do |warning|
+    .bs-callout.bs-callout-warning
+      %p
+        %b= _("Warning:")
+        = markdown(warning)

app/views/projects/pipelines/_pipeline_warnings.html.haml

+- if warnings.any?
+  - warnings.map(&:content).each do |warning|
+    .bs-callout.bs-callout-warning
+      %p
+        %b= _("Warning:")
+        = markdown(warning)

app/views/projects/pipelines/_with_tabs.html.haml

+- return if pipeline_has_errors
 - test_reports_enabled = Feature.enabled?(:junit_pipeline_view, @project)
 - dag_pipeline_tab_enabled = Feature.enabled?(:dag_pipeline_tab, @project, default_enabled: true)
 

app/views/projects/pipelines/new.html.haml

@@ -12,6 +12,7 @@
 - else
   = form_for @pipeline, as: :pipeline, url: project_pipelines_path(@project), html: { id: "new-pipeline-form", class: "js-new-pipeline-form js-requires-input" } do |f|
     = form_errors(@pipeline)
+    = pipeline_warnings(@pipeline)
     .form-group.row
       .col-sm-12
         = f.label :ref, s_('Pipeline|Run for'), class: 'col-form-label'

app/views/projects/pipelines/show.html.haml

 - add_to_breadcrumbs _('Pipelines'), project_pipelines_path(@project)
 - breadcrumb_title "##{@pipeline.id}"
 - page_title _('Pipeline')
+- pipeline_has_errors = @pipeline.builds.empty? && @pipeline.yaml_errors.present?
 
 .js-pipeline-container{ data: { controller_action: "#{controller.action_name}" } }
   #js-pipeline-header-vue.pipeline-header-container
@@ -8,7 +9,7 @@
   - if @pipeline.commit.present?
     = render "projects/pipelines/info", commit: @pipeline.commit
 
-  - if @pipeline.builds.empty? && @pipeline.yaml_errors.present?
+  - if pipeline_has_errors
     .bs-callout.bs-callout-danger
       %h4= _('Found errors in your %{gitlab_ci_yml}:') % { gitlab_ci_yml: '.gitlab-ci.yml' }
       %ul
@@ -17,7 +18,8 @@
       - lint_link_url = project_ci_lint_path(@project)
       - 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 }
-  - else
-    = render "projects/pipelines/with_tabs", pipeline: @pipeline
+
+  = render "projects/pipelines/pipeline_warnings", warnings: @pipeline.warning_messages
+  = 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) } }

changelogs/unreleased/ci-lint-view-error-messages.yml

+---
+title: UI warning messages for pipeline configurations
+merge_request: 38734
+author:
+type: added

spec/helpers/ci/pipelines_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::PipelinesHelper do
+  include Devise::Test::ControllerHelpers
+
+  describe 'pipeline_warnings' do
+    let(:pipeline) { double(:pipeline, warning_messages: warning_messages) }
+
+    subject { helper.pipeline_warnings(pipeline) }
+
+    context 'when pipeline has no warnings' do
+      let(:warning_messages) { [] }
+
+      it 'is empty' do
+        expect(subject).to be_nil
+      end
+    end
+
+    context 'when pipeline has warnings' 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:'
+      end
+
+      it 'lists the the warnings' do
+        expect(subject).to include 'Warning 1'
+        expect(subject).to include 'Warning 2'
+      end
+    end
+  end
+end

spec/views/projects/ci/lints/show.html.haml_spec.rb

@@ -82,6 +82,20 @@ RSpec.describe 'projects/ci/lints/show' do
       expect(rendered).to have_content('Environment: testing')
       expect(rendered).to have_content('When: on_success')
     end
+
+    context 'when content has warnings' do
+      before do
+        assign(:warnings, ['Warning 1', 'Warning 2'])
+      end
+
+      it 'shows warning messages' do
+        render
+
+        expect(rendered).to have_content('Warning:')
+        expect(rendered).to have_content('Warning 1')
+        expect(rendered).to have_content('Warning 2')
+      end
+    end
   end
 
   context 'when the content is invalid' do
@@ -89,6 +103,7 @@ RSpec.describe 'projects/ci/lints/show' do
       assign(:project, project)
       assign(:status, false)
       assign(:errors, ['Undefined error'])
+      assign(:warnings, ['Warning 1', 'Warning 2'])
     end
 
     it 'shows error message' do
@@ -98,5 +113,13 @@ RSpec.describe 'projects/ci/lints/show' do
       expect(rendered).to have_content('Undefined error')
       expect(rendered).not_to have_content('Tag list:')
     end
+
+    it 'shows warning messages' do
+      render
+
+      expect(rendered).to have_content('Warning:')
+      expect(rendered).to have_content('Warning 1')
+      expect(rendered).to have_content('Warning 2')
+    end
   end
 end

spec/views/projects/pipelines/new.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'projects/pipelines/new' do
+  include Devise::Test::ControllerHelpers
+  let_it_be(:project) { create(:project, :repository) }
+  let(:pipeline) { create(:ci_pipeline, project: project) }
+
+  before do
+    assign(:project, project)
+    assign(:pipeline, pipeline)
+
+    stub_feature_flags(new_pipeline_form: false)
+  end
+
+  describe 'warning messages' do
+    let(:warning_messages) do
+      [double(content: 'warning 1'), double(content: 'warning 2')]
+    end
+
+    before do
+      allow(pipeline).to receive(:warning_messages).and_return(warning_messages)
+    end
+
+    it 'displays the warnings' do
+      render
+
+      expect(rendered).to have_css('div.alert-warning')
+      expect(rendered).to have_content('warning 1')
+      expect(rendered).to have_content('warning 2')
+    end
+  end
+end

spec/views/projects/pipelines/show.html.haml_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'projects/pipelines/show' do
+  include Devise::Test::ControllerHelpers
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+  let(:pipeline) { create(:ci_pipeline, project: project) }
+  let(:presented_pipeline) { pipeline.present(current_user: user) }
+
+  before do
+    assign(:project, project)
+    assign(:pipeline, presented_pipeline)
+
+    stub_feature_flags(new_pipeline_form: false)
+  end
+
+  shared_examples 'pipeline with warning messages' do
+    let(:warning_messages) do
+      [double(content: 'warning 1'), double(content: 'warning 2')]
+    end
+
+    before do
+      allow(pipeline).to receive(:warning_messages).and_return(warning_messages)
+    end
+
+    it 'displays the warnings' do
+      render
+
+      expect(rendered).to have_css('.bs-callout-warning')
+      expect(rendered).to have_content('warning 1')
+      expect(rendered).to have_content('warning 2')
+    end
+  end
+
+  context 'when pipeline has errors' do
+    before do
+      allow(pipeline).to receive(:yaml_errors).and_return('some errors')
+    end
+
+    it 'shows errors' do
+      render
+
+      expect(rendered).to have_content('Found errors in your .gitlab-ci.yml')
+      expect(rendered).to have_content('some errors')
+    end
+
+    it 'does not render the pipeline tabs' do
+      render
+
+      expect(rendered).not_to have_css('ul.pipelines-tabs')
+    end
+
+    context 'when pipeline has also warnings' do
+      it_behaves_like 'pipeline with warning messages'
+    end
+  end
+
+  context 'when pipeline is valid' do
+    it 'does not show errors' do
+      render
+
+      expect(rendered).not_to have_content('Found errors in your .gitlab-ci.yml')
+    end
+
+    it 'renders the pipeline tabs' do
+      render
+
+      expect(rendered).to have_css('ul.pipelines-tabs')
+    end
+
+    context 'when pipeline has warnings' do
+      it_behaves_like 'pipeline with warning messages'
+    end
+  end
+end