Merge branch 'move-pipeline-warnings-to-vue' into 'master'

Move pipeline lint warnings to Vue

See merge request gitlab-org/gitlab!41819

app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue

@@ -14,7 +14,7 @@ import {
   GlSearchBoxByType,
   GlSprintf,
 } from '@gitlab/ui';
-import { s__, __ } from '~/locale';
+import { s__, __, n__ } from '~/locale';
 import axios from '~/lib/utils/axios_utils';
 import { redirectTo } from '~/lib/utils/url_utility';
 import { VARIABLE_TYPE, FILE_TYPE } from '../constants';
@@ -29,6 +29,8 @@ export default {
   ),
   formElementClasses: 'gl-mr-3 gl-mb-3 table-section section-15',
   errorTitle: __('The form contains the following error:'),
+  warningTitle: __('The form contains the following warning:'),
+  maxWarningsSummary: __('%{total} warnings found: showing first %{warningsDisplayed}'),
   components: {
     GlAlert,
     GlButton,
@@ -74,13 +76,20 @@ export default {
       required: false,
       default: () => ({}),
     },
+    maxWarnings: {
+      type: Number,
+      required: true,
+    },
   },
   data() {
     return {
       searchTerm: '',
       refValue: this.refParam,
       variables: {},
-      error: false,
+      error: null,
+      warnings: [],
+      totalWarnings: 0,
+      isWarningDismissed: false,
     };
   },
   computed: {
@@ -91,6 +100,18 @@ export default {
     variablesLength() {
       return Object.keys(this.variables).length;
     },
+    overMaxWarningsLimit() {
+      return this.totalWarnings > this.maxWarnings;
+    },
+    warningsSummary() {
+      return n__('%d warning found:', '%d warnings found:', this.warnings.length);
+    },
+    summaryMessage() {
+      return this.overMaxWarningsLimit ? this.$options.maxWarningsSummary : this.warningsSummary;
+    },
+    shouldShowWarning() {
+      return this.warnings.length > 0 && !this.isWarningDismissed;
+    },
   },
   created() {
     if (this.variableParams) {
@@ -154,8 +175,11 @@ export default {
           redirectTo(`${this.pipelinesPath}/${data.id}`);
         })
         .catch(err => {
-          const [message] = err.response.data.base;
-          this.error = message;
+          const { errors, warnings, total_warnings: totalWarnings } = err.response.data;
+          const [error] = errors;
+          this.error = error;
+          this.warnings = warnings;
+          this.totalWarnings = totalWarnings;
         });
     },
   },
@@ -170,8 +194,37 @@ export default {
       :dismissible="false"
       variant="danger"
       class="gl-mb-4"
+      data-testid="run-pipeline-error-alert"
       >{{ error }}</gl-alert
     >
+    <gl-alert
+      v-if="shouldShowWarning"
+      :title="$options.warningTitle"
+      variant="warning"
+      class="gl-mb-4"
+      data-testid="run-pipeline-warning-alert"
+      @dismiss="isWarningDismissed = true"
+    >
+      <details>
+        <summary>
+          <gl-sprintf :message="summaryMessage">
+            <template #total>
+              {{ totalWarnings }}
+            </template>
+            <template #warningsDisplayed>
+              {{ maxWarnings }}
+            </template>
+          </gl-sprintf>
+        </summary>
+        <p
+          v-for="(warning, index) in warnings"
+          :key="`warning-${index}`"
+          data-testid="run-pipeline-warning"
+        >
+          {{ warning }}
+        </p>
+      </details>
+    </gl-alert>
     <gl-form-group :label="s__('Pipeline|Run for')">
       <gl-dropdown :text="refValue" block>
         <gl-search-box-by-type

app/assets/javascripts/pipeline_new/index.js

@@ -11,6 +11,7 @@ export default () => {
     fileParam,
     refNames,
     settingsLink,
+    maxWarnings,
   } = el?.dataset;
 
   const variableParams = JSON.parse(varParam);
@@ -29,6 +30,7 @@ export default () => {
           fileParams,
           refs,
           settingsLink,
+          maxWarnings: Number(maxWarnings),
         },
       });
     },

app/controllers/projects/pipelines_controller.rb

@@ -78,7 +78,10 @@ class Projects::PipelinesController < Projects::ApplicationController
                          .represent(@pipeline),
                  status: :created
         else
-          render json: @pipeline.errors, status: :bad_request
+          render json: { errors: @pipeline.error_messages.map(&:content),
+                         warnings: @pipeline.warning_messages(limit: ::Gitlab::Ci::Warnings::MAX_LIMIT).map(&:content),
+                         total_warnings: @pipeline.warning_messages.length },
+                 status: :bad_request
         end
       end
     end

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

@@ -7,7 +7,7 @@
 %hr
 
 - if Feature.enabled?(:new_pipeline_form)
-  #js-new-pipeline{ data: { project_id: @project.id, pipelines_path: project_pipelines_path(@project), ref_param: params[:ref] || @project.default_branch, var_param: params[:var].to_json, file_param: params[:file_var].to_json, ref_names: @project.repository.ref_names.to_json.html_safe, settings_link: project_settings_ci_cd_path(@project) } }
+  #js-new-pipeline{ data: { project_id: @project.id, pipelines_path: project_pipelines_path(@project), ref_param: params[:ref] || @project.default_branch, var_param: params[:var].to_json, file_param: params[:file_var].to_json, ref_names: @project.repository.ref_names.to_json.html_safe, settings_link: project_settings_ci_cd_path(@project), max_warnings: ::Gitlab::Ci::Warnings::MAX_LIMIT } }
 
 - 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|

locale/gitlab.pot

@@ -305,6 +305,11 @@ msgid_plural "%d vulnerabilities dismissed"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "%d warning found:"
+msgid_plural "%d warnings found:"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "%s additional commit has been omitted to prevent performance issues."
 msgid_plural "%s additional commits have been omitted to prevent performance issues."
 msgstr[0] ""
@@ -819,6 +824,9 @@ msgstr ""
 msgid "%{total} open issue weight"
 msgstr ""
 
+msgid "%{total} warnings found: showing first %{warningsDisplayed}"
+msgstr ""
+
 msgid "%{usage_ping_link_start}Learn more%{usage_ping_link_end} about what information is shared with GitLab Inc."
 msgstr ""
 
@@ -25064,6 +25072,9 @@ msgstr ""
 msgid "The form contains the following error:"
 msgstr ""
 
+msgid "The form contains the following warning:"
+msgstr ""
+
 msgid "The global settings require you to enable Two-Factor Authentication for your account."
 msgstr ""
 

spec/controllers/projects/pipelines_controller_spec.rb

@@ -801,6 +801,11 @@ RSpec.describe Projects::PipelinesController do
     context 'with an invalid .gitlab-ci.yml file' do
       before do
         stub_ci_pipeline_yaml_file(YAML.dump({
+          build: {
+            stage: 'build',
+            script: 'echo',
+            rules: [{ when: 'always' }]
+          },
           test: {
             stage: 'invalid',
             script: 'echo'
@@ -812,9 +817,13 @@ RSpec.describe Projects::PipelinesController do
         expect { subject }.not_to change { project.ci_pipelines.count }
 
         expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response['base']).to include(
+        expect(json_response['errors']).to eq([
           'test job: chosen stage does not exist; available stages are .pre, build, test, deploy, .post'
+        ])
+        expect(json_response['warnings'][0]).to include(
+          'jobs:build may allow multiple pipelines to run for a single action due to `rules:when`'
         )
+        expect(json_response['total_warnings']).to eq(1)
       end
     end
   end

spec/frontend/pipeline_new/components/pipeline_new_form_spec.js

 import { mount, shallowMount } from '@vue/test-utils';
-import { GlDropdown, GlDropdownItem, GlForm } from '@gitlab/ui';
+import { GlDropdown, GlDropdownItem, GlForm, GlSprintf } from '@gitlab/ui';
 import MockAdapter from 'axios-mock-adapter';
 import waitForPromises from 'helpers/wait_for_promises';
 import axios from '~/lib/utils/axios_utils';
 import PipelineNewForm from '~/pipeline_new/components/pipeline_new_form.vue';
-import { mockRefs, mockParams, mockPostParams, mockProjectId } from '../mock_data';
+import { mockRefs, mockParams, mockPostParams, mockProjectId, mockError } from '../mock_data';
 import { redirectTo } from '~/lib/utils/url_utility';
 
 jest.mock('~/lib/utils/url_utility', () => ({
@@ -28,6 +28,10 @@ describe('Pipeline New Form', () => {
   const findVariableRows = () => wrapper.findAll('[data-testid="ci-variable-row"]');
   const findRemoveIcons = () => wrapper.findAll('[data-testid="remove-ci-variable-row"]');
   const findKeyInputs = () => wrapper.findAll('[data-testid="pipeline-form-ci-variable-key"]');
+  const findErrorAlert = () => wrapper.find('[data-testid="run-pipeline-error-alert"]');
+  const findWarningAlert = () => wrapper.find('[data-testid="run-pipeline-warning-alert"]');
+  const findWarningAlertSummary = () => findWarningAlert().find(GlSprintf);
+  const findWarnings = () => wrapper.findAll('[data-testid="run-pipeline-warning"]');
   const getExpectedPostParams = () => JSON.parse(mock.history.post[0].data);
 
   const createComponent = (term = '', props = {}, method = shallowMount) => {
@@ -38,6 +42,7 @@ describe('Pipeline New Form', () => {
         refs: mockRefs,
         defaultBranch: 'master',
         settingsLink: '',
+        maxWarnings: 25,
         ...props,
       },
       data() {
@@ -50,8 +55,6 @@ describe('Pipeline New Form', () => {
 
   beforeEach(() => {
     mock = new MockAdapter(axios);
-
-    mock.onPost(pipelinesPath).reply(200, postResponse);
   });
 
   afterEach(() => {
@@ -62,6 +65,10 @@ describe('Pipeline New Form', () => {
   });
 
   describe('Dropdown with branches and tags', () => {
+    beforeEach(() => {
+      mock.onPost(pipelinesPath).reply(200, postResponse);
+    });
+
     it('displays dropdown with all branches and tags', () => {
       createComponent();
       expect(findDropdownItems()).toHaveLength(mockRefs.length);
@@ -82,6 +89,8 @@ describe('Pipeline New Form', () => {
   describe('Form', () => {
     beforeEach(() => {
       createComponent('', mockParams, mount);
+
+      mock.onPost(pipelinesPath).reply(200, postResponse);
     });
     it('displays the correct values for the provided query params', async () => {
       expect(findDropdown().props('text')).toBe('tag-1');
@@ -124,4 +133,34 @@ describe('Pipeline New Form', () => {
       expect(findVariableRows()).toHaveLength(4);
     });
   });
+
+  describe('Form errors and warnings', () => {
+    beforeEach(() => {
+      createComponent();
+
+      mock.onPost(pipelinesPath).reply(400, mockError);
+
+      findForm().vm.$emit('submit', dummySubmitEvent);
+
+      return waitForPromises();
+    });
+
+    it('shows both error and warning', () => {
+      expect(findErrorAlert().exists()).toBe(true);
+      expect(findWarningAlert().exists()).toBe(true);
+    });
+
+    it('shows the correct error', () => {
+      expect(findErrorAlert().text()).toBe(mockError.errors[0]);
+    });
+
+    it('shows the correct warning title', () => {
+      const { length } = mockError.warnings;
+      expect(findWarningAlertSummary().attributes('message')).toBe(`${length} warnings found:`);
+    });
+
+    it('shows the correct amount of warnings', () => {
+      expect(findWarnings()).toHaveLength(mockError.warnings.length);
+    });
+  });
 });

spec/frontend/pipeline_new/mock_data.js

@@ -19,3 +19,15 @@ export const mockPostParams = {
     { key: 'test_file', value: 'test_file_val', variable_type: 'file' },
   ],
 };
+
+export const mockError = {
+  errors: [
+    'test job: chosen stage does not exist; available stages are .pre, build, test, deploy, .post',
+  ],
+  warnings: [
+    'jobs:build1 may allow multiple pipelines to run for a single action due to `rules:when` clause with no `workflow:rules` - read more: https://docs.gitlab.com/ee/ci/troubleshooting.html#pipeline-warnings',
+    'jobs:build2 may allow multiple pipelines to run for a single action due to `rules:when` clause with no `workflow:rules` - read more: https://docs.gitlab.com/ee/ci/troubleshooting.html#pipeline-warnings',
+    'jobs:build3 may allow multiple pipelines to run for a single action due to `rules:when` clause with no `workflow:rules` - read more: https://docs.gitlab.com/ee/ci/troubleshooting.html#pipeline-warnings',
+  ],
+  total_warnings: 7,
+};