Merge branch 'terraform-state-errors-graphql' into 'master'

Display Terraform action errors to user

See merge request gitlab-org/gitlab!51397

app/assets/javascripts/terraform/components/states_table.vue

 <script>
-import { GlBadge, GlIcon, GlLink, GlSprintf, GlTable, GlTooltip } from '@gitlab/ui';
+import { GlAlert, GlBadge, GlIcon, GlLink, GlSprintf, GlTable, GlTooltip } from '@gitlab/ui';
 import { s__ } from '~/locale';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import CiBadge from '~/vue_shared/components/ci_badge_link.vue';
@@ -10,6 +10,7 @@ import timeagoMixin from '~/vue_shared/mixins/timeago';
 export default {
   components: {
     CiBadge,
+    GlAlert,
     GlBadge,
     GlIcon,
     GlLink,
@@ -105,6 +106,7 @@ export default {
     :items="states"
     :fields="fields"
     data-testid="terraform-states-table"
+    details-td-class="gl-p-0!"
     fixed
     stacked="md"
   >
@@ -189,5 +191,21 @@ export default {
     <template v-if="terraformAdmin" #cell(actions)="{ item }">
       <state-actions :state="item" />
     </template>
+
+    <template #row-details="row">
+      <gl-alert
+        data-testid="terraform-states-table-error"
+        variant="danger"
+        @dismiss="row.toggleDetails"
+      >
+        <span
+          v-for="errorMessage in row.item.errorMessages"
+          :key="errorMessage"
+          class="gl-display-flex gl-justify-content-start"
+        >
+          {{ errorMessage }}
+        </span>
+      </gl-alert>
+    </template>
   </gl-table>
 </template>

app/assets/javascripts/terraform/components/states_table_actions.vue

@@ -10,6 +10,7 @@ import {
   GlSprintf,
 } from '@gitlab/ui';
 import { s__ } from '~/locale';
+import addDataToState from '../graphql/mutations/add_data_to_state.mutation.graphql';
 import lockState from '../graphql/mutations/lock_state.mutation.graphql';
 import unlockState from '../graphql/mutations/unlock_state.mutation.graphql';
 import removeState from '../graphql/mutations/remove_state.mutation.graphql';
@@ -33,13 +34,13 @@ export default {
   },
   data() {
     return {
-      loading: false,
       showRemoveModal: false,
       removeConfirmText: '',
     };
   },
   i18n: {
     downloadJSON: s__('Terraform|Download JSON'),
+    errorUpdate: s__('Terraform|An error occurred while changing the state file'),
     lock: s__('Terraform|Lock'),
     modalBody: s__(
       'Terraform|You are about to remove the State file %{name}. This will permanently delete all the State versions and history. The infrastructure provisioned previously	will remain intact, only the state file with all its versions are to be removed. This action is non-revertible.',
@@ -76,19 +77,37 @@ export default {
       this.removeConfirmText = '';
     },
     lock() {
-      this.stateMutation(lockState);
+      this.stateActionMutation(lockState);
     },
     unlock() {
-      this.stateMutation(unlockState);
+      this.stateActionMutation(unlockState);
+    },
+    updateStateCache(newData) {
+      this.$apollo.mutate({
+        mutation: addDataToState,
+        variables: {
+          terraformState: {
+            ...this.state,
+            ...newData,
+          },
+        },
+      });
     },
     remove() {
       if (!this.disableModalSubmit) {
         this.hideModal();
-        this.stateMutation(removeState);
+        this.stateActionMutation(removeState);
       }
     },
-    stateMutation(mutation) {
-      this.loading = true;
+    stateActionMutation(mutation) {
+      let errorMessages = [];
+
+      this.updateStateCache({
+        _showDetails: false,
+        errorMessages,
+        loadingActions: true,
+      });
+
       this.$apollo
         .mutate({
           mutation,
@@ -99,9 +118,22 @@ export default {
           awaitRefetchQueries: true,
           notifyOnNetworkStatusChange: true,
         })
-        .catch(() => {})
+        .then(({ data }) => {
+          errorMessages =
+            data?.terraformStateDelete?.errors ||
+            data?.terraformStateLock?.errors ||
+            data?.terraformStateUnlock?.errors ||
+            [];
+        })
+        .catch(() => {
+          errorMessages = [this.$options.i18n.errorUpdate];
+        })
         .finally(() => {
-          this.loading = false;
+          this.updateStateCache({
+            _showDetails: Boolean(errorMessages.length),
+            errorMessages,
+            loadingActions: false,
+          });
         });
     },
   },
@@ -114,7 +146,7 @@ export default {
       icon="ellipsis_v"
       right
       :data-testid="`terraform-state-actions-${state.name}`"
-      :disabled="loading"
+      :disabled="state.loadingActions"
       toggle-class="gl-px-3! gl-shadow-none!"
     >
       <template #button-content>

app/assets/javascripts/terraform/graphql/fragments/state.fragment.graphql

@@ -2,6 +2,10 @@
 #import "./state_version.fragment.graphql"
 
 fragment State on TerraformState {
+  _showDetails @client
+  errorMessages @client
+  loadingActions @client
+
   id
   name
   lockedAt

app/assets/javascripts/terraform/graphql/mutations/add_data_to_state.mutation.graphql

+mutation addDataToTerraformState($terraformState: State!) {
+  addDataToTerraformState(terraformState: $terraformState) @client
+}

app/assets/javascripts/terraform/graphql/resolvers.js

+import TerraformState from './fragments/state.fragment.graphql';
+
+export default {
+  TerraformState: {
+    _showDetails: (state) => {
+      // eslint-disable-next-line no-underscore-dangle
+      return state._showDetails || false;
+    },
+    errorMessages: (state) => {
+      return state.errorMessages || [];
+    },
+    loadingActions: (state) => {
+      return state.loadingActions || false;
+    },
+  },
+  Mutation: {
+    addDataToTerraformState: (_, { terraformState }, { client }) => {
+      const fragmentData = {
+        id: terraformState.id,
+        fragment: TerraformState,
+        // eslint-disable-next-line @gitlab/require-i18n-strings
+        fragmentName: 'State',
+      };
+
+      const previousTerraformState = client.readFragment(fragmentData);
+
+      if (previousTerraformState) {
+        client.writeFragment({
+          ...fragmentData,
+          data: {
+            ...previousTerraformState,
+            // eslint-disable-next-line no-underscore-dangle
+            _showDetails: terraformState._showDetails,
+            errorMessages: terraformState.errorMessages,
+            loadingActions: terraformState.loadingActions,
+          },
+        });
+      }
+    },
+  },
+};

app/assets/javascripts/terraform/index.js

+import { defaultDataIdFromObject } from 'apollo-cache-inmemory';
 import Vue from 'vue';
 import VueApollo from 'vue-apollo';
 import TerraformList from './components/terraform_list.vue';
 import createDefaultClient from '~/lib/graphql';
+import resolvers from './graphql/resolvers';
 
 Vue.use(VueApollo);
 
@@ -12,7 +14,13 @@ export default () => {
     return null;
   }
 
-  const defaultClient = createDefaultClient();
+  const defaultClient = createDefaultClient(resolvers, {
+    cacheConfig: {
+      dataIdFromObject: (object) => {
+        return object.id || defaultDataIdFromObject(object);
+      },
+    },
+  });
 
   const { emptyStateImage, projectPath } = el.dataset;
 

changelogs/unreleased/terraform-state-errors-graphql.yml

+---
+title: Display Terraform list errors to user
+merge_request: 51397
+author:
+type: changed

locale/gitlab.pot

@@ -27881,6 +27881,9 @@ msgstr ""
 msgid "Terraform|Actions"
 msgstr ""
 
+msgid "Terraform|An error occurred while changing the state file"
+msgstr ""
+
 msgid "Terraform|An error occurred while loading your Terraform States"
 msgstr ""
 

spec/frontend/terraform/components/states_table_actions_spec.js

 import { GlDropdown, GlModal, GlSprintf } from '@gitlab/ui';
 import { createLocalVue, shallowMount } from '@vue/test-utils';
 import createMockApollo from 'helpers/mock_apollo_helper';
+import waitForPromises from 'helpers/wait_for_promises';
 import VueApollo from 'vue-apollo';
 import StateActions from '~/terraform/components/states_table_actions.vue';
 import lockStateMutation from '~/terraform/graphql/mutations/lock_state.mutation.graphql';
@@ -14,6 +15,7 @@ describe('StatesTableActions', () => {
   let lockResponse;
   let removeResponse;
   let unlockResponse;
+  let updateStateResponse;
   let wrapper;
 
   const defaultProps = {
@@ -26,7 +28,9 @@ describe('StatesTableActions', () => {
   };
 
   const createMockApolloProvider = () => {
-    lockResponse = jest.fn().mockResolvedValue({ data: { terraformStateLock: { errors: [] } } });
+    lockResponse = jest
+      .fn()
+      .mockResolvedValue({ data: { terraformStateLock: { errors: ['There was an error'] } } });
 
     removeResponse = jest
       .fn()
@@ -36,11 +40,20 @@ describe('StatesTableActions', () => {
       .fn()
       .mockResolvedValue({ data: { terraformStateUnlock: { errors: [] } } });
 
-    return createMockApollo([
+    updateStateResponse = jest.fn().mockResolvedValue({});
+
+    return createMockApollo(
+      [
         [lockStateMutation, lockResponse],
         [removeStateMutation, removeResponse],
         [unlockStateMutation, unlockResponse],
-    ]);
+      ],
+      {
+        Mutation: {
+          addDataToTerraformState: updateStateResponse,
+        },
+      },
+    );
   };
 
   const createComponent = (propsData = defaultProps) => {
@@ -56,6 +69,7 @@ describe('StatesTableActions', () => {
     return wrapper.vm.$nextTick();
   };
 
+  const findActionsDropdown = () => wrapper.find(GlDropdown);
   const findLockBtn = () => wrapper.find('[data-testid="terraform-state-lock"]');
   const findUnlockBtn = () => wrapper.find('[data-testid="terraform-state-unlock"]');
   const findDownloadBtn = () => wrapper.find('[data-testid="terraform-state-download"]');
@@ -70,9 +84,25 @@ describe('StatesTableActions', () => {
     lockResponse = null;
     removeResponse = null;
     unlockResponse = null;
+    updateStateResponse = null;
     wrapper.destroy();
   });
 
+  describe('when the state is loading', () => {
+    beforeEach(() => {
+      return createComponent({
+        state: {
+          ...defaultProps.state,
+          loadingActions: true,
+        },
+      });
+    });
+
+    it('disables the actions dropdown', () => {
+      expect(findActionsDropdown().props('disabled')).toBe(true);
+    });
+  });
+
   describe('download button', () => {
     it('displays a download button', () => {
       expect(findDownloadBtn().text()).toBe('Download JSON');
@@ -104,7 +134,8 @@ describe('StatesTableActions', () => {
     describe('when clicking the unlock button', () => {
       beforeEach(() => {
         findUnlockBtn().vm.$emit('click');
-        return wrapper.vm.$nextTick();
+
+        return waitForPromises();
       });
 
       it('calls the unlock mutation', () => {
@@ -137,7 +168,8 @@ describe('StatesTableActions', () => {
     describe('when clicking the lock button', () => {
       beforeEach(() => {
         findLockBtn().vm.$emit('click');
-        return wrapper.vm.$nextTick();
+
+        return waitForPromises();
       });
 
       it('calls the lock mutation', () => {
@@ -145,6 +177,42 @@ describe('StatesTableActions', () => {
           stateID: unlockedProps.state.id,
         });
       });
+
+      it('calls mutations to set loading and errors', () => {
+        // loading update
+        expect(updateStateResponse).toHaveBeenNthCalledWith(
+          1,
+          {},
+          {
+            terraformState: {
+              ...unlockedProps.state,
+              _showDetails: false,
+              errorMessages: [],
+              loadingActions: true,
+            },
+          },
+          // Apollo fields
+          expect.any(Object),
+          expect.any(Object),
+        );
+
+        // final update
+        expect(updateStateResponse).toHaveBeenNthCalledWith(
+          2,
+          {},
+          {
+            terraformState: {
+              ...unlockedProps.state,
+              _showDetails: true,
+              errorMessages: ['There was an error'],
+              loadingActions: false,
+            },
+          },
+          // Apollo fields
+          expect.any(Object),
+          expect.any(Object),
+        );
+      });
     });
   });
 
@@ -156,7 +224,8 @@ describe('StatesTableActions', () => {
     describe('when clicking the remove button', () => {
       beforeEach(() => {
         findRemoveBtn().vm.$emit('click');
-        return wrapper.vm.$nextTick();
+
+        return waitForPromises();
       });
 
       it('displays a remove modal', () => {

spec/frontend/terraform/components/states_table_spec.js

@@ -11,6 +11,8 @@ describe('StatesTable', () => {
   const defaultProps = {
     states: [
       {
+        _showDetails: true,
+        errorMessages: ['State 1 has errored'],
         name: 'state-1',
         lockedAt: '2020-10-13T00:00:00Z',
         lockedByUser: {
@@ -20,6 +22,8 @@ describe('StatesTable', () => {
         latestVersion: null,
       },
       {
+        _showDetails: false,
+        errorMessages: [],
         name: 'state-2',
         lockedAt: null,
         lockedByUser: null,
@@ -27,6 +31,8 @@ describe('StatesTable', () => {
         latestVersion: null,
       },
       {
+        _showDetails: false,
+        errorMessages: [],
         name: 'state-3',
         lockedAt: '2020-10-10T00:00:00Z',
         lockedByUser: {
@@ -54,6 +60,8 @@ describe('StatesTable', () => {
         },
       },
       {
+        _showDetails: true,
+        errorMessages: ['State 4 has errored'],
         name: 'state-4',
         lockedAt: '2020-10-10T00:00:00Z',
         lockedByUser: null,
@@ -154,6 +162,17 @@ describe('StatesTable', () => {
     expect(findActions().length).toEqual(0);
   });
 
+  it.each`
+    errorMessage                               | lineNumber
+    ${defaultProps.states[0].errorMessages[0]} | ${0}
+    ${defaultProps.states[3].errorMessages[0]} | ${1}
+  `('displays table error message "$errorMessage"', ({ errorMessage, lineNumber }) => {
+    const states = wrapper.findAll('[data-testid="terraform-states-table-error"]');
+    const state = states.at(lineNumber);
+
+    expect(state.text()).toBe(errorMessage);
+  });
+
   describe('when user is a terraform administrator', () => {
     beforeEach(() => {
       return createComponent({

spec/frontend/terraform/components/terraform_list_spec.js

@@ -27,6 +27,15 @@ describe('TerraformList', () => {
       },
     };
 
+    // Override @client  _showDetails
+    getStatesQuery.getStates.definitions[1].selectionSet.selections[0].directives = [];
+
+    // Override @client errorMessages
+    getStatesQuery.getStates.definitions[1].selectionSet.selections[1].directives = [];
+
+    // Override @client loadingActions
+    getStatesQuery.getStates.definitions[1].selectionSet.selections[2].directives = [];
+
     const statsQueryResponse = queryResponse || jest.fn().mockResolvedValue(apolloQueryResponse);
     const apolloProvider = createMockApollo([[getStatesQuery, statsQueryResponse]]);
 
@@ -52,20 +61,26 @@ describe('TerraformList', () => {
     describe('when there is a list of terraform states', () => {
       const states = [
         {
+          _showDetails: false,
+          errorMessages: [],
           id: 'gid://gitlab/Terraform::State/1',
           name: 'state-1',
+          latestVersion: null,
+          loadingActions: false,
           lockedAt: null,
-          updatedAt: null,
           lockedByUser: null,
-          latestVersion: null,
+          updatedAt: null,
         },
         {
+          _showDetails: false,
+          errorMessages: [],
           id: 'gid://gitlab/Terraform::State/2',
           name: 'state-2',
+          latestVersion: null,
+          loadingActions: false,
           lockedAt: null,
-          updatedAt: null,
           lockedByUser: null,
-          latestVersion: null,
+          updatedAt: null,
         },
       ];