From 26f86e5fa353532b8b9f86f793aefa0d49026ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Fri, 5 Aug 2022 11:08:14 -0400 Subject: [PATCH 1/2] Add graphql CI/CD project variables The project section is added to the graphql refactor of the CI/CD variables. --- .../components/ci_project_variables.vue | 120 ++++++++++ .../add_project_environment.mutation.graphql | 3 + .../project_add_variable.mutation.graphql | 30 +++ .../project_delete_variable.mutation.graphql | 30 +++ .../project_update_variable.mutation.graphql | 30 +++ .../project_environments.query.graphql | 11 + .../queries/project_variables.query.graphql | 15 ++ .../ci_variable_list/graphql/resolvers.js | 56 ++++- .../javascripts/ci_variable_list/index.js | 3 + spec/features/project_variables_spec.rb | 30 ++- .../components/ci_project_variables_spec.js | 217 ++++++++++++++++++ spec/frontend/ci_variable_list/mocks.js | 9 +- 12 files changed, 548 insertions(+), 6 deletions(-) create mode 100644 app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue create mode 100644 app/assets/javascripts/ci_variable_list/graphql/mutations/client/add_project_environment.mutation.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/mutations/project_add_variable.mutation.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/mutations/project_delete_variable.mutation.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/mutations/project_update_variable.mutation.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/queries/project_environments.query.graphql create mode 100644 app/assets/javascripts/ci_variable_list/graphql/queries/project_variables.query.graphql create mode 100644 spec/frontend/ci_variable_list/components/ci_project_variables_spec.js 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 new file mode 100644 index 00000000000000..2ec7307862f5bc --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/ci_project_variables.vue @@ -0,0 +1,120 @@ + + + diff --git a/app/assets/javascripts/ci_variable_list/graphql/mutations/client/add_project_environment.mutation.graphql b/app/assets/javascripts/ci_variable_list/graphql/mutations/client/add_project_environment.mutation.graphql new file mode 100644 index 00000000000000..45109762e80764 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/mutations/client/add_project_environment.mutation.graphql @@ -0,0 +1,3 @@ +mutation addProjectEnvironment($environment: CiEnvironment, $fullPath: ID!) { + addProjectEnvironment(environment: $environment, fullPath: $fullPath) @client +} diff --git a/app/assets/javascripts/ci_variable_list/graphql/mutations/project_add_variable.mutation.graphql b/app/assets/javascripts/ci_variable_list/graphql/mutations/project_add_variable.mutation.graphql new file mode 100644 index 00000000000000..ab3a46da85439c --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/mutations/project_add_variable.mutation.graphql @@ -0,0 +1,30 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +mutation addProjectVariable( + $variable: CiVariable! + $endpoint: String! + $fullPath: ID! + $projectId: ID! +) { + addProjectVariable( + variable: $variable + endpoint: $endpoint + fullPath: $fullPath + projectId: $projectId + ) @client { + project { + id + ciVariables { + nodes { + ...BaseCiVariable + ... on CiProjectVariable { + environmentScope + masked + protected + } + } + } + } + errors + } +} diff --git a/app/assets/javascripts/ci_variable_list/graphql/mutations/project_delete_variable.mutation.graphql b/app/assets/javascripts/ci_variable_list/graphql/mutations/project_delete_variable.mutation.graphql new file mode 100644 index 00000000000000..e83dc9a5e5ec3c --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/mutations/project_delete_variable.mutation.graphql @@ -0,0 +1,30 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +mutation deleteProjectVariable( + $variable: CiVariable! + $endpoint: String! + $fullPath: ID! + $projectId: ID! +) { + deleteProjectVariable( + variable: $variable + endpoint: $endpoint + fullPath: $fullPath + projectId: $projectId + ) @client { + project { + id + ciVariables { + nodes { + ...BaseCiVariable + ... on CiProjectVariable { + environmentScope + masked + protected + } + } + } + } + errors + } +} diff --git a/app/assets/javascripts/ci_variable_list/graphql/mutations/project_update_variable.mutation.graphql b/app/assets/javascripts/ci_variable_list/graphql/mutations/project_update_variable.mutation.graphql new file mode 100644 index 00000000000000..4788911431bbd6 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/mutations/project_update_variable.mutation.graphql @@ -0,0 +1,30 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +mutation updateProjectVariable( + $variable: CiVariable! + $endpoint: String! + $fullPath: ID! + $projectId: ID! +) { + updateProjectVariable( + variable: $variable + endpoint: $endpoint + fullPath: $fullPath + projectId: $projectId + ) @client { + project { + id + ciVariables { + nodes { + ...BaseCiVariable + ... on CiProjectVariable { + environmentScope + masked + protected + } + } + } + } + errors + } +} diff --git a/app/assets/javascripts/ci_variable_list/graphql/queries/project_environments.query.graphql b/app/assets/javascripts/ci_variable_list/graphql/queries/project_environments.query.graphql new file mode 100644 index 00000000000000..921e0ca25b935e --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/queries/project_environments.query.graphql @@ -0,0 +1,11 @@ +query getProjectEnvironments($fullPath: ID!) { + project(fullPath: $fullPath) { + id + environments { + nodes { + id + name + } + } + } +} 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 new file mode 100644 index 00000000000000..a60a50e4bc4681 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/graphql/queries/project_variables.query.graphql @@ -0,0 +1,15 @@ +#import "~/ci_variable_list/graphql/fragments/ci_variable.fragment.graphql" + +query getProjectVariables($fullPath: ID!) { + project(fullPath: $fullPath) { + id + ciVariables { + nodes { + ...BaseCiVariable + 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 be7e3f88cfd4fc..c041531ae308ee 100644 --- a/app/assets/javascripts/ci_variable_list/graphql/resolvers.js +++ b/app/assets/javascripts/ci_variable_list/graphql/resolvers.js @@ -4,9 +4,16 @@ import { convertObjectPropsToSnakeCase, } from '../../lib/utils/common_utils'; import { getIdFromGraphQLId } from '../../graphql_shared/utils'; -import { GRAPHQL_GROUP_TYPE, groupString, instanceString } from '../constants'; -import getAdminVariables from './queries/variables.query.graphql'; +import { + GRAPHQL_GROUP_TYPE, + GRAPHQL_PROJECT_TYPE, + groupString, + instanceString, + projectString, +} from '../constants'; +import getProjectVariables from './queries/project_variables.query.graphql'; import getGroupVariables from './queries/group_variables.query.graphql'; +import getAdminVariables from './queries/variables.query.graphql'; const prepareVariableForApi = ({ variable, destroy = false }) => { return { @@ -28,6 +35,20 @@ const mapVariableTypes = (variables = [], kind) => { }); }; +const prepareProjectGraphQLResponse = ({ data, projectId, errors = [] }) => { + return { + errors, + project: { + __typename: GRAPHQL_PROJECT_TYPE, + id: projectId, + ciVariables: { + __typename: 'CiVariableConnection', + nodes: mapVariableTypes(data.variables, projectString), + }, + }, + }; +}; + const prepareGroupGraphQLResponse = ({ data, groupId, errors = [] }) => { return { errors, @@ -52,6 +73,28 @@ const prepareAdminGraphQLResponse = ({ data, errors = [] }) => { }; }; +const callProjectEndpoint = async ({ + endpoint, + fullPath, + variable, + projectId, + cache, + destroy = false, +}) => { + try { + const { data } = await axios.patch(endpoint, { + variables_attributes: [prepareVariableForApi({ variable, destroy })], + }); + return prepareProjectGraphQLResponse({ data, projectId }); + } catch (e) { + return prepareProjectGraphQLResponse({ + data: cache.readQuery({ query: getProjectVariables, variables: { fullPath } }), + projectId, + errors: [...e.response.data], + }); + } +}; + const callGroupEndpoint = async ({ endpoint, fullPath, @@ -91,6 +134,15 @@ const callAdminEndpoint = async ({ endpoint, variable, cache, destroy = false }) export const resolvers = { Mutation: { + addProjectVariable: async (_, { endpoint, fullPath, variable, projectId }, { cache }) => { + return callProjectEndpoint({ endpoint, fullPath, variable, projectId, cache }); + }, + updateProjectVariable: async (_, { endpoint, fullPath, variable, projectId }, { cache }) => { + return callProjectEndpoint({ endpoint, fullPath, variable, projectId, cache }); + }, + deleteProjectVariable: async (_, { endpoint, fullPath, variable, projectId }, { cache }) => { + return callProjectEndpoint({ endpoint, fullPath, variable, projectId, cache, destroy: true }); + }, addGroupVariable: async (_, { endpoint, fullPath, variable, groupId }, { cache }) => { return callGroupEndpoint({ endpoint, fullPath, variable, groupId, cache }); }, diff --git a/app/assets/javascripts/ci_variable_list/index.js b/app/assets/javascripts/ci_variable_list/index.js index a74af8aed1231a..9aef2e08ea4ee8 100644 --- a/app/assets/javascripts/ci_variable_list/index.js +++ b/app/assets/javascripts/ci_variable_list/index.js @@ -4,6 +4,7 @@ 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 CiProjectVariables from './components/ci_project_variables.vue'; import LegacyCiVariableSettings from './components/legacy_ci_variable_settings.vue'; import { resolvers } from './graphql/resolvers'; import createStore from './store'; @@ -37,6 +38,8 @@ const mountCiVariableListApp = (containerEl) => { if (parsedIsGroup) { component = CiGroupVariables; + } else if (parsedIsProject) { + component = CiProjectVariables; } Vue.use(VueApollo); diff --git a/spec/features/project_variables_spec.rb b/spec/features/project_variables_spec.rb index 89dbd1afc6bc13..d3bedbf3a75574 100644 --- a/spec/features/project_variables_spec.rb +++ b/spec/features/project_variables_spec.rb @@ -14,8 +14,6 @@ project.variables << variable end - # TODO: Add same tests but with FF enabled context when - # the new graphQL app for variable settings is enabled. context 'with disabled ff `ci_variable_settings_graphql' do before do stub_feature_flags(ci_variable_settings_graphql: false) @@ -44,4 +42,32 @@ end end end + + context 'with enabled ff `ci_variable_settings_graphql' do + before do + visit page_path + end + + it_behaves_like 'variable list' + + it 'adds a new variable with an environment scope' do + click_button('Add variable') + + page.within('#add-ci-variable') do + fill_in 'Key', with: 'akey' + find('#ci-variable-value').set('akey_value') + find('[data-testid="environment-scope"]').click + find('[data-testid="ci-environment-search"]').set('review/*') + find('[data-testid="create-wildcard-button"]').click + + click_button('Add variable') + end + + wait_for_requests + + page.within('[data-testid="ci-variable-table"]') do + expect(find('.js-ci-variable-row:first-child [data-label="Environments"]').text).to eq('review/*') + end + end + end end 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 new file mode 100644 index 00000000000000..6bfaf3bcb5706e --- /dev/null +++ b/spec/frontend/ci_variable_list/components/ci_project_variables_spec.js @@ -0,0 +1,217 @@ +import { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; +import { GlLoadingIcon, GlTable } 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 createFlash from '~/flash'; +import { resolvers } from '~/ci_variable_list/graphql/resolvers'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; + +import ciProjectVariables from '~/ci_variable_list/components/ci_project_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 getProjectEnvironments from '~/ci_variable_list/graphql/queries/project_environments.query.graphql'; +import getProjectVariables from '~/ci_variable_list/graphql/queries/project_variables.query.graphql'; + +import addProjectVariable from '~/ci_variable_list/graphql/mutations/project_add_variable.mutation.graphql'; +import deleteProjectVariable from '~/ci_variable_list/graphql/mutations/project_delete_variable.mutation.graphql'; +import updateProjectVariable from '~/ci_variable_list/graphql/mutations/project_update_variable.mutation.graphql'; + +import { + environmentFetchErrorText, + genericMutationErrorText, + variableFetchErrorText, +} from '~/ci_variable_list/constants'; + +import { + devName, + mockProjectEnvironments, + mockProjectVariables, + newVariable, + prodName, +} from '../mocks'; + +jest.mock('~/flash'); + +const localVue = createLocalVue(); +localVue.use(VueApollo); + +const mockProvide = { + endpoint: '/variables', + projectFullPath: '/namespace/project', + projectId: 1, +}; + +describe('Ci Project Variable list', () => { + let wrapper; + + let mockApollo; + let mockEnvironments; + 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 = [ + [getProjectEnvironments, mockEnvironments], + [getProjectVariables, mockVariables], + ]; + + mockApollo = createMockApollo(handlers, resolvers); + + wrapper = shallowMount(ciProjectVariables, { + provide: mockProvide, + apolloProvider: mockApollo, + stubs: { ciVariableSettings, ciVariableTable }, + localVue, + }); + + if (!isLoading) { + return waitForPromises(); + } + }; + + beforeEach(() => { + mockEnvironments = jest.fn(); + 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 () => { + mockEnvironments.mockResolvedValue(mockProjectEnvironments); + mockVariables.mockResolvedValue(mockProjectVariables); + + await createComponentWithApollo(); + }); + + it('passes down the expected environments as props', () => { + expect(findCiSettings().props('environments')).toEqual([prodName, devName]); + }); + + it('passes down the expected variables as props', () => { + expect(findCiSettings().props('variables')).toEqual( + mockProjectVariables.data.project.ciVariables.nodes, + ); + }); + + it('createFlash was not called', () => { + expect(createFlash).not.toHaveBeenCalled(); + }); + }); + + describe('with an error for variables', () => { + beforeEach(async () => { + mockEnvironments.mockResolvedValue(mockProjectEnvironments); + mockVariables.mockRejectedValue(); + + await createComponentWithApollo(); + }); + + it('calls createFlash with the expected error message', () => { + expect(createFlash).toHaveBeenCalledWith({ message: variableFetchErrorText }); + }); + }); + + describe('with an error for environments', () => { + beforeEach(async () => { + mockEnvironments.mockRejectedValue(); + mockVariables.mockResolvedValue(mockProjectVariables); + + await createComponentWithApollo(); + }); + + it('calls createFlash with the expected error message', () => { + expect(createFlash).toHaveBeenCalledWith({ message: environmentFetchErrorText }); + }); + }); + }); + + describe('mutations', () => { + beforeEach(async () => { + mockEnvironments.mockResolvedValue(mockProjectEnvironments); + mockVariables.mockResolvedValue(mockProjectVariables); + + await createComponentWithApollo(); + }); + it.each` + actionName | mutation | event + ${'add'} | ${addProjectVariable} | ${'add-variable'} + ${'update'} | ${updateProjectVariable} | ${'update-variable'} + ${'delete'} | ${deleteProjectVariable} | ${'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.projectFullPath, + projectId: convertToGraphQLId('Project', mockProvide.projectId), + variable: newVariable, + }, + }); + }, + ); + + it.each` + actionName | event | mutationName + ${'add'} | ${'add-variable'} | ${'addProjectVariable'} + ${'update'} | ${'update-variable'} | ${'updateProjectVariable'} + ${'delete'} | ${'delete-variable'} | ${'deleteProjectVariable'} + `( + '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 89ba77858dcc45..b030005cb8d830 100644 --- a/spec/frontend/ci_variable_list/mocks.js +++ b/spec/frontend/ci_variable_list/mocks.js @@ -1,4 +1,9 @@ -import { variableTypes, groupString, instanceString } from '~/ci_variable_list/constants'; +import { + variableTypes, + groupString, + instanceString, + projectString, +} from '~/ci_variable_list/constants'; export const devName = 'dev'; export const prodName = 'prod'; @@ -77,7 +82,7 @@ export const mockProjectVariables = { project: { __typename: 'Project', id: 1, - ciVariables: createDefaultVars(), + ciVariables: createDefaultVars({ kind: projectString }), }, }, }; -- GitLab From e7b837ef4762307d34d60938e0199d869faf2f8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Thu, 18 Aug 2022 09:08:07 -0400 Subject: [PATCH 2/2] Remove deprecated localVue --- .../components/ci_project_variables_spec.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 6bfaf3bcb5706e..867f8e0cf8fddb 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 @@ -1,7 +1,7 @@ -import { nextTick } from 'vue'; +import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import { GlLoadingIcon, GlTable } from '@gitlab/ui'; -import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import createFlash from '~/flash'; @@ -34,8 +34,7 @@ import { jest.mock('~/flash'); -const localVue = createLocalVue(); -localVue.use(VueApollo); +Vue.use(VueApollo); const mockProvide = { endpoint: '/variables', @@ -67,7 +66,6 @@ describe('Ci Project Variable list', () => { provide: mockProvide, apolloProvider: mockApollo, stubs: { ciVariableSettings, ciVariableTable }, - localVue, }); if (!isLoading) { -- GitLab