diff --git a/app/assets/javascripts/ci/pipeline_new/components/pipeline_new_form.vue b/app/assets/javascripts/ci/pipeline_new/components/pipeline_new_form.vue index 8bdd064bbf19b06633943ddfdb3d5aefefaef5b5..d314becac71153a6ed5a1db61fbe9359c36db085 100644 --- a/app/assets/javascripts/ci/pipeline_new/components/pipeline_new_form.vue +++ b/app/assets/javascripts/ci/pipeline_new/components/pipeline_new_form.vue @@ -19,6 +19,7 @@ import { fetchPolicies } from '~/lib/graphql'; import SafeHtml from '~/vue_shared/directives/safe_html'; import { visitUrl } from '~/lib/utils/url_utility'; import { s__, __, n__ } from '~/locale'; +import { createAlert } from '~/alert'; import { helpPagePath } from '~/helpers/help_page_helper'; import { IDENTITY_VERIFICATION_REQUIRED_ERROR, @@ -348,35 +349,45 @@ export default { }, async createPipeline() { this.submitted = true; + try { + const { + data: { + pipelineCreate: { errors, pipeline }, + }, + } = await this.$apollo.mutate({ + mutation: createPipelineMutation, + variables: { + input: { + projectPath: this.projectPath, + ref: this.refShortName, + variables: filterVariables(this.variables), + }, + }, + }); - const { data } = await this.$apollo.mutate({ - mutation: createPipelineMutation, - variables: { - endpoint: this.pipelinesPath, - // send shortName as fall back for query params - // https://gitlab.com/gitlab-org/gitlab/-/issues/287815 - ref: this.refQueryParam, - variablesAttributes: filterVariables(this.variables), - }, - }); + const pipelineErrors = pipeline?.errorMessages?.nodes?.map((node) => node?.content) || ''; + const totalWarnings = pipeline?.warningMessages?.nodes?.length || 0; - const { id, errors, totalWarnings, warnings } = data.createPipeline; + if (pipeline?.path) { + visitUrl(pipeline.path); + } else if (errors?.length > 0 || pipelineErrors.length || totalWarnings) { + const warnings = pipeline?.warningMessages?.nodes?.map((node) => node?.content) || ''; + const error = errors[0] || pipelineErrors[0] || ''; - if (id) { - visitUrl(`${this.pipelinesPath}/${id}`); - return; + this.reportError({ + title: i18n.submitErrorTitle, + error, + warnings, + totalWarnings, + }); + } + } catch (error) { + createAlert({ message: i18n.submitErrorTitle }); + Sentry.captureException(error); } // always re-enable submit button this.submitted = false; - const [error] = errors; - - this.reportError({ - title: i18n.submitErrorTitle, - error, - warnings, - totalWarnings, - }); }, onRefsLoadingError(error) { this.reportError({ title: i18n.refsLoadingErrorTitle }); diff --git a/app/assets/javascripts/ci/pipeline_new/graphql/mutations/create_pipeline.mutation.graphql b/app/assets/javascripts/ci/pipeline_new/graphql/mutations/create_pipeline.mutation.graphql index a76e8f6b95b2c8ee46e98db541bec9650cc0208c..f3fea7af37d05b5f85e6b1d8e362e02c172af68b 100644 --- a/app/assets/javascripts/ci/pipeline_new/graphql/mutations/create_pipeline.mutation.graphql +++ b/app/assets/javascripts/ci/pipeline_new/graphql/mutations/create_pipeline.mutation.graphql @@ -1,9 +1,23 @@ -mutation createPipeline($endpoint: String, $ref: String, $variablesAttributes: Array) { - createPipeline(endpoint: $endpoint, ref: $ref, variablesAttributes: $variablesAttributes) - @client { - id +# This name is specifically needed for backend logic +mutation internalPipelineCreate($input: PipelineCreateInput!) { + pipelineCreate(input: $input) { + clientMutationId errors - totalWarnings - warnings + pipeline { + id + path + errorMessages { + nodes { + id + content + } + } + warningMessages { + nodes { + id + content + } + } + } } } diff --git a/app/assets/javascripts/ci/pipeline_new/graphql/resolvers.js b/app/assets/javascripts/ci/pipeline_new/graphql/resolvers.js deleted file mode 100644 index 7b0f58e8cf97e313ee0c8aeffea6965c55c6f6be..0000000000000000000000000000000000000000 --- a/app/assets/javascripts/ci/pipeline_new/graphql/resolvers.js +++ /dev/null @@ -1,29 +0,0 @@ -import axios from '~/lib/utils/axios_utils'; - -export const resolvers = { - Mutation: { - createPipeline: (_, { endpoint, ref, variablesAttributes }) => { - return axios - .post(endpoint, { ref, variables_attributes: variablesAttributes }) - .then((response) => { - const { id } = response.data; - return { - id, - errors: [], - totalWarnings: 0, - warnings: [], - }; - }) - .catch((err) => { - const { errors = [], totalWarnings = 0, warnings = [] } = err.response.data; - - return { - id: null, - errors, - totalWarnings, - warnings, - }; - }); - }, - }, -}; diff --git a/app/assets/javascripts/ci/pipeline_new/index.js b/app/assets/javascripts/ci/pipeline_new/index.js index 73f2ec40ac24552d5d403aa8996eb2a5329e9a90..61d3c0093a7342d70690b880d2eed2b29cf09c84 100644 --- a/app/assets/javascripts/ci/pipeline_new/index.js +++ b/app/assets/javascripts/ci/pipeline_new/index.js @@ -2,7 +2,6 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; import PipelineNewForm from './components/pipeline_new_form.vue'; -import { resolvers } from './graphql/resolvers'; const mountPipelineNewForm = (el) => { const { @@ -31,7 +30,7 @@ const mountPipelineNewForm = (el) => { Vue.use(VueApollo); const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient(resolvers), + defaultClient: createDefaultClient(), }); return new Vue({ diff --git a/app/assets/javascripts/ci/pipeline_new/utils/filter_variables.js b/app/assets/javascripts/ci/pipeline_new/utils/filter_variables.js index 57ce3d13a9ab09638e9b9be5caa459812f8bc8ab..59169b5624c3ce55018ab1ed7a3a58e8a8a3ac7d 100644 --- a/app/assets/javascripts/ci/pipeline_new/utils/filter_variables.js +++ b/app/assets/javascripts/ci/pipeline_new/utils/filter_variables.js @@ -1,13 +1,16 @@ -// We need to filter out blank variables -// and filter out variables that have no key -// before sending to the API to create a pipeline. +/** + * We need to filter out blank variables as well as variables that have no key + * and then format the variables to GraphQL + * before sending to the API to create a pipeline. + */ export default (variables) => { return variables .filter(({ key }) => key !== '') - .map(({ variable_type, key, value }) => ({ - variable_type, + .map(({ key, value, variable_type: variableType }) => ({ key, - secret_value: value, + value, + // CiVariableType must be all caps + variableType: variableType.toUpperCase(), })); }; diff --git a/spec/frontend/ci/pipeline_new/components/identity_verification_required_pipeline_new_form_spec.js b/spec/frontend/ci/pipeline_new/components/identity_verification_required_pipeline_new_form_spec.js index bd1800abab2ee5ad13362ae57be1b28d39b82ec7..da500b2f741f03d2abd9158d88d78a2e6fb8b393 100644 --- a/spec/frontend/ci/pipeline_new/components/identity_verification_required_pipeline_new_form_spec.js +++ b/spec/frontend/ci/pipeline_new/components/identity_verification_required_pipeline_new_form_spec.js @@ -7,10 +7,9 @@ import createMockApollo from 'helpers/mock_apollo_helper'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import axios from '~/lib/utils/axios_utils'; -import { HTTP_STATUS_BAD_REQUEST } from '~/lib/utils/http_status'; import PipelineNewForm from '~/ci/pipeline_new/components/pipeline_new_form.vue'; +import pipelineCreateMutation from '~/ci/pipeline_new/graphql/mutations/create_pipeline.mutation.graphql'; import ciConfigVariablesQuery from '~/ci/pipeline_new/graphql/queries/ci_config_variables.graphql'; -import { resolvers } from '~/ci/pipeline_new/graphql/resolvers'; import { mockIdentityVerificationRequiredError, mockEmptyCiConfigVariablesResponse, @@ -30,6 +29,7 @@ describe('Pipeline New Form', () => { let mockApollo; let mockCiConfigVariables; let dummySubmitEvent; + const pipelineCreateMutationHandler = jest.fn(); const findForm = () => wrapper.findComponent(GlForm); const findErrorAlert = () => wrapper.findByTestId('run-pipeline-error-alert'); @@ -37,8 +37,11 @@ describe('Pipeline New Form', () => { wrapper.findComponent(PipelineAccountVerificationAlert); const createComponentWithApollo = ({ props = {} } = {}) => { - const handlers = [[ciConfigVariablesQuery, mockCiConfigVariables]]; - mockApollo = createMockApollo(handlers, resolvers); + const handlers = [ + [ciConfigVariablesQuery, mockCiConfigVariables], + [pipelineCreateMutation, pipelineCreateMutationHandler], + ]; + mockApollo = createMockApollo(handlers); wrapper = shallowMountExtended(PipelineNewForm, { apolloProvider: mockApollo, @@ -56,6 +59,7 @@ describe('Pipeline New Form', () => { refParam: defaultBranch, settingsLink: '', maxWarnings: 25, + isMaintainer: false, ...props, }, }); @@ -77,13 +81,10 @@ describe('Pipeline New Form', () => { describe('Form errors and warnings', () => { describe('when the error response is identity verification required', () => { beforeEach(async () => { + pipelineCreateMutationHandler.mockResolvedValue(mockIdentityVerificationRequiredError); mockCiConfigVariables.mockResolvedValue(mockEmptyCiConfigVariablesResponse); createComponentWithApollo(); - mock - .onPost(pipelinesPath) - .reply(HTTP_STATUS_BAD_REQUEST, mockIdentityVerificationRequiredError); - findForm().vm.$emit('submit', dummySubmitEvent); await waitForPromises(); diff --git a/spec/frontend/ci/pipeline_new/components/pipeline_new_form_spec.js b/spec/frontend/ci/pipeline_new/components/pipeline_new_form_spec.js index 12042dd4058364d3d734fd3d1db4fc26e743d0ee..762b788fef5b75b06229a389c2f8b63c6705d861 100644 --- a/spec/frontend/ci/pipeline_new/components/pipeline_new_form_spec.js +++ b/spec/frontend/ci/pipeline_new/components/pipeline_new_form_spec.js @@ -1,32 +1,28 @@ import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; -import { GlForm, GlSprintf, GlLoadingIcon, GlIcon } from '@gitlab/ui'; +import { GlForm, GlLoadingIcon, GlIcon } from '@gitlab/ui'; import MockAdapter from 'axios-mock-adapter'; +import mockPipelineCreateMutationResponse from 'test_fixtures/graphql/pipelines/create_pipeline.mutation.graphql.json'; +import mockPipelineCreateMutationErrorResponse from 'test_fixtures/graphql/pipelines/create_pipeline_error.mutation.graphql.json'; import createMockApollo from 'helpers/mock_apollo_helper'; import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import axios from '~/lib/utils/axios_utils'; -import { - HTTP_STATUS_BAD_REQUEST, - HTTP_STATUS_INTERNAL_SERVER_ERROR, - HTTP_STATUS_OK, -} from '~/lib/utils/http_status'; +import { HTTP_STATUS_INTERNAL_SERVER_ERROR } from '~/lib/utils/http_status'; import { visitUrl } from '~/lib/utils/url_utility'; import PipelineNewForm, { POLLING_INTERVAL, } from '~/ci/pipeline_new/components/pipeline_new_form.vue'; import ciConfigVariablesQuery from '~/ci/pipeline_new/graphql/queries/ci_config_variables.graphql'; -import { resolvers } from '~/ci/pipeline_new/graphql/resolvers'; +import pipelineCreateMutation from '~/ci/pipeline_new/graphql/mutations/create_pipeline.mutation.graphql'; import RefsDropdown from '~/ci/pipeline_new/components/refs_dropdown.vue'; import VariableValuesListbox from '~/ci/pipeline_new/components/variable_values_listbox.vue'; import { mockCiConfigVariablesResponse, mockCiConfigVariablesResponseWithoutDesc, mockEmptyCiConfigVariablesResponse, - mockError, mockNoCachedCiConfigVariablesResponse, mockQueryParams, - mockPostParams, mockProjectId, mockYamlVariables, mockPipelineConfigButtonText, @@ -43,7 +39,6 @@ jest.mock('~/lib/utils/url_utility', () => ({ const pipelinesPath = '/root/project/-/pipelines'; const pipelinesEditorPath = '/root/project/-/ci/editor'; const projectPath = '/root/project/-/pipelines/config_variables'; -const newPipelinePostResponse = { id: 1 }; const defaultBranch = 'main'; describe('Pipeline New Form', () => { @@ -53,6 +48,8 @@ describe('Pipeline New Form', () => { let mockCiConfigVariables; let dummySubmitEvent; + const pipelineCreateMutationHandler = jest.fn(); + const findForm = () => wrapper.findComponent(GlForm); const findRefsDropdown = () => wrapper.findComponent(RefsDropdown); const findSubmitButton = () => wrapper.findByTestId('run-pipeline-button'); @@ -66,11 +63,8 @@ describe('Pipeline New Form', () => { const findErrorAlert = () => wrapper.findByTestId('run-pipeline-error-alert'); const findPipelineConfigButton = () => wrapper.findByTestId('ci-cd-pipeline-configuration'); const findWarningAlert = () => wrapper.findByTestId('run-pipeline-warning-alert'); - const findWarningAlertSummary = () => findWarningAlert().findComponent(GlSprintf); - const findWarnings = () => wrapper.findAllByTestId('run-pipeline-warning'); const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findCiCdSettingsLink = () => wrapper.findByTestId('ci-cd-settings-link'); - const getFormPostParams = () => JSON.parse(mock.history.post[0].data); const advanceToNextFetch = (milliseconds) => { jest.advanceTimersByTime(milliseconds); @@ -99,8 +93,11 @@ describe('Pipeline New Form', () => { mountFn = shallowMountExtended, stubs = {}, } = {}) => { - const handlers = [[ciConfigVariablesQuery, mockCiConfigVariables]]; - mockApollo = createMockApollo(handlers, resolvers); + const handlers = [ + [ciConfigVariablesQuery, mockCiConfigVariables], + [pipelineCreateMutation, pipelineCreateMutationHandler], + ]; + mockApollo = createMockApollo(handlers); wrapper = mountFn(PipelineNewForm, { apolloProvider: mockApollo, @@ -208,7 +205,7 @@ describe('Pipeline New Form', () => { describe('Pipeline creation', () => { beforeEach(() => { mockCiConfigVariables.mockResolvedValue(mockEmptyCiConfigVariablesResponse); - mock.onPost(pipelinesPath).reply(HTTP_STATUS_OK, newPipelinePostResponse); + pipelineCreateMutationHandler.mockResolvedValue(mockPipelineCreateMutationResponse); }); it('does not submit the native HTML form', () => { @@ -224,33 +221,45 @@ describe('Pipeline New Form', () => { expect(findSubmitButton().props('disabled')).toBe(false); + await findForm().vm.$emit('submit', dummySubmitEvent); + + expect(findSubmitButton().props('disabled')).toBe(true); + }); + + it('fires the mutation when the submit button is clicked', async () => { + createComponentWithApollo(); + findForm().vm.$emit('submit', dummySubmitEvent); await waitForPromises(); - expect(findSubmitButton().props('disabled')).toBe(true); + expect(pipelineCreateMutationHandler).toHaveBeenCalled(); }); - it('creates pipeline with full ref and variables', async () => { + it('creates pipeline with ref and variables', async () => { createComponentWithApollo(); findForm().vm.$emit('submit', dummySubmitEvent); await waitForPromises(); - expect(getFormPostParams().ref).toEqual(`refs/heads/${defaultBranch}`); - expect(visitUrl).toHaveBeenCalledWith(`${pipelinesPath}/${newPipelinePostResponse.id}`); + expect(pipelineCreateMutationHandler).toHaveBeenCalledWith({ + input: { + ref: 'main', + projectPath: '/root/project/-/pipelines/config_variables', + variables: [], + }, + }); }); - it('creates a pipeline with short ref and variables from the query params', async () => { - createComponentWithApollo({ props: mockQueryParams }); + it('navigates to the created pipeline', async () => { + const pipelinePath = mockPipelineCreateMutationResponse.data.pipelineCreate.pipeline.path; + createComponentWithApollo(); await waitForPromises(); findForm().vm.$emit('submit', dummySubmitEvent); - await waitForPromises(); - expect(getFormPostParams()).toEqual(mockPostParams); - expect(visitUrl).toHaveBeenCalledWith(`${pipelinesPath}/${newPipelinePostResponse.id}`); + expect(visitUrl).toHaveBeenCalledWith(pipelinePath); }); }); @@ -491,38 +500,29 @@ describe('Pipeline New Form', () => { findRefsDropdown().vm.$emit('loadingError'); }); - it('shows both an error alert', () => { + it('shows an error alert', () => { expect(findErrorAlert().exists()).toBe(true); expect(findWarningAlert().exists()).toBe(false); }); }); - describe('when the error response can be handled', () => { + describe('when pipeline creation mutation is not successful', () => { beforeEach(async () => { - mock.onPost(pipelinesPath).reply(HTTP_STATUS_BAD_REQUEST, mockError); + pipelineCreateMutationHandler.mockResolvedValue(mockPipelineCreateMutationErrorResponse); findForm().vm.$emit('submit', dummySubmitEvent); await waitForPromises(); }); - it('shows both error and warning', () => { + it('shows error', () => { expect(findErrorAlert().exists()).toBe(true); - expect(findWarningAlert().exists()).toBe(true); }); it('shows the correct error', () => { - expect(findErrorAlert().text()).toBe(mockError.errors[0]); - }); - - it('shows the correct warning title', () => { - const { length } = mockError.warnings; - - expect(findWarningAlertSummary().attributes('message')).toBe(`${length} warnings found:`); - }); + const error = mockPipelineCreateMutationErrorResponse.data.pipelineCreate.errors[0]; - it('shows the correct amount of warnings', () => { - expect(findWarnings()).toHaveLength(mockError.warnings.length); + expect(findErrorAlert().text()).toBe(error); }); it('re-enables the submit button', () => { diff --git a/spec/frontend/ci/pipeline_new/mock_data.js b/spec/frontend/ci/pipeline_new/mock_data.js index 9ed101b8c8e7fd7ae3bc5886caf8c4a0ebc05625..8a57f675637598f7b2062cb791c31725c3304220 100644 --- a/spec/frontend/ci/pipeline_new/mock_data.js +++ b/spec/frontend/ci/pipeline_new/mock_data.js @@ -38,9 +38,14 @@ export const mockError = { }; export const mockIdentityVerificationRequiredError = { - errors: ['Identity verification is required in order to run CI jobs'], - warnings: [], - total_warnings: 0, + data: { + pipelineCreate: { + clientMutationId: 'test-mutation-id', + errors: ['Identity verification is required in order to run CI jobs'], + pipeline: null, + __typename: 'PipelineCreatePayload', + }, + }, }; export const mockBranchRefs = ['main', 'dev', 'release']; diff --git a/spec/frontend/ci/pipeline_new/utils/filter_variables_spec.js b/spec/frontend/ci/pipeline_new/utils/filter_variables_spec.js index d1b89704b58a6f0278e74c353847b0f00861bbf8..8cd1aca100e3a7fc27bb4a5f5747f898294dbe51 100644 --- a/spec/frontend/ci/pipeline_new/utils/filter_variables_spec.js +++ b/spec/frontend/ci/pipeline_new/utils/filter_variables_spec.js @@ -3,19 +3,19 @@ import { mockVariables } from '../mock_data'; describe('Filter variables utility function', () => { it('filters variables that do not contain a key', () => { - const expectedVaraibles = [ + const expectedVariables = [ { - variable_type: 'env_var', key: 'var_without_value', - secret_value: '', + value: '', + variableType: 'ENV_VAR', }, { - variable_type: 'env_var', key: 'var_with_value', - secret_value: 'test_value', + value: 'test_value', + variableType: 'ENV_VAR', }, ]; - expect(filterVariables(mockVariables)).toEqual(expectedVaraibles); + expect(filterVariables(mockVariables)).toEqual(expectedVariables); }); }); diff --git a/spec/frontend/fixtures/pipeline_create.rb b/spec/frontend/fixtures/pipeline_create.rb new file mode 100644 index 0000000000000000000000000000000000000000..9cb2e551195ffa611b64ff85d49615834048f44b --- /dev/null +++ b/spec/frontend/fixtures/pipeline_create.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'GraphQL Pipeline Mutations', type: :request, feature_category: :pipeline_composition do + include ApiHelpers + include GraphqlHelpers + include JavaScriptFixturesHelpers + + # Project setup + let_it_be(:namespace) { create(:namespace, name: 'frontend-fixtures') } + let_it_be(:project) { create(:project, :repository, namespace: namespace, path: 'pipelines-project') } + let_it_be(:user) { create(:user, developer_of: project) } + + before_all do + project.add_developer(user) + # Ensure we have a valid repository with CI config + project.repository.create_file( + user, + '.gitlab-ci.yml', + 'test: + script: echo "test"', + message: 'Add CI config', + branch_name: 'main' + ) + end + + before do + sign_in(user) + end + + describe GraphQL::Query do + describe 'Pipeline Create Mutation' do + let_it_be(:query_file) { 'create_pipeline.mutation.graphql' } + let_it_be(:query_path) { "ci/pipeline_new/graphql/mutations/" } + let_it_be(:create_mutation) do + get_graphql_query_as_string("#{query_path}#{query_file}") + end + + context 'with valid input' do + let(:input_variables) do + { + input: { + projectPath: project.full_path, + ref: 'main', + clientMutationId: 'test-mutation-id', + variables: [ + { + key: 'var_one', + value: '', + variableType: 'ENV_VAR' + } + ] + } + } + end + + it 'graphql/pipelines/create_pipeline.mutation.graphql.json' do + post_graphql(create_mutation, + current_user: user, + variables: input_variables + ) + + expect_graphql_errors_to_be_empty + end + end + + context 'with empty ref' do + let(:input_variables) do + { + input: { + projectPath: project.full_path, + ref: '', + clientMutationId: 'test-mutation-id', + variables: [] + } + } + end + + it 'graphql/pipelines/create_pipeline_error.mutation.graphql.json' do + post_graphql(create_mutation, + current_user: user, + variables: input_variables + ) + + expect(graphql_data.dig('pipelineCreate', 'errors')).not_to be_empty + end + end + end + end +end