Merge branch 'mrincon-improve-error-messages-missing-file' into 'master'

Improve error messages when CI blob is not loaded

See merge request gitlab-org/gitlab!51584

app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue

@@ -3,6 +3,7 @@ import { GlAlert, GlLoadingIcon, GlTabs, GlTab } from '@gitlab/ui';
 import { __, s__, sprintf } from '~/locale';
 import { mergeUrlParams, redirectTo } from '~/lib/utils/url_utility';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import httpStatusCodes from '~/lib/utils/http_status';
 
 import PipelineGraph from '~/pipelines/components/pipeline_graph/pipeline_graph.vue';
 import CiLint from './components/lint/ci_lint.vue';
@@ -23,7 +24,6 @@ const COMMIT_FAILURE = 'COMMIT_FAILURE';
 const COMMIT_SUCCESS = 'COMMIT_SUCCESS';
 const DEFAULT_FAILURE = 'DEFAULT_FAILURE';
 const LOAD_FAILURE_NO_FILE = 'LOAD_FAILURE_NO_FILE';
-const LOAD_FAILURE_NO_REF = 'LOAD_FAILURE_NO_REF';
 const LOAD_FAILURE_UNKNOWN = 'LOAD_FAILURE_UNKNOWN';
 
 export default {
@@ -125,6 +125,9 @@ export default {
     isBlobContentLoading() {
       return this.$apollo.queries.content.loading;
     },
+    isBlobContentError() {
+      return this.failureType === LOAD_FAILURE_NO_FILE || this.failureType === LOAD_FAILURE_UNKNOWN;
+    },
     isCiConfigDataLoading() {
       return this.$apollo.queries.ciConfigData.loading;
     },
@@ -144,14 +147,11 @@ export default {
     },
     failure() {
       switch (this.failureType) {
-        case LOAD_FAILURE_NO_REF:
-          return {
-            text: this.$options.alertTexts[LOAD_FAILURE_NO_REF],
-            variant: 'danger',
-          };
         case LOAD_FAILURE_NO_FILE:
           return {
-            text: this.$options.alertTexts[LOAD_FAILURE_NO_FILE],
+            text: sprintf(this.$options.alertTexts[LOAD_FAILURE_NO_FILE], {
+              filePath: this.ciConfigPath,
+            }),
             variant: 'danger',
           };
         case LOAD_FAILURE_UNKNOWN:
@@ -182,9 +182,8 @@ export default {
     [COMMIT_FAILURE]: s__('Pipelines|The GitLab CI configuration could not be updated.'),
     [COMMIT_SUCCESS]: __('Your changes have been successfully committed.'),
     [DEFAULT_FAILURE]: __('Something went wrong on our end.'),
-    [LOAD_FAILURE_NO_FILE]: s__('Pipelines|No CI file found in this repository, please add one.'),
-    [LOAD_FAILURE_NO_REF]: s__(
-      'Pipelines|Repository does not have a default branch, please set one.',
+    [LOAD_FAILURE_NO_FILE]: s__(
+      'Pipelines|There is no %{filePath} file in this repository, please add one and visit the Pipeline Editor again.',
     ),
     [LOAD_FAILURE_UNKNOWN]: s__('Pipelines|The CI configuration was not loaded, please try again.'),
   },
@@ -193,12 +192,13 @@ export default {
       const { networkError } = error;
 
       const { response } = networkError;
-      if (response?.status === 404) {
-        // 404 for missing CI file
+      // 404 for missing CI file
+      // 400 for blank projects with no repository
+      if (
+        response?.status === httpStatusCodes.NOT_FOUND ||
+        response?.status === httpStatusCodes.BAD_REQUEST
+      ) {
         this.reportFailure(LOAD_FAILURE_NO_FILE);
-      } else if (response?.status === 400) {
-        // 400 for a missing ref when no default branch is set
-        this.reportFailure(LOAD_FAILURE_NO_REF);
       } else {
         this.reportFailure(LOAD_FAILURE_UNKNOWN);
       }
@@ -299,9 +299,9 @@ export default {
         <li v-for="reason in failureReasons" :key="reason">{{ reason }}</li>
       </ul>
     </gl-alert>
-    <div class="gl-mt-4">
-      <gl-loading-icon v-if="isBlobContentLoading" size="lg" class="gl-m-3" />
-      <div v-else class="file-editor gl-mb-3">
+    <gl-loading-icon v-if="isBlobContentLoading" size="lg" class="gl-m-3" />
+    <div v-else-if="!isBlobContentError" class="gl-mt-4">
+      <div class="file-editor gl-mb-3">
         <div class="info-well gl-display-none gl-display-sm-block">
           <validation-segment
             class="well-segment"

locale/gitlab.pot

@@ -20790,9 +20790,6 @@ msgstr ""
 msgid "Pipelines|More Information"
 msgstr ""
 
-msgid "Pipelines|No CI file found in this repository, please add one."
-msgstr ""
-
 msgid "Pipelines|No triggers have been created yet. Add one using the form above."
 msgstr ""
 
@@ -20805,9 +20802,6 @@ msgstr ""
 msgid "Pipelines|Project cache successfully reset."
 msgstr ""
 
-msgid "Pipelines|Repository does not have a default branch, please set one."
-msgstr ""
-
 msgid "Pipelines|Revoke"
 msgstr ""
 
@@ -20829,6 +20823,9 @@ msgstr ""
 msgid "Pipelines|There are currently no pipelines."
 msgstr ""
 
+msgid "Pipelines|There is no %{filePath} file in this repository, please add one and visit the Pipeline Editor again."
+msgstr ""
+
 msgid "Pipelines|There was an error fetching the pipelines. Try again in a few moments or contact your support team."
 msgstr ""
 

spec/frontend/pipeline_editor/pipeline_editor_app_spec.js

@@ -5,6 +5,7 @@ import waitForPromises from 'helpers/wait_for_promises';
 import VueApollo from 'vue-apollo';
 import createMockApollo from 'jest/helpers/mock_apollo_helper';
 
+import httpStatusCodes from '~/lib/utils/http_status';
 import { objectToQuery, redirectTo, refreshCurrentPage } from '~/lib/utils/url_utility';
 import {
   mockCiConfigPath,
@@ -414,58 +415,81 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
       mockCiConfigData.mockResolvedValue(mockCiConfigQueryResponse);
     });
 
-    it('no error is shown when data is set', async () => {
-      createComponentWithApollo();
+    describe('when file exists', () => {
+      beforeEach(async () => {
+        createComponentWithApollo();
 
-      await waitForPromises();
+        await waitForPromises();
+      });
 
-      expect(findAlert().exists()).toBe(false);
-      expect(findEditorLite().attributes('value')).toBe(mockCiYml);
-    });
+      it('shows editor and commit form', () => {
+        expect(findEditorLite().exists()).toBe(true);
+        expect(findTextEditor().exists()).toBe(true);
+      });
 
-    it('ci config query is called with correct variables', async () => {
-      createComponentWithApollo();
+      it('no error is shown when data is set', async () => {
+        expect(findAlert().exists()).toBe(false);
+        expect(findEditorLite().attributes('value')).toBe(mockCiYml);
+      });
 
-      await waitForPromises();
+      it('ci config query is called with correct variables', async () => {
+        createComponentWithApollo();
 
-      expect(mockCiConfigData).toHaveBeenCalledWith({
-        content: mockCiYml,
-        projectPath: mockProjectFullPath,
+        await waitForPromises();
+
+        expect(mockCiConfigData).toHaveBeenCalledWith({
+          content: mockCiYml,
+          projectPath: mockProjectFullPath,
+        });
       });
     });
 
-    it('shows a 404 error message', async () => {
-      mockBlobContentData.mockRejectedValueOnce({
-        response: {
-          status: 404,
-        },
+    describe('when no file exists', () => {
+      const expectedAlertMsg =
+        'There is no .gitlab-ci.yml file in this repository, please add one and visit the Pipeline Editor again.';
+
+      it('does not show editor or commit form', async () => {
+        mockBlobContentData.mockRejectedValueOnce(new Error('My error!'));
+        createComponentWithApollo();
+        await waitForPromises();
+
+        expect(findEditorLite().exists()).toBe(false);
+        expect(findTextEditor().exists()).toBe(false);
       });
-      createComponentWithApollo();
 
-      await waitForPromises();
+      it('shows a 404 error message', async () => {
+        mockBlobContentData.mockRejectedValueOnce({
+          response: {
+            status: httpStatusCodes.NOT_FOUND,
+          },
+        });
+        createComponentWithApollo();
 
-      expect(findAlert().text()).toBe('No CI file found in this repository, please add one.');
-    });
+        await waitForPromises();
 
-    it('shows a 400 error message', async () => {
-      mockBlobContentData.mockRejectedValueOnce({
-        response: {
-          status: 400,
-        },
+        expect(findAlert().text()).toBe(expectedAlertMsg);
       });
-      createComponentWithApollo();
 
-      await waitForPromises();
+      it('shows a 400 error message', async () => {
+        mockBlobContentData.mockRejectedValueOnce({
+          response: {
+            status: httpStatusCodes.BAD_REQUEST,
+          },
+        });
+        createComponentWithApollo();
 
-      expect(findAlert().text()).toBe('Repository does not have a default branch, please set one.');
-    });
+        await waitForPromises();
 
-    it('shows a unkown error message', async () => {
-      mockBlobContentData.mockRejectedValueOnce(new Error('My error!'));
-      createComponentWithApollo();
-      await waitForPromises();
+        expect(findAlert().text()).toBe(expectedAlertMsg);
+      });
+
+      it('shows a unkown error message', async () => {
+        mockBlobContentData.mockRejectedValueOnce(new Error('My error!'));
+        createComponentWithApollo();
+        await waitForPromises();
 
-      expect(findAlert().text()).toBe('The CI configuration was not loaded, please try again.');
+        expect(findAlert().text()).toBe('The CI configuration was not loaded, please try again.');
+      });
     });
   });
 });