From ca27d948ba75faaaf3b4d682dd9ec3f81ffa3670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Fri, 5 Aug 2022 10:53:57 -0400 Subject: [PATCH 1/2] Add group CI/CD variables graphql This commit adds the graphql app for group CI/CD variables and the corresponding specs. --- .../components/ci_group_variables.vue | 104 ++++++++++ .../group_add_variable.mutation.graphql | 30 +++ .../group_delete_variable.mutation.graphql | 30 +++ .../group_update_variable.mutation.graphql | 30 +++ .../queries/group_variables.query.graphql | 17 ++ .../ci_variable_list/graphql/resolvers.js | 48 ++++- .../javascripts/ci_variable_list/index.js | 7 +- .../groups/settings/ci_cd_controller.rb | 3 + spec/features/group_variables_spec.rb | 10 +- .../components/ci_group_variables_spec.js | 183 ++++++++++++++++++ spec/frontend/ci_variable_list/mocks.js | 14 +- 11 files changed, 459 insertions(+), 17 deletions(-) create mode 100644 app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue create mode 100644 app/assets/javascripts/ci_variable_list/graphql/mutations/group_add_variable.mutation.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/mutations/group_delete_variable.mutation.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/mutations/group_update_variable.mutation.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/queries/group_variables.query.graphql create mode 100644 spec/frontend/ci_variable_list/components/ci_group_variables_spec.js 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 new file mode 100644 index 00000000000000..7aac26601b6c91 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/ci_group_variables.vue @@ -0,0 +1,104 @@ + + + diff --git a/app/assets/javascripts/ci_variable_list/graphql/mutations/group_add_variable.mutation.graphql b/app/assets/javascripts/ci_variable_list/graphql/mutations/group_add_variable.mutation.graphql new file mode 100644 index 00000000000000..f8e4dc55fa4858 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/mutations/group_add_variable.mutation.graphql @@ -0,0 +1,30 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +mutation addGroupVariable( + $variable: CiVariable! + $endpoint: String! + $fullPath: ID! + $groupId: ID! +) { + addGroupVariable( + variable: $variable + endpoint: $endpoint + fullPath: $fullPath + groupId: $groupId + ) @client { + group { + id + ciVariables { + nodes { + ...BaseCiVariable + ... on CiGroupVariable { + environmentScope + masked + protected + } + } + } + } + errors + } +} diff --git a/app/assets/javascripts/ci_variable_list/graphql/mutations/group_delete_variable.mutation.graphql b/app/assets/javascripts/ci_variable_list/graphql/mutations/group_delete_variable.mutation.graphql new file mode 100644 index 00000000000000..310e4a6e55123a --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/mutations/group_delete_variable.mutation.graphql @@ -0,0 +1,30 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +mutation deleteGroupVariable( + $variable: CiVariable! + $endpoint: String! + $fullPath: ID! + $groupId: ID! +) { + deleteGroupVariable( + variable: $variable + endpoint: $endpoint + fullPath: $fullPath + groupId: $groupId + ) @client { + group { + id + ciVariables { + nodes { + ...BaseCiVariable + ... on CiGroupVariable { + environmentScope + masked + protected + } + } + } + } + errors + } +} diff --git a/app/assets/javascripts/ci_variable_list/graphql/mutations/group_update_variable.mutation.graphql b/app/assets/javascripts/ci_variable_list/graphql/mutations/group_update_variable.mutation.graphql new file mode 100644 index 00000000000000..5291942eb87e91 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/mutations/group_update_variable.mutation.graphql @@ -0,0 +1,30 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +mutation updateGroupVariable( + $variable: CiVariable! + $endpoint: String! + $fullPath: ID! + $groupId: ID! +) { + updateGroupVariable( + variable: $variable + endpoint: $endpoint + fullPath: $fullPath + groupId: $groupId + ) @client { + group { + id + ciVariables { + nodes { + ...BaseCiVariable + ... on CiGroupVariable { + environmentScope + masked + protected + } + } + } + } + errors + } +} 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 new file mode 100644 index 00000000000000..c6dd6d4faafbad --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/queries/group_variables.query.graphql @@ -0,0 +1,17 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +query getGroupVariables($fullPath: ID!) { + group(fullPath: $fullPath) { + id + ciVariables { + nodes { + ...BaseCiVariable + ... on CiGroupVariable { + environmentScope + masked + protected + } + } + } + } +} diff --git a/app/assets/javascripts/ci_variable_list/graphql/resolvers.js b/app/assets/javascripts/ci_variable_list/graphql/resolvers.js index 7b57e97a4b8bc0..be7e3f88cfd4fc 100644 --- a/app/assets/javascripts/ci_variable_list/graphql/resolvers.js +++ b/app/assets/javascripts/ci_variable_list/graphql/resolvers.js @@ -4,8 +4,9 @@ import { convertObjectPropsToSnakeCase, } from '../../lib/utils/common_utils'; import { getIdFromGraphQLId } from '../../graphql_shared/utils'; -import { instanceString } from '../constants'; +import { GRAPHQL_GROUP_TYPE, groupString, instanceString } from '../constants'; import getAdminVariables from './queries/variables.query.graphql'; +import getGroupVariables from './queries/group_variables.query.graphql'; const prepareVariableForApi = ({ variable, destroy = false }) => { return { @@ -27,6 +28,20 @@ const mapVariableTypes = (variables = [], kind) => { }); }; +const prepareGroupGraphQLResponse = ({ data, groupId, errors = [] }) => { + return { + errors, + group: { + __typename: GRAPHQL_GROUP_TYPE, + id: groupId, + ciVariables: { + __typename: 'CiVariableConnection', + nodes: mapVariableTypes(data.variables, groupString), + }, + }, + }; +}; + const prepareAdminGraphQLResponse = ({ data, errors = [] }) => { return { errors, @@ -37,6 +52,28 @@ const prepareAdminGraphQLResponse = ({ data, errors = [] }) => { }; }; +const callGroupEndpoint = async ({ + endpoint, + fullPath, + variable, + groupId, + cache, + destroy = false, +}) => { + try { + const { data } = await axios.patch(endpoint, { + variables_attributes: [prepareVariableForApi({ variable, destroy })], + }); + return prepareGroupGraphQLResponse({ data, groupId }); + } catch (e) { + return prepareGroupGraphQLResponse({ + data: cache.readQuery({ query: getGroupVariables, variables: { fullPath } }), + groupId, + errors: [...e.response.data], + }); + } +}; + const callAdminEndpoint = async ({ endpoint, variable, cache, destroy = false }) => { try { const { data } = await axios.patch(endpoint, { @@ -54,6 +91,15 @@ const callAdminEndpoint = async ({ endpoint, variable, cache, destroy = false }) export const resolvers = { Mutation: { + addGroupVariable: async (_, { endpoint, fullPath, variable, groupId }, { cache }) => { + return callGroupEndpoint({ endpoint, fullPath, variable, groupId, cache }); + }, + updateGroupVariable: async (_, { endpoint, fullPath, variable, groupId }, { cache }) => { + return callGroupEndpoint({ endpoint, fullPath, variable, groupId, cache }); + }, + deleteGroupVariable: async (_, { endpoint, fullPath, variable, groupId }, { cache }) => { + return callGroupEndpoint({ endpoint, fullPath, variable, groupId, cache, destroy: true }); + }, addAdminVariable: async (_, { endpoint, variable }, { cache }) => { return callAdminEndpoint({ endpoint, variable, cache }); }, diff --git a/app/assets/javascripts/ci_variable_list/index.js b/app/assets/javascripts/ci_variable_list/index.js index 713a453561e433..a74af8aed1231a 100644 --- a/app/assets/javascripts/ci_variable_list/index.js +++ b/app/assets/javascripts/ci_variable_list/index.js @@ -3,6 +3,7 @@ import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; import { parseBoolean } from '~/lib/utils/common_utils'; import CiAdminVariables from './components/ci_admin_variables.vue'; +import CiGroupVariables from './components/ci_group_variables.vue'; import LegacyCiVariableSettings from './components/legacy_ci_variable_settings.vue'; import { resolvers } from './graphql/resolvers'; import createStore from './store'; @@ -32,7 +33,11 @@ const mountCiVariableListApp = (containerEl) => { const parsedIsGroup = parseBoolean(isGroup); const isProtectedByDefault = parseBoolean(protectedByDefault); - const component = CiAdminVariables; + let component = CiAdminVariables; + + if (parsedIsGroup) { + component = CiGroupVariables; + } Vue.use(VueApollo); diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index b1afac1f1c701c..e164a834519b45 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -10,6 +10,9 @@ class CiCdController < Groups::ApplicationController before_action :define_variables, only: [:show] before_action :push_licensed_features, only: [:show] before_action :assign_variables_to_gon, only: [:show] + before_action do + push_frontend_feature_flag(:ci_variable_settings_graphql, @group) + end feature_category :continuous_integration urgency :low diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb index 9af9baeb5bb0e9..ab24162ad5a5b4 100644 --- a/spec/features/group_variables_spec.rb +++ b/spec/features/group_variables_spec.rb @@ -23,7 +23,11 @@ it_behaves_like 'variable list' end - # TODO: Uncomment when the new graphQL app for variable settings - # is enabled. - # it_behaves_like 'variable list' + context 'with enabled ff `ci_variable_settings_graphql' do + before do + visit page_path + end + + it_behaves_like 'variable list' + end end 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 new file mode 100644 index 00000000000000..e45656acfd8ae2 --- /dev/null +++ b/spec/frontend/ci_variable_list/components/ci_group_variables_spec.js @@ -0,0 +1,183 @@ +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; +import { GlLoadingIcon, GlTable } from '@gitlab/ui'; +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 { convertToGraphQLId } from '~/graphql_shared/utils'; + +import ciGroupVariables from '~/ci_variable_list/components/ci_group_variables.vue'; +import ciVariableSettings from '~/ci_variable_list/components/ci_variable_settings.vue'; +import ciVariableTable from '~/ci_variable_list/components/ci_variable_table.vue'; +import getGroupVariables from '~/ci_variable_list/graphql/queries/group_variables.query.graphql'; + +import addGroupVariable from '~/ci_variable_list/graphql/mutations/group_add_variable.mutation.graphql'; +import deleteGroupVariable from '~/ci_variable_list/graphql/mutations/group_delete_variable.mutation.graphql'; +import updateGroupVariable from '~/ci_variable_list/graphql/mutations/group_update_variable.mutation.graphql'; + +import { genericMutationErrorText, variableFetchErrorText } from '~/ci_variable_list/constants'; + +import { mockGroupVariables, newVariable } from '../mocks'; + +jest.mock('~/flash'); + +Vue.use(VueApollo); + +const mockProvide = { + endpoint: '/variables', + groupPath: '/namespace/group', + groupId: 1, +}; + +describe('Ci Group Variable list', () => { + let wrapper; + + let mockApollo; + let mockVariables; + + const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const findCiTable = () => wrapper.findComponent(GlTable); + const findCiSettings = () => wrapper.findComponent(ciVariableSettings); + + // eslint-disable-next-line consistent-return + const createComponentWithApollo = async ({ isLoading = false } = {}) => { + const handlers = [[getGroupVariables, mockVariables]]; + + mockApollo = createMockApollo(handlers, resolvers); + + wrapper = shallowMount(ciGroupVariables, { + provide: mockProvide, + apolloProvider: mockApollo, + stubs: { ciVariableSettings, ciVariableTable }, + }); + + if (!isLoading) { + return waitForPromises(); + } + }; + + beforeEach(() => { + mockVariables = jest.fn(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('while queries are being fetch', () => { + beforeEach(() => { + createComponentWithApollo({ isLoading: true }); + }); + + it('shows a loading icon', () => { + expect(findLoadingIcon().exists()).toBe(true); + expect(findCiTable().exists()).toBe(false); + }); + }); + + describe('when queries are resolved', () => { + describe('successfuly', () => { + beforeEach(async () => { + mockVariables.mockResolvedValue(mockGroupVariables); + + await createComponentWithApollo(); + }); + + it('passes down the expected environments as props', () => { + expect(findCiSettings().props('environments')).toEqual([]); + }); + + it('passes down the expected variables as props', () => { + expect(findCiSettings().props('variables')).toEqual( + mockGroupVariables.data.group.ciVariables.nodes, + ); + }); + + it('createFlash was not called', () => { + expect(createFlash).not.toHaveBeenCalled(); + }); + }); + + describe('with an error for variables', () => { + beforeEach(async () => { + mockVariables.mockRejectedValue(); + + await createComponentWithApollo(); + }); + + it('calls createFlash with the expected error message', () => { + expect(createFlash).toHaveBeenCalledWith({ message: variableFetchErrorText }); + }); + }); + }); + + describe('mutations', () => { + beforeEach(async () => { + mockVariables.mockResolvedValue(mockGroupVariables); + + await createComponentWithApollo(); + }); + it.each` + actionName | mutation | event + ${'add'} | ${addGroupVariable} | ${'add-variable'} + ${'update'} | ${updateGroupVariable} | ${'update-variable'} + ${'delete'} | ${deleteGroupVariable} | ${'delete-variable'} + `( + 'calls the right mutation when user performs $actionName variable', + async ({ event, mutation }) => { + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue(); + await findCiSettings().vm.$emit(event, newVariable); + + expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({ + mutation, + variables: { + endpoint: mockProvide.endpoint, + fullPath: mockProvide.groupPath, + groupId: convertToGraphQLId('Group', mockProvide.groupId), + variable: newVariable, + }, + }); + }, + ); + + it.each` + actionName | event | mutationName + ${'add'} | ${'add-variable'} | ${'addGroupVariable'} + ${'update'} | ${'update-variable'} | ${'updateGroupVariable'} + ${'delete'} | ${'delete-variable'} | ${'deleteGroupVariable'} + `( + 'throws with the specific graphql error if present when user performs $actionName variable', + async ({ event, mutationName }) => { + const graphQLErrorMessage = 'There is a problem with this graphQL action'; + jest + .spyOn(wrapper.vm.$apollo, 'mutate') + .mockResolvedValue({ data: { [mutationName]: { errors: [graphQLErrorMessage] } } }); + await findCiSettings().vm.$emit(event, newVariable); + await nextTick(); + + expect(wrapper.vm.$apollo.mutate).toHaveBeenCalled(); + expect(createFlash).toHaveBeenCalledWith({ message: graphQLErrorMessage }); + }, + ); + + it.each` + actionName | event + ${'add'} | ${'add-variable'} + ${'update'} | ${'update-variable'} + ${'delete'} | ${'delete-variable'} + `( + 'throws generic error when the mutation fails with no graphql errors and user performs $actionName variable', + async ({ event }) => { + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockImplementationOnce(() => { + throw new Error(); + }); + await findCiSettings().vm.$emit(event, newVariable); + + expect(wrapper.vm.$apollo.mutate).toHaveBeenCalled(); + expect(createFlash).toHaveBeenCalledWith({ message: genericMutationErrorText }); + }, + ); + }); +}); diff --git a/spec/frontend/ci_variable_list/mocks.js b/spec/frontend/ci_variable_list/mocks.js index 07dc7a8c91f1bd..89ba77858dcc45 100644 --- a/spec/frontend/ci_variable_list/mocks.js +++ b/spec/frontend/ci_variable_list/mocks.js @@ -1,4 +1,4 @@ -import { variableTypes, instanceString } from '~/ci_variable_list/constants'; +import { variableTypes, groupString, instanceString } from '~/ci_variable_list/constants'; export const devName = 'dev'; export const prodName = 'prod'; @@ -82,22 +82,12 @@ export const mockProjectVariables = { }, }; -export const mockGroupEnvironments = { - data: { - group: { - __typename: 'Group', - id: 1, - environments: defaultEnvs, - }, - }, -}; - export const mockGroupVariables = { data: { group: { __typename: 'Group', id: 1, - ciVariables: createDefaultVars(), + ciVariables: createDefaultVars({ kind: groupString }), }, }, }; -- GitLab From 37c392ea427f072199889a072dc9162461899fe4 Mon Sep 17 00:00:00 2001 From: Mireya Andres Date: Wed, 17 Aug 2022 13:18:49 +0000 Subject: [PATCH 2/2] Apply 1 suggestion(s) to 1 file(s) --- .../ci_variable_list/components/ci_group_variables.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7aac26601b6c91..3af83ffa8ed94f 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 @@ -36,7 +36,7 @@ export default { }; }, update(data) { - return data?.group?.ciVariables?.nodes; + return data?.group?.ciVariables?.nodes || []; }, error() { createFlash({ message: variableFetchErrorText }); -- GitLab