From 8367298fe93059886b3702833bba46f136dfa518 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= <fcaplette@gitlab.com>
Date: Wed, 7 Sep 2022 13:31:34 -0400
Subject: [PATCH 1/3] Add batch loading to CI/CD variables

In the context of the CI/CD refactor, we did not
take into account that graphQL queries limit is 100
ms. Therefore, this commit adds batch loading
to the list of variables for project and group. Instance
variables have a hard limit at 25, but can be changed
so there is also batch loading there.
---
 .../components/ci_admin_variables.vue         |  49 +++++++-
 .../components/ci_group_variables.vue         |  54 ++++++++-
 .../components/ci_project_variables.vue       |  68 +++++++++--
 .../components/ci_variable_table.vue          |   3 +-
 .../javascripts/ci_variable_list/constants.js |   2 +-
 .../queries/group_variables.query.graphql     |   8 +-
 .../queries/project_variables.query.graphql   |   8 +-
 .../graphql/queries/variables.query.graphql   |   8 +-
 .../graphql/{resolvers.js => settings.js}     | 113 ++++++++++++++++--
 .../javascripts/ci_variable_list/index.js     |   4 +-
 .../javascripts/ci_variable_list/utils.js     |  10 ++
 locale/gitlab.pot                             |   3 +
 .../components/ci_admin_variables_spec.js     |   2 +-
 .../components/ci_group_variables_spec.js     |   2 +-
 .../components/ci_project_variables_spec.js   |   2 +-
 spec/frontend/ci_variable_list/mocks.js       |   6 +
 16 files changed, 308 insertions(+), 34 deletions(-)
 rename app/assets/javascripts/ci_variable_list/graphql/{resolvers.js => settings.js} (66%)

diff --git a/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
index 59ddf4b19d8f46..06688bdaa7bdd1 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
@@ -1,5 +1,7 @@
 <script>
 import createFlash from '~/flash';
+import { __ } from '~/locale';
+import { reportMessageToSentry } from '../utils';
 import getAdminVariables from '../graphql/queries/variables.query.graphql';
 import {
   ADD_MUTATION_ACTION,
@@ -21,7 +23,11 @@ export default {
   data() {
     return {
       adminVariables: [],
+      hasNextPage: false,
       isInitialLoading: true,
+      isLoadingMoreItems: false,
+      loadingCounter: 0,
+      pageInfo: {},
     };
   },
   apollo: {
@@ -30,7 +36,16 @@ export default {
       update(data) {
         return data?.ciVariables?.nodes || [];
       },
+      result({ data }) {
+        this.pageInfo = data?.ciVariables?.pageInfo || this.pageInfo;
+        this.hasNextPage = this.pageInfo?.hasNextPage || false;
+        if (!this.hasNextPage) {
+          this.isLoadingMoreItems = false;
+        }
+      },
       error() {
+        this.isLoadingMoreItems = false;
+        this.hasNextPage = false;
         createFlash({ message: variableFetchErrorText });
       },
       watchLoading(flag) {
@@ -42,7 +57,26 @@ export default {
   },
   computed: {
     isLoading() {
-      return this.$apollo.queries.adminVariables.loading && this.isInitialLoading;
+      return (
+        (this.$apollo.queries.adminVariables.loading && this.isInitialLoading) ||
+        this.isLoadingMoreItems
+      );
+    },
+  },
+  watch: {
+    hasNextPage(flag) {
+      // Because graphQL has a limit of 100 items,
+      // we batch load all the variables by making successive queries
+      // to keep the same UX. As a safeguard, we make sure that we cannot go over
+      // 20 consecutive API calls, which means 2000 variables loaded maximum.
+      if (flag && this.loadingCounter < 20) {
+        this.hasNextPage = false;
+        this.fetchMoreVariables();
+        this.loadingCounter += 1;
+      } else if (this.loadingCounter >= 20) {
+        createFlash({ message: this.$options.tooManyCallsError });
+        reportMessageToSentry(this.$options.componentName, this.$options.tooManyCallsError, {});
+      }
     },
   },
   methods: {
@@ -52,6 +86,15 @@ export default {
     deleteVariable(variable) {
       this.variableMutation(DELETE_MUTATION_ACTION, variable);
     },
+    fetchMoreVariables() {
+      this.isLoadingMoreItems = true;
+
+      this.$apollo.queries.adminVariables.fetchMore({
+        variables: {
+          after: this.pageInfo.endCursor,
+        },
+      });
+    },
     updateVariable(variable) {
       this.variableMutation(UPDATE_MUTATION_ACTION, variable);
     },
@@ -81,6 +124,10 @@ export default {
       }
     },
   },
+  componentName: 'InstanceVariables',
+  i18n: {
+    tooManyCallsError: __('Maximum number of variables loaded (2000)'),
+  },
   mutationData: {
     [ADD_MUTATION_ACTION]: { action: addAdminVariable, name: 'addAdminVariable' },
     [UPDATE_MUTATION_ACTION]: { action: updateAdminVariable, name: 'updateAdminVariable' },
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
index 3522243e3e72e0..7d5b4b8766ce89 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
@@ -1,7 +1,9 @@
 <script>
 import createFlash from '~/flash';
+import { __ } from '~/locale';
 import { convertToGraphQLId } from '~/graphql_shared/utils';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import { reportMessageToSentry } from '../utils';
 import getGroupVariables from '../graphql/queries/group_variables.query.graphql';
 import {
   ADD_MUTATION_ACTION,
@@ -25,6 +27,10 @@ export default {
   data() {
     return {
       groupVariables: [],
+      hasNextPage: false,
+      isLoadingMoreItems: false,
+      loadingCounter: 0,
+      pageInfo: {},
     };
   },
   apollo: {
@@ -38,7 +44,18 @@ export default {
       update(data) {
         return data?.group?.ciVariables?.nodes || [];
       },
+      result({ data }) {
+        this.pageInfo = data?.group?.ciVariables?.pageInfo || this.pageInfo;
+        this.hasNextPage = this.pageInfo?.hasNextPage || false;
+        // If there are no more pages to load, we are done with the initial
+        // batch of loading
+        if (!this.hasNextPage) {
+          this.isLoadingMoreItems = false;
+        }
+      },
       error() {
+        this.isLoadingMoreItems = false;
+        this.hasNextPage = false;
         createFlash({ message: variableFetchErrorText });
       },
     },
@@ -48,7 +65,28 @@ export default {
       return this.glFeatures.groupScopedCiVariables;
     },
     isLoading() {
-      return this.$apollo.queries.groupVariables.loading;
+      return this.$apollo.queries.groupVariables.loading || this.isLoadingMoreItems;
+    },
+  },
+  watch: {
+    hasNextPage(flag) {
+      // Because graphQL has a limit of 100 items,
+      // we batch load all the variables by making successive queries
+      // to keep the same UX. As a safeguard, we make sure that we cannot go over
+      // 20 consecutive API calls, which means 2000 variables loaded maximum.
+      if (flag && this.loadingCounter < 20) {
+        this.hasNextPage = false;
+        this.fetchMoreVariables();
+        this.loadingCounter += 1;
+      } else if (this.loadingCounter >= 20) {
+        this.isLoadingMoreItems = false;
+        createFlash({ message: this.$options.i18n.tooManyCallsError });
+        reportMessageToSentry(
+          this.$options.componentName,
+          this.$options.i18n.tooManyCallsError,
+          {},
+        );
+      }
     },
   },
   methods: {
@@ -58,6 +96,16 @@ export default {
     deleteVariable(variable) {
       this.variableMutation(DELETE_MUTATION_ACTION, variable);
     },
+    fetchMoreVariables() {
+      this.isLoadingMoreItems = true;
+
+      this.$apollo.queries.groupVariables.fetchMore({
+        variables: {
+          fullPath: this.groupPath,
+          after: this.pageInfo.endCursor,
+        },
+      });
+    },
     updateVariable(variable) {
       this.variableMutation(UPDATE_MUTATION_ACTION, variable);
     },
@@ -84,6 +132,10 @@ export default {
       }
     },
   },
+  componentName: 'GroupVariables',
+  i18n: {
+    tooManyCallsError: __('Maximum number of variables loaded (2000)'),
+  },
   mutationData: {
     [ADD_MUTATION_ACTION]: { action: addGroupVariable, name: 'addGroupVariable' },
     [UPDATE_MUTATION_ACTION]: { action: updateGroupVariable, name: 'updateGroupVariable' },
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
index 29db02a3c59a9c..d7784fb8b03606 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
@@ -1,9 +1,10 @@
 <script>
 import createFlash from '~/flash';
+import { __ } from '~/locale';
 import { convertToGraphQLId } from '~/graphql_shared/utils';
 import getProjectEnvironments from '../graphql/queries/project_environments.query.graphql';
 import getProjectVariables from '../graphql/queries/project_variables.query.graphql';
-import { mapEnvironmentNames } from '../utils';
+import { mapEnvironmentNames, reportMessageToSentry } from '../utils';
 import {
   ADD_MUTATION_ACTION,
   DELETE_MUTATION_ACTION,
@@ -25,6 +26,10 @@ export default {
   inject: ['endpoint', 'projectFullPath', 'projectId'],
   data() {
     return {
+      hasNextPage: false,
+      isLoadingMoreItems: false,
+      loadingCounter: 0,
+      pageInfo: {},
       projectEnvironments: [],
       projectVariables: [],
     };
@@ -48,13 +53,25 @@ export default {
       query: getProjectVariables,
       variables() {
         return {
+          after: null,
           fullPath: this.projectFullPath,
         };
       },
       update(data) {
         return data?.project?.ciVariables?.nodes || [];
       },
+      result({ data }) {
+        this.pageInfo = data?.project?.ciVariables?.pageInfo || this.pageInfo;
+        this.hasNextPage = this.pageInfo?.hasNextPage || false;
+        // If there are no more pages to load, we are done with the initial
+        // batch of loading
+        if (!this.hasNextPage) {
+          this.isLoadingMoreItems = false;
+        }
+      },
       error() {
+        this.isLoadingMoreItems = false;
+        this.hasNextPage = false;
         createFlash({ message: variableFetchErrorText });
       },
     },
@@ -63,10 +80,32 @@ export default {
     isLoading() {
       return (
         this.$apollo.queries.projectVariables.loading ||
-        this.$apollo.queries.projectEnvironments.loading
+        this.$apollo.queries.projectEnvironments.loading ||
+        this.isLoadingMoreItems
       );
     },
   },
+  watch: {
+    hasNextPage(flag) {
+      // Because graphQL has a limit of 100 items,
+      // we batch load all the variables by making successive queries
+      // to keep the same UX. As a safeguard, we make sure that we cannot go over
+      // 20 consecutive API calls, which means 2000 variables loaded maximum.
+      if (flag && this.loadingCounter < 20) {
+        this.hasNextPage = false;
+        this.fetchMoreVariables();
+        this.loadingCounter += 1;
+      } else if (this.loadingCounter >= 20) {
+        this.isLoadingMoreItems = false;
+        createFlash({ message: this.$options.i18n.tooManyCallsError });
+        reportMessageToSentry(
+          this.$options.i18n.componentName,
+          this.$options.i18n.tooManyCallsError,
+          {},
+        );
+      }
+    },
+  },
   methods: {
     addVariable(variable) {
       this.variableMutation(ADD_MUTATION_ACTION, variable);
@@ -74,6 +113,16 @@ export default {
     deleteVariable(variable) {
       this.variableMutation(DELETE_MUTATION_ACTION, variable);
     },
+    fetchMoreVariables() {
+      this.isLoadingMoreItems = true;
+
+      this.$apollo.queries.projectVariables.fetchMore({
+        variables: {
+          fullPath: this.projectFullPath,
+          after: this.pageInfo.endCursor,
+        },
+      });
+    },
     updateVariable(variable) {
       this.variableMutation(UPDATE_MUTATION_ACTION, variable);
     },
@@ -89,16 +138,21 @@ export default {
             variable,
           },
         });
-
-        const { errors } = data[currentMutation.name];
-        if (errors.length > 0) {
-          createFlash({ message: errors[0] });
+        if (data[currentMutation.name]) {
+          const { errors } = data[currentMutation.name];
+          if (errors?.length > 0) {
+            createFlash({ message: errors[0] });
+          }
         }
-      } catch (e) {
+      } catch {
         createFlash({ message: genericMutationErrorText });
       }
     },
   },
+  componentName: 'ProjectVariables',
+  i18n: {
+    tooManyCallsError: __('Maximum number of variables loaded (2000)'),
+  },
   mutationData: {
     [ADD_MUTATION_ACTION]: { action: addProjectVariable, name: 'addProjectVariable' },
     [UPDATE_MUTATION_ACTION]: { action: updateProjectVariable, name: 'updateProjectVariable' },
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue
index 1bb94080694704..959ef6864fb0a5 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue
@@ -75,8 +75,7 @@ export default {
   props: {
     isLoading: {
       type: Boolean,
-      required: false,
-      default: false,
+      required: true,
     },
     variables: {
       type: Array,
diff --git a/app/assets/javascripts/ci_variable_list/constants.js b/app/assets/javascripts/ci_variable_list/constants.js
index e2dd28cdaa17ff..ccad08ef8b6b03 100644
--- a/app/assets/javascripts/ci_variable_list/constants.js
+++ b/app/assets/javascripts/ci_variable_list/constants.js
@@ -52,7 +52,7 @@ export const groupString = 'Group';
 // eslint-disable-next-line @gitlab/require-i18n-strings
 export const instanceString = 'Instance';
 // eslint-disable-next-line @gitlab/require-i18n-strings
-export const projectString = 'Instance';
+export const projectString = 'Project';
 
 export const AWS_TIP_DISMISSED_COOKIE_NAME = 'ci_variable_list_constants_aws_tip_dismissed';
 export const AWS_TIP_MESSAGE = __(
diff --git a/app/assets/javascripts/ci_variable_list/graphql/queries/group_variables.query.graphql b/app/assets/javascripts/ci_variable_list/graphql/queries/group_variables.query.graphql
index c6dd6d4faafbad..b5555fe44016af 100644
--- a/app/assets/javascripts/ci_variable_list/graphql/queries/group_variables.query.graphql
+++ b/app/assets/javascripts/ci_variable_list/graphql/queries/group_variables.query.graphql
@@ -1,9 +1,13 @@
 #import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql"
+#import "~/graphql_shared/fragments/page_info.fragment.graphql"
 
-query getGroupVariables($fullPath: ID!) {
+query getGroupVariables($after: String, $first: Int = 100, $fullPath: ID!) {
   group(fullPath: $fullPath) {
     id
-    ciVariables {
+    ciVariables(after: $after, first: $first) {
+      pageInfo {
+        ...PageInfo
+      }
       nodes {
         ...BaseCiVariable
         ... on CiGroupVariable {
diff --git a/app/assets/javascripts/ci_variable_list/graphql/queries/project_variables.query.graphql b/app/assets/javascripts/ci_variable_list/graphql/queries/project_variables.query.graphql
index a60a50e4bc4681..08b5bf7af16432 100644
--- a/app/assets/javascripts/ci_variable_list/graphql/queries/project_variables.query.graphql
+++ b/app/assets/javascripts/ci_variable_list/graphql/queries/project_variables.query.graphql
@@ -1,9 +1,13 @@
 #import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql"
+#import "~/graphql_shared/fragments/page_info.fragment.graphql"
 
-query getProjectVariables($fullPath: ID!) {
+query getProjectVariables($after: String, $first: Int = 100, $fullPath: ID!) {
   project(fullPath: $fullPath) {
     id
-    ciVariables {
+    ciVariables(after: $after, first: $first) {
+      pageInfo {
+        ...PageInfo
+      }
       nodes {
         ...BaseCiVariable
         environmentScope
diff --git a/app/assets/javascripts/ci_variable_list/graphql/queries/variables.query.graphql b/app/assets/javascripts/ci_variable_list/graphql/queries/variables.query.graphql
index 95056842b4932a..2667d6606fe794 100644
--- a/app/assets/javascripts/ci_variable_list/graphql/queries/variables.query.graphql
+++ b/app/assets/javascripts/ci_variable_list/graphql/queries/variables.query.graphql
@@ -1,7 +1,11 @@
 #import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql"
+#import "~/graphql_shared/fragments/page_info.fragment.graphql"
 
-query getVariables {
-  ciVariables {
+query getVariables($after: String, $first: Int = 100) {
+  ciVariables(after: $after, first: $first) {
+    pageInfo {
+      ...PageInfo
+    }
     nodes {
       ...BaseCiVariable
       ... on CiInstanceVariable {
diff --git a/app/assets/javascripts/ci_variable_list/graphql/resolvers.js b/app/assets/javascripts/ci_variable_list/graphql/settings.js
similarity index 66%
rename from app/assets/javascripts/ci_variable_list/graphql/resolvers.js
rename to app/assets/javascripts/ci_variable_list/graphql/settings.js
index c041531ae308ee..ecdc4f220bd085 100644
--- a/app/assets/javascripts/ci_variable_list/graphql/resolvers.js
+++ b/app/assets/javascripts/ci_variable_list/graphql/settings.js
@@ -3,7 +3,7 @@ import {
   convertObjectPropsToCamelCase,
   convertObjectPropsToSnakeCase,
 } from '../../lib/utils/common_utils';
-import { getIdFromGraphQLId } from '../../graphql_shared/utils';
+import { convertToGraphQLId, getIdFromGraphQLId } from '../../graphql_shared/utils';
 import {
   GRAPHQL_GROUP_TYPE,
   GRAPHQL_PROJECT_TYPE,
@@ -30,6 +30,7 @@ const mapVariableTypes = (variables = [], kind) => {
     return {
       __typename: `Ci${kind}Variable`,
       ...convertObjectPropsToCamelCase(ciVar),
+      id: convertToGraphQLId('Ci::Variable', ciVar.id),
       variableType: ciVar.variable_type ? ciVar.variable_type.toUpperCase() : ciVar.variableType,
     };
   });
@@ -40,9 +41,16 @@ const prepareProjectGraphQLResponse = ({ data, projectId, errors = [] }) => {
     errors,
     project: {
       __typename: GRAPHQL_PROJECT_TYPE,
-      id: projectId,
+      id: convertToGraphQLId(GRAPHQL_PROJECT_TYPE, projectId),
       ciVariables: {
-        __typename: 'CiVariableConnection',
+        __typename: `Ci${GRAPHQL_PROJECT_TYPE}VariableConnection`,
+        pageInfo: {
+          __typename: 'PageInfo',
+          hasNextPage: false,
+          hasPreviousPage: false,
+          startCursor: '',
+          endCursor: '',
+        },
         nodes: mapVariableTypes(data.variables, projectString),
       },
     },
@@ -54,9 +62,16 @@ const prepareGroupGraphQLResponse = ({ data, groupId, errors = [] }) => {
     errors,
     group: {
       __typename: GRAPHQL_GROUP_TYPE,
-      id: groupId,
+      id: convertToGraphQLId(GRAPHQL_GROUP_TYPE, groupId),
       ciVariables: {
-        __typename: 'CiVariableConnection',
+        __typename: `Ci${GRAPHQL_GROUP_TYPE}VariableConnection`,
+        pageInfo: {
+          __typename: 'PageInfo',
+          hasNextPage: false,
+          hasPreviousPage: false,
+          startCursor: '',
+          endCursor: '',
+        },
         nodes: mapVariableTypes(data.variables, groupString),
       },
     },
@@ -68,24 +83,42 @@ const prepareAdminGraphQLResponse = ({ data, errors = [] }) => {
     errors,
     ciVariables: {
       __typename: `Ci${instanceString}VariableConnection`,
+      pageInfo: {
+        __typename: 'PageInfo',
+        hasNextPage: false,
+        hasPreviousPage: false,
+        startCursor: '',
+        endCursor: '',
+      },
       nodes: mapVariableTypes(data.variables, instanceString),
     },
   };
 };
 
-const callProjectEndpoint = async ({
+async function callProjectEndpoint({
   endpoint,
   fullPath,
   variable,
   projectId,
   cache,
   destroy = false,
-}) => {
+}) {
   try {
     const { data } = await axios.patch(endpoint, {
       variables_attributes: [prepareVariableForApi({ variable, destroy })],
     });
-    return prepareProjectGraphQLResponse({ data, projectId });
+
+    const graphqlData = prepareProjectGraphQLResponse({ data, projectId });
+
+    cache.writeQuery({
+      query: getProjectVariables,
+      variables: {
+        fullPath,
+        after: null,
+      },
+      data: graphqlData,
+    });
+    return graphqlData;
   } catch (e) {
     return prepareProjectGraphQLResponse({
       data: cache.readQuery({ query: getProjectVariables, variables: { fullPath } }),
@@ -93,7 +126,7 @@ const callProjectEndpoint = async ({
       errors: [...e.response.data],
     });
   }
-};
+}
 
 const callGroupEndpoint = async ({
   endpoint,
@@ -107,7 +140,15 @@ const callGroupEndpoint = async ({
     const { data } = await axios.patch(endpoint, {
       variables_attributes: [prepareVariableForApi({ variable, destroy })],
     });
-    return prepareGroupGraphQLResponse({ data, groupId });
+
+    const graphqlData = prepareGroupGraphQLResponse({ data, groupId });
+
+    cache.writeQuery({
+      query: getGroupVariables,
+      data: graphqlData,
+    });
+
+    return graphqlData;
   } catch (e) {
     return prepareGroupGraphQLResponse({
       data: cache.readQuery({ query: getGroupVariables, variables: { fullPath } }),
@@ -123,7 +164,14 @@ const callAdminEndpoint = async ({ endpoint, variable, cache, destroy = false })
       variables_attributes: [prepareVariableForApi({ variable, destroy })],
     });
 
-    return prepareAdminGraphQLResponse({ data });
+    const graphqlData = prepareAdminGraphQLResponse({ data });
+
+    cache.writeQuery({
+      query: getAdminVariables,
+      data: graphqlData,
+    });
+
+    return graphqlData;
   } catch (e) {
     return prepareAdminGraphQLResponse({
       data: cache.readQuery({ query: getAdminVariables }),
@@ -163,3 +211,46 @@ export const resolvers = {
     },
   },
 };
+
+export const mergeVariables = (existing, incoming, { args }) => {
+  if (!existing || !args?.after) {
+    return incoming;
+  }
+
+  const { nodes, ...rest } = incoming;
+  const result = rest;
+  result.nodes = [...existing.nodes, ...nodes];
+
+  return result;
+};
+
+export const cacheConfig = {
+  cacheConfig: {
+    typePolicies: {
+      Query: {
+        fields: {
+          ciVariables: {
+            keyArgs: false,
+            merge: mergeVariables,
+          },
+        },
+      },
+      Project: {
+        fields: {
+          ciVariables: {
+            keyArgs: ['fullPath', 'endpoint', 'projectId'],
+            merge: mergeVariables,
+          },
+        },
+      },
+      Group: {
+        fields: {
+          ciVariables: {
+            keyArgs: ['fullPath'],
+            merge: mergeVariables,
+          },
+        },
+      },
+    },
+  },
+};
diff --git a/app/assets/javascripts/ci_variable_list/index.js b/app/assets/javascripts/ci_variable_list/index.js
index f5bdd4c7b1e026..a55052b650a69a 100644
--- a/app/assets/javascripts/ci_variable_list/index.js
+++ b/app/assets/javascripts/ci_variable_list/index.js
@@ -6,7 +6,7 @@ import CiAdminVariables from './components/ci_admin_variables.vue';
 import CiGroupVariables from './components/ci_group_variables.vue';
 import CiProjectVariables from './components/ci_project_variables.vue';
 import LegacyCiVariableSettings from './components/legacy_ci_variable_settings.vue';
-import { resolvers } from './graphql/resolvers';
+import { cacheConfig, resolvers } from './graphql/settings';
 import createStore from './store';
 
 const mountCiVariableListApp = (containerEl) => {
@@ -45,7 +45,7 @@ const mountCiVariableListApp = (containerEl) => {
   Vue.use(VueApollo);
 
   const apolloProvider = new VueApollo({
-    defaultClient: createDefaultClient(resolvers),
+    defaultClient: createDefaultClient(resolvers, cacheConfig),
   });
 
   return new Vue({
diff --git a/app/assets/javascripts/ci_variable_list/utils.js b/app/assets/javascripts/ci_variable_list/utils.js
index 1faa97a5f73009..eeca69274ce2cf 100644
--- a/app/assets/javascripts/ci_variable_list/utils.js
+++ b/app/assets/javascripts/ci_variable_list/utils.js
@@ -1,3 +1,4 @@
+import * as Sentry from '@sentry/browser';
 import { uniq } from 'lodash';
 import { allEnvironments } from './constants';
 
@@ -48,3 +49,12 @@ export const convertEnvironmentScope = (environmentScope = '') => {
 export const mapEnvironmentNames = (nodes = []) => {
   return nodes.map((env) => env.name);
 };
+
+export const reportMessageToSentry = (component, message, context) => {
+  Sentry.withScope((scope) => {
+    // eslint-disable-next-line @gitlab/require-i18n-strings
+    scope.setContext('Vue data', context);
+    scope.setTag('component', component);
+    Sentry.captureMessage(message);
+  });
+};
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 0742b7bba2bdea..1ae9f965d0e1b6 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -24425,6 +24425,9 @@ msgstr ""
 msgid "Maximum number of unique IP addresses per user."
 msgstr ""
 
+msgid "Maximum number of variables loaded (2000)"
+msgstr ""
+
 msgid "Maximum of 255 characters"
 msgstr ""
 
diff --git a/spec/frontend/ci_variable_list/components/ci_admin_variables_spec.js b/spec/frontend/ci_variable_list/components/ci_admin_variables_spec.js
index 920ceaefb70b17..0c28a3703c4dd6 100644
--- a/spec/frontend/ci_variable_list/components/ci_admin_variables_spec.js
+++ b/spec/frontend/ci_variable_list/components/ci_admin_variables_spec.js
@@ -5,7 +5,7 @@ import { shallowMount } from '@vue/test-utils';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import createFlash from '~/flash';
-import { resolvers } from '~/ci_variable_list/graphql/resolvers';
+import { resolvers } from '~/ci_variable_list/graphql/settings';
 
 import ciAdminVariables from '~/ci_variable_list/components/ci_admin_variables.vue';
 import ciVariableSettings from '~/ci_variable_list/components/ci_variable_settings.vue';
diff --git a/spec/frontend/ci_variable_list/components/ci_group_variables_spec.js b/spec/frontend/ci_variable_list/components/ci_group_variables_spec.js
index e45656acfd8ae2..7f65bd32f40230 100644
--- a/spec/frontend/ci_variable_list/components/ci_group_variables_spec.js
+++ b/spec/frontend/ci_variable_list/components/ci_group_variables_spec.js
@@ -5,7 +5,7 @@ import { shallowMount } from '@vue/test-utils';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import createFlash from '~/flash';
-import { resolvers } from '~/ci_variable_list/graphql/resolvers';
+import { resolvers } from '~/ci_variable_list/graphql/settings';
 import { convertToGraphQLId } from '~/graphql_shared/utils';
 
 import ciGroupVariables from '~/ci_variable_list/components/ci_group_variables.vue';
diff --git a/spec/frontend/ci_variable_list/components/ci_project_variables_spec.js b/spec/frontend/ci_variable_list/components/ci_project_variables_spec.js
index 867f8e0cf8fddb..aa6cb12271ace6 100644
--- a/spec/frontend/ci_variable_list/components/ci_project_variables_spec.js
+++ b/spec/frontend/ci_variable_list/components/ci_project_variables_spec.js
@@ -5,7 +5,7 @@ import { shallowMount } from '@vue/test-utils';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import createFlash from '~/flash';
-import { resolvers } from '~/ci_variable_list/graphql/resolvers';
+import { resolvers } from '~/ci_variable_list/graphql/settings';
 import { convertToGraphQLId } from '~/graphql_shared/utils';
 
 import ciProjectVariables from '~/ci_variable_list/components/ci_project_variables.vue';
diff --git a/spec/frontend/ci_variable_list/mocks.js b/spec/frontend/ci_variable_list/mocks.js
index 6d633c8b7400a4..6f3e73f8b83a2b 100644
--- a/spec/frontend/ci_variable_list/mocks.js
+++ b/spec/frontend/ci_variable_list/mocks.js
@@ -45,6 +45,12 @@ const createDefaultVars = ({ withScope = true, kind } = {}) => {
 
   return {
     __typename: `Ci${kind}VariableConnection`,
+    pageInfo: {
+      startCursor: 'adsjsd12kldpsa',
+      endCursor: 'adsjsd12kldpsa',
+      hasPreviousPage: false,
+      hasNextPage: true,
+    },
     nodes: base,
   };
 };
-- 
GitLab


From 33cff90ec0732523841c19e069c8ed03b7626371 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= <fcaplette@gitlab.com>
Date: Wed, 21 Sep 2022 09:35:22 -0400
Subject: [PATCH 2/3] Move watchers to result hooks

---
 .../components/ci_admin_variables.vue         | 28 +++++++--------
 .../components/ci_group_variables.vue         | 34 ++++++-------------
 .../components/ci_project_variables.vue       | 34 ++++++-------------
 3 files changed, 34 insertions(+), 62 deletions(-)

diff --git a/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
index 06688bdaa7bdd1..fcb4e855b2aadc 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
@@ -39,8 +39,20 @@ export default {
       result({ data }) {
         this.pageInfo = data?.ciVariables?.pageInfo || this.pageInfo;
         this.hasNextPage = this.pageInfo?.hasNextPage || false;
+
+        // Because graphQL has a limit of 100 items,
+        // we batch load all the variables by making successive queries
+        // to keep the same UX. As a safeguard, we make sure that we cannot go over
+        // 20 consecutive API calls, which means 2000 variables loaded maximum.
         if (!this.hasNextPage) {
           this.isLoadingMoreItems = false;
+        } else if (this.loadingCounter < 20) {
+          this.hasNextPage = false;
+          this.fetchMoreVariables();
+          this.loadingCounter += 1;
+        } else {
+          createFlash({ message: this.$options.tooManyCallsError });
+          reportMessageToSentry(this.$options.componentName, this.$options.tooManyCallsError, {});
         }
       },
       error() {
@@ -63,22 +75,6 @@ export default {
       );
     },
   },
-  watch: {
-    hasNextPage(flag) {
-      // Because graphQL has a limit of 100 items,
-      // we batch load all the variables by making successive queries
-      // to keep the same UX. As a safeguard, we make sure that we cannot go over
-      // 20 consecutive API calls, which means 2000 variables loaded maximum.
-      if (flag && this.loadingCounter < 20) {
-        this.hasNextPage = false;
-        this.fetchMoreVariables();
-        this.loadingCounter += 1;
-      } else if (this.loadingCounter >= 20) {
-        createFlash({ message: this.$options.tooManyCallsError });
-        reportMessageToSentry(this.$options.componentName, this.$options.tooManyCallsError, {});
-      }
-    },
-  },
   methods: {
     addVariable(variable) {
       this.variableMutation(ADD_MUTATION_ACTION, variable);
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
index 7d5b4b8766ce89..f0d117197377f5 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
@@ -47,10 +47,19 @@ export default {
       result({ data }) {
         this.pageInfo = data?.group?.ciVariables?.pageInfo || this.pageInfo;
         this.hasNextPage = this.pageInfo?.hasNextPage || false;
-        // If there are no more pages to load, we are done with the initial
-        // batch of loading
+        // Because graphQL has a limit of 100 items,
+        // we batch load all the variables by making successive queries
+        // to keep the same UX. As a safeguard, we make sure that we cannot go over
+        // 20 consecutive API calls, which means 2000 variables loaded maximum.
         if (!this.hasNextPage) {
           this.isLoadingMoreItems = false;
+        } else if (this.loadingCounter < 20) {
+          this.hasNextPage = false;
+          this.fetchMoreVariables();
+          this.loadingCounter += 1;
+        } else {
+          createFlash({ message: this.$options.tooManyCallsError });
+          reportMessageToSentry(this.$options.componentName, this.$options.tooManyCallsError, {});
         }
       },
       error() {
@@ -68,27 +77,6 @@ export default {
       return this.$apollo.queries.groupVariables.loading || this.isLoadingMoreItems;
     },
   },
-  watch: {
-    hasNextPage(flag) {
-      // Because graphQL has a limit of 100 items,
-      // we batch load all the variables by making successive queries
-      // to keep the same UX. As a safeguard, we make sure that we cannot go over
-      // 20 consecutive API calls, which means 2000 variables loaded maximum.
-      if (flag && this.loadingCounter < 20) {
-        this.hasNextPage = false;
-        this.fetchMoreVariables();
-        this.loadingCounter += 1;
-      } else if (this.loadingCounter >= 20) {
-        this.isLoadingMoreItems = false;
-        createFlash({ message: this.$options.i18n.tooManyCallsError });
-        reportMessageToSentry(
-          this.$options.componentName,
-          this.$options.i18n.tooManyCallsError,
-          {},
-        );
-      }
-    },
-  },
   methods: {
     addVariable(variable) {
       this.variableMutation(ADD_MUTATION_ACTION, variable);
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
index d7784fb8b03606..26e7b4b2206d5e 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
@@ -63,10 +63,19 @@ export default {
       result({ data }) {
         this.pageInfo = data?.project?.ciVariables?.pageInfo || this.pageInfo;
         this.hasNextPage = this.pageInfo?.hasNextPage || false;
-        // If there are no more pages to load, we are done with the initial
-        // batch of loading
+        // Because graphQL has a limit of 100 items,
+        // we batch load all the variables by making successive queries
+        // to keep the same UX. As a safeguard, we make sure that we cannot go over
+        // 20 consecutive API calls, which means 2000 variables loaded maximum.
         if (!this.hasNextPage) {
           this.isLoadingMoreItems = false;
+        } else if (this.loadingCounter < 20) {
+          this.hasNextPage = false;
+          this.fetchMoreVariables();
+          this.loadingCounter += 1;
+        } else {
+          createFlash({ message: this.$options.tooManyCallsError });
+          reportMessageToSentry(this.$options.componentName, this.$options.tooManyCallsError, {});
         }
       },
       error() {
@@ -85,27 +94,6 @@ export default {
       );
     },
   },
-  watch: {
-    hasNextPage(flag) {
-      // Because graphQL has a limit of 100 items,
-      // we batch load all the variables by making successive queries
-      // to keep the same UX. As a safeguard, we make sure that we cannot go over
-      // 20 consecutive API calls, which means 2000 variables loaded maximum.
-      if (flag && this.loadingCounter < 20) {
-        this.hasNextPage = false;
-        this.fetchMoreVariables();
-        this.loadingCounter += 1;
-      } else if (this.loadingCounter >= 20) {
-        this.isLoadingMoreItems = false;
-        createFlash({ message: this.$options.i18n.tooManyCallsError });
-        reportMessageToSentry(
-          this.$options.i18n.componentName,
-          this.$options.i18n.tooManyCallsError,
-          {},
-        );
-      }
-    },
-  },
   methods: {
     addVariable(variable) {
       this.variableMutation(ADD_MUTATION_ACTION, variable);
-- 
GitLab


From 893cdcbeedcaa6d1b0b30819d55da7350ff5df14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= <fcaplette@gitlab.com>
Date: Wed, 21 Sep 2022 09:56:47 -0400
Subject: [PATCH 3/3] Restructure mutation conditionals

---
 .../ci_variable_list/components/ci_admin_variables.vue      | 5 ++---
 .../ci_variable_list/components/ci_group_variables.vue      | 5 ++---
 .../ci_variable_list/components/ci_project_variables.vue    | 6 ++----
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
index fcb4e855b2aadc..7d91e0f9550dd2 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_admin_variables.vue
@@ -105,9 +105,8 @@ export default {
           },
         });
 
-        const { errors } = data[currentMutation.name];
-
-        if (errors.length > 0) {
+        if (data[currentMutation.name]?.errors?.length) {
+          const { errors } = data[currentMutation.name];
           createFlash({ message: errors[0] });
         } else {
           // The writing to cache for admin variable is not working
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
index f0d117197377f5..0f549a596cf0fd 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue
@@ -110,9 +110,8 @@ export default {
           },
         });
 
-        const { errors } = data[currentMutation.name];
-
-        if (errors.length > 0) {
+        if (data[currentMutation.name]?.errors?.length) {
+          const { errors } = data[currentMutation.name];
           createFlash({ message: errors[0] });
         }
       } catch {
diff --git a/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue b/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
index 26e7b4b2206d5e..d601f7a424ccec 100644
--- a/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
+++ b/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue
@@ -126,11 +126,9 @@ export default {
             variable,
           },
         });
-        if (data[currentMutation.name]) {
+        if (data[currentMutation.name]?.errors?.length) {
           const { errors } = data[currentMutation.name];
-          if (errors?.length > 0) {
-            createFlash({ message: errors[0] });
-          }
+          createFlash({ message: errors[0] });
         }
       } catch {
         createFlash({ message: genericMutationErrorText });
-- 
GitLab