Merge branch...

Merge branch 'fc-fix-sending-graphql-query-with-undefined-variables-in-pipeline-editor' into 'master'

Fix sending BlobContent query with empty variables

See merge request gitlab-org/gitlab!81156

app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue

@@ -69,9 +69,10 @@ export default {
       // If it's a brand new file, we don't want to fetch the content.
       // Then when the user commits the first time, the query would run
       // to get the initial file content, but we already have it in `lastCommitedContent`
-      // so we skip the loading altogether.
-      skip({ isNewCiConfigFile, lastCommittedContent }) {
-        return isNewCiConfigFile || lastCommittedContent;
+      // so we skip the loading altogether. We also wait for the currentBranch
+      // to have been fetched
+      skip() {
+        return this.shouldSkipBlobContentQuery;
       },
       variables() {
         return {
@@ -128,8 +129,8 @@ export default {
     },
     ciConfigData: {
       query: getCiConfigData,
-      skip({ currentCiFileContent }) {
-        return !currentCiFileContent;
+      skip() {
+        return this.shouldSkipCiConfigQuery;
       },
       variables() {
         return {
@@ -174,6 +175,9 @@ export default {
     },
     commitSha: {
       query: getLatestCommitShaQuery,
+      skip({ currentBranch }) {
+        return !currentBranch;
+      },
       variables() {
         return {
           projectPath: this.projectFullPath,
@@ -181,7 +185,7 @@ export default {
         };
       },
       update(data) {
-        const latestCommitSha = data.project?.repository?.tree?.lastCommit?.sha;
+        const latestCommitSha = data?.project?.repository?.tree?.lastCommit?.sha;
 
         if (this.isFetchingCommitSha && latestCommitSha === this.commitSha) {
           this.$apollo.queries.commitSha.startPolling(COMMIT_SHA_POLL_INTERVAL);
@@ -192,6 +196,9 @@ export default {
         this.$apollo.queries.commitSha.stopPolling();
         return latestCommitSha;
       },
+      error() {
+        this.reportFailure(LOAD_FAILURE_UNKNOWN);
+      },
     },
     currentBranch: {
       query: getCurrentBranch,
@@ -234,6 +241,12 @@ export default {
     isEmpty() {
       return this.currentCiFileContent === '';
     },
+    shouldSkipBlobContentQuery() {
+      return this.isNewCiConfigFile || this.lastCommittedContent || !this.currentBranch;
+    },
+    shouldSkipCiConfigQuery() {
+      return !this.currentCiFileContent || !this.commitSha;
+    },
   },
   i18n: {
     resetModal: {

spec/frontend/pipeline_editor/pipeline_editor_app_spec.js

@@ -18,12 +18,15 @@ import {
   COMMIT_SUCCESS,
   COMMIT_SUCCESS_WITH_REDIRECT,
   COMMIT_FAILURE,
+  EDITOR_APP_STATUS_LOADING,
 } from '~/pipeline_editor/constants';
 import getBlobContent from '~/pipeline_editor/graphql/queries/blob_content.query.graphql';
 import getCiConfigData from '~/pipeline_editor/graphql/queries/ci_config.query.graphql';
 import getTemplate from '~/pipeline_editor/graphql/queries/get_starter_template.query.graphql';
 import getLatestCommitShaQuery from '~/pipeline_editor/graphql/queries/latest_commit_sha.query.graphql';
 import getPipelineQuery from '~/pipeline_editor/graphql/queries/pipeline.query.graphql';
+import getCurrentBranch from '~/pipeline_editor/graphql/queries/client/current_branch.query.graphql';
+import getAppStatus from '~/pipeline_editor/graphql/queries/client/app_status.query.graphql';
 
 import PipelineEditorApp from '~/pipeline_editor/pipeline_editor_app.vue';
 import PipelineEditorHome from '~/pipeline_editor/pipeline_editor_home.vue';
@@ -84,9 +87,6 @@ describe('Pipeline editor app component', () => {
             initialCiFileContent: {
               loading: blobLoading,
             },
-            ciConfigData: {
-              loading: false,
-            },
           },
         },
       },
@@ -94,7 +94,11 @@ describe('Pipeline editor app component', () => {
     });
   };
 
-  const createComponentWithApollo = async ({ provide = {}, stubs = {} } = {}) => {
+  const createComponentWithApollo = async ({
+    provide = {},
+    stubs = {},
+    withUndefinedBranch = false,
+  } = {}) => {
     const handlers = [
       [getBlobContent, mockBlobContentData],
       [getCiConfigData, mockCiConfigData],
@@ -105,6 +109,31 @@ describe('Pipeline editor app component', () => {
 
     mockApollo = createMockApollo(handlers, resolvers);
 
+    if (!withUndefinedBranch) {
+      mockApollo.clients.defaultClient.cache.writeQuery({
+        query: getCurrentBranch,
+        data: {
+          workBranches: {
+            __typename: 'BranchList',
+            current: {
+              __typename: 'WorkBranch',
+              name: mockDefaultBranch,
+            },
+          },
+        },
+      });
+    }
+
+    mockApollo.clients.defaultClient.cache.writeQuery({
+      query: getAppStatus,
+      data: {
+        app: {
+          __typename: 'AppData',
+          status: EDITOR_APP_STATUS_LOADING,
+        },
+      },
+    });
+
     const options = {
       localVue,
       mocks: {},
@@ -145,6 +174,55 @@ describe('Pipeline editor app component', () => {
     });
   });
 
+  describe('skipping queries', () => {
+    describe('when branchName is undefined', () => {
+      beforeEach(async () => {
+        await createComponentWithApollo({ withUndefinedBranch: true });
+      });
+
+      it('does not calls getBlobContent', () => {
+        expect(mockBlobContentData).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('when branchName is defined', () => {
+      beforeEach(async () => {
+        await createComponentWithApollo();
+      });
+
+      it('calls getBlobContent', () => {
+        expect(mockBlobContentData).toHaveBeenCalled();
+      });
+    });
+
+    describe('when commit sha is undefined', () => {
+      beforeEach(async () => {
+        mockLatestCommitShaQuery.mockResolvedValue(undefined);
+        await createComponentWithApollo();
+      });
+
+      it('calls getBlobContent', () => {
+        expect(mockBlobContentData).toHaveBeenCalled();
+      });
+
+      it('does not call ciConfigData', () => {
+        expect(mockCiConfigData).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('when commit sha is defined', () => {
+      beforeEach(async () => {
+        mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponse);
+        mockLatestCommitShaQuery.mockResolvedValue(mockCommitShaResults);
+        await createComponentWithApollo();
+      });
+
+      it('calls ciConfigData', () => {
+        expect(mockCiConfigData).toHaveBeenCalled();
+      });
+    });
+  });
+
   describe('when queries are called', () => {
     beforeEach(() => {
       mockBlobContentData.mockResolvedValue(mockBlobContentQueryResponse);