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 865398051f3c6900a989bc75559a6c40c5afd8dd..6b18c9329274c2d7b89ff06c3ccd7dcd27e32b67 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 @@ -147,6 +147,7 @@ export default { form: {}, errorTitle: null, error: null, + pipelineVariables: [], predefinedVariables: null, warnings: [], totalWarnings: 0, @@ -277,6 +278,9 @@ export default { clearTimeout(pollTimeout); this.$apollo.queries.ciConfigVariables.stopPolling(); }, + handleVariablesUpdated(updatedVariables) { + this.pipelineVariables = updatedVariables; + }, populateForm() { this.configVariablesWithDescription = this.predefinedVariables.reduce( (accumulator, { description, key, value, valueOptions }) => { @@ -367,7 +371,9 @@ export default { input: { projectPath: this.projectPath, ref: this.refShortName, - variables: filterVariables(this.variables), + variables: this.isUsingPipelineInputs + ? this.pipelineVariables + : filterVariables(this.variables), }, }, }); @@ -488,8 +494,13 @@ export default { <pipeline-variables-form v-if="isUsingPipelineInputs" :default-branch="defaultBranch" + :file-params="fileParams" + :is-maintainer="isMaintainer" :project-path="projectPath" :ref-param="refParam" + :settings-link="settingsLink" + :variable-params="variableParams" + @variables-updated="handleVariablesUpdated" /> <div v-else> <gl-loading-icon v-if="isLoading" class="gl-mb-5" size="md" /> diff --git a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue index f3a5802b605decd86256bf156e7f53b5d477b486..087450148ad76ac8b095c955f1568735b2b95276 100644 --- a/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue +++ b/app/assets/javascripts/ci/pipeline_new/components/pipeline_variables_form.vue @@ -1,16 +1,68 @@ <script> -import { GlLoadingIcon, GlFormGroup } from '@gitlab/ui'; -import { fetchPolicies } from '~/lib/graphql'; +import { + GlIcon, + GlButton, + GlCollapsibleListbox, + GlFormGroup, + GlFormInput, + GlFormTextarea, + GlLink, + GlLoadingIcon, + GlSprintf, +} from '@gitlab/ui'; +import { uniqueId } from 'lodash'; +import { GlBreakpointInstance } from '@gitlab/ui/dist/utils'; +import { helpPagePath } from '~/helpers/help_page_helper'; +import { s__ } from '~/locale'; import { reportToSentry } from '~/ci/utils'; +import { fetchPolicies } from '~/lib/graphql'; +import filterVariables from '../utils/filter_variables'; +import { + CONFIG_VARIABLES_TIMEOUT, + CI_VARIABLE_TYPE_FILE, + CI_VARIABLE_TYPE_ENV_VAR, +} from '../constants'; import ciConfigVariablesQuery from '../graphql/queries/ci_config_variables.graphql'; +import VariableValuesListbox from './variable_values_listbox.vue'; + +let pollTimeout; +export const POLLING_INTERVAL = 2000; export default { name: 'PipelineVariablesForm', + formElementClasses: 'gl-basis-1/4 gl-shrink-0 gl-flex-grow-0', + learnMorePath: helpPagePath('ci/variables/_index', { + anchor: 'cicd-variable-precedence', + }), + // this height value is used inline on the textarea to match the input field height + // it's used to prevent the overwrite if 'gl-h-7' or '!gl-h-7' were used + textAreaStyle: { height: '32px' }, components: { - GlLoadingIcon, + GlIcon, + GlButton, + GlCollapsibleListbox, GlFormGroup, + GlFormInput, + GlFormTextarea, + GlLink, + GlLoadingIcon, + GlSprintf, + VariableValuesListbox, }, props: { + defaultBranch: { + type: String, + required: true, + }, + fileParams: { + type: Object, + required: false, + default: () => ({}), + }, + isMaintainer: { + type: Boolean, + required: true, + }, projectPath: { type: String, required: true, @@ -19,16 +71,26 @@ export default { type: String, required: true, }, - defaultBranch: { + settingsLink: { type: String, required: true, }, + variableParams: { + type: Object, + required: false, + default: () => ({}), + }, }, data() { return { ciConfigVariables: null, + configVariablesWithDescription: {}, + form: {}, refValue: { shortName: this.refParam, + // this is needed until we add support for ref type in url query strings + // ensure default branch is called with full ref on load + // https://gitlab.com/gitlab-org/gitlab/-/issues/287815 fullName: this.refParam === this.defaultBranch ? `refs/heads/${this.refParam}` : undefined, }, }; @@ -37,6 +99,9 @@ export default { ciConfigVariables: { fetchPolicy: fetchPolicies.NO_CACHE, query: ciConfigVariablesQuery, + skip() { + return Object.keys(this.form).includes(this.refFullName); + }, variables() { return { fullPath: this.projectPath, @@ -46,21 +111,183 @@ export default { update({ project }) { return project?.ciConfigVariables || []; }, + result() { + // API cache is empty when ciConfigVariables === null, so we need to + // poll while cache values are being populated in the backend. + // After CONFIG_VARIABLES_TIMEOUT ms have passed, we stop polling + // and populate the form regardless. + if (this.isFetchingCiConfigVariables && !pollTimeout) { + pollTimeout = setTimeout(() => { + this.ciConfigVariables = []; + this.clearPolling(); + this.populateForm(); + }, CONFIG_VARIABLES_TIMEOUT); + } + if (!this.isFetchingCiConfigVariables) { + this.clearPolling(); + this.populateForm(); + } + }, error(error) { reportToSentry(this.$options.name, error); }, + pollInterval: POLLING_INTERVAL, }, }, computed: { + descriptions() { + return this.form[this.refFullName]?.descriptions ?? {}; + }, isFetchingCiConfigVariables() { return this.ciConfigVariables === null; }, isLoading() { return this.$apollo.queries.ciConfigVariables.loading || this.isFetchingCiConfigVariables; }, + isMobile() { + return ['sm', 'xs'].includes(GlBreakpointInstance.getBreakpointSize()); + }, + refFullName() { + return this.refValue.fullName; + }, refQueryParam() { - return this.refValue.fullName || this.refValue.shortName; + return this.refFullName || this.refShortName; + }, + refShortName() { + return this.refValue.shortName; + }, + removeButtonCategory() { + return this.isMobile ? 'secondary' : 'tertiary'; + }, + variables() { + return this.form[this.refFullName]?.variables ?? []; + }, + variableTypeListboxItems() { + return [ + { + value: CI_VARIABLE_TYPE_ENV_VAR, + text: s__('Pipeline|Variable'), + }, + { + value: CI_VARIABLE_TYPE_FILE, + text: s__('Pipeline|File'), + }, + ]; + }, + }, + watch: { + variables: { + handler(newVariables) { + this.$emit('variables-updated', filterVariables(newVariables)); + }, + deep: true, + }, + }, + methods: { + addEmptyVariable(refValue) { + const { variables } = this.form[refValue]; + + const lastVar = variables[variables.length - 1]; + if (lastVar?.key === '' && lastVar?.value === '') { + return; + } + + variables.push({ + uniqueId: uniqueId(`var-${refValue}`), + variableType: CI_VARIABLE_TYPE_ENV_VAR, + key: '', + value: '', + }); + }, + canRemove(index) { + return index < this.variables.length - 1; + }, + clearPolling() { + clearTimeout(pollTimeout); + this.$apollo.queries.ciConfigVariables.stopPolling(); + }, + createListItemsFromVariableOptions(key) { + return this.configVariablesWithDescription.options[key].map((option) => ({ + text: option, + value: option, + })); + }, + getPipelineAriaLabel(index) { + return `${s__('Pipeline|Variable')} ${index + 1}`; + }, + populateForm() { + this.configVariablesWithDescription = this.ciConfigVariables.reduce( + (accumulator, { description, key, value, valueOptions }) => { + if (description) { + accumulator.descriptions[key] = description; + accumulator.values[key] = value; + accumulator.options[key] = valueOptions; + } + + return accumulator; + }, + { descriptions: {}, values: {}, options: {} }, + ); + + this.form = { + ...this.form, + [this.refFullName]: { + descriptions: this.configVariablesWithDescription.descriptions, + variables: [], + }, + }; + + // Add default variables from yml + this.setVariableParams( + this.refFullName, + CI_VARIABLE_TYPE_ENV_VAR, + this.configVariablesWithDescription.values, + ); + + // Add/update variables, e.g. from query string + if (this.variableParams) { + this.setVariableParams(this.refFullName, CI_VARIABLE_TYPE_ENV_VAR, this.variableParams); + } + + if (this.fileParams) { + this.setVariableParams(this.refFullName, CI_VARIABLE_TYPE_FILE, this.fileParams); + } + + // Adds empty var at the end of the form + this.addEmptyVariable(this.refFullName); + }, + removeVariable(index) { + this.variables.splice(index, 1); + }, + setVariableAttribute(key, attribute, value) { + const { variables } = this.form[this.refFullName]; + const variable = variables.find((v) => v.key === key); + variable[attribute] = value; + }, + setVariable(refValue, { type, key, value }) { + const { variables } = this.form[refValue]; + + const variable = variables.find((v) => v.key === key); + if (variable) { + variable.variableType = type; + variable.value = value; + } else { + variables.push({ + uniqueId: uniqueId(`var-${refValue}`), + key, + value, + variableType: type, + }); + } + }, + setVariableParams(refValue, type, paramsObj) { + Object.entries(paramsObj).forEach(([key, value]) => { + this.setVariable(refValue, { type, key, value }); + }); + }, + shouldShowValuesDropdown(key) { + return this.configVariablesWithDescription.options[key]?.length > 1; }, }, }; @@ -69,8 +296,105 @@ export default { <template> <div> <gl-loading-icon v-if="isLoading" class="gl-mb-5" size="md" /> - <gl-form-group v-else> - <pre>{{ ciConfigVariables }}</pre> + <gl-form-group v-else :label="s__('Pipeline|Variables')"> + <div + v-for="(variable, index) in variables" + :key="variable.uniqueId" + class="gl-mb-4" + data-testid="ci-variable-row-container" + > + <div class="gl-flex gl-flex-col gl-items-stretch gl-gap-4 md:gl-flex-row"> + <gl-collapsible-listbox + :items="variableTypeListboxItems" + :selected="variable.variableType" + block + fluid-width + :aria-label="getPipelineAriaLabel(index)" + :class="$options.formElementClasses" + data-testid="pipeline-form-ci-variable-type" + @select="setVariableAttribute(variable.key, 'variableType', $event)" + /> + <gl-form-input + v-model="variable.key" + :placeholder="s__('CiVariables|Input variable key')" + :class="$options.formElementClasses" + data-testid="pipeline-form-ci-variable-key-field" + @change="addEmptyVariable(refFullName)" + /> + <variable-values-listbox + v-if="shouldShowValuesDropdown(variable.key)" + :items="createListItemsFromVariableOptions(variable.key)" + :selected="variable.value" + :class="$options.formElementClasses" + class="!gl-mr-0 gl-grow" + data-testid="pipeline-form-ci-variable-value-dropdown" + @select="setVariableAttribute(variable.key, 'value', $event)" + /> + <gl-form-textarea + v-else + v-model="variable.value" + :placeholder="s__('CiVariables|Input variable value')" + :style="$options.textAreaStyle" + :no-resize="false" + data-testid="pipeline-form-ci-variable-value-field" + /> + <template v-if="variables.length > 1"> + <gl-button + v-if="canRemove(index)" + size="small" + class="gl-shrink-0" + data-testid="remove-ci-variable-row" + :category="removeButtonCategory" + :aria-label="s__('CiVariables|Remove variable')" + @click="removeVariable(index)" + > + <gl-icon class="!gl-mr-0" name="remove" /> + <span class="md:gl-hidden">{{ s__('CiVariables|Remove variable') }}</span> + </gl-button> + <gl-button + v-else + class="gl-invisible gl-hidden gl-shrink-0 md:gl-block" + icon="remove" + :aria-label="s__('CiVariables|Remove variable')" + /> + </template> + </div> + <div v-if="descriptions[variable.key]" class="gl-text-subtle"> + {{ descriptions[variable.key] }} + </div> + </div> + <template #description> + <gl-sprintf + :message=" + s__( + 'Pipeline|Specify variable values to be used in this run. The variables specified in the configuration file as well as %{linkStart}CI/CD settings%{linkEnd} are used by default.', + ) + " + > + <template #link="{ content }"> + <gl-link v-if="isMaintainer" :href="settingsLink" data-testid="ci-cd-settings-link"> + {{ content }} + </gl-link> + <template v-else>{{ content }}</template> + </template> + </gl-sprintf> + <gl-link :href="$options.learnMorePath" target="_blank"> + {{ __('Learn more') }} + </gl-link> + <div class="gl-mt-4 gl-text-subtle"> + <gl-sprintf + :message=" + s__( + 'CiVariables|Variables specified here are %{boldStart}expanded%{boldEnd} and not %{boldStart}masked.%{boldEnd}', + ) + " + > + <template #bold="{ content }"> + <strong>{{ content }}</strong> + </template> + </gl-sprintf> + </div> + </template> </gl-form-group> </div> </template> 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 59a153e73e227a8e384effce60ea3fdd9524ced3..0507f62c96090e2e75e5b85a88f1a71c83b3f113 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 @@ -164,6 +164,7 @@ describe('Pipeline New Form', () => { expect(mockCiConfigVariables).toHaveBeenCalled(); }); }); + describe('when the ciInputsForPipelines flag is enabled', () => { beforeEach(async () => { mockCiConfigVariables.mockResolvedValue(mockEmptyCiConfigVariablesResponse); diff --git a/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js b/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js index c7575e695e8374bc7c354f080cebaabee571a5d6..a6c55abd90e40b0210a35dc0a4ccc35c92b17ce9 100644 --- a/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js +++ b/spec/frontend/ci/pipeline_new/components/pipeline_variables_form_spec.js @@ -1,17 +1,24 @@ import { GlFormGroup, GlLoadingIcon } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; +import { GlBreakpointInstance } from '@gitlab/ui/dist/utils'; import VueApollo from 'vue-apollo'; import Vue from 'vue'; -import { fetchPolicies } from '~/lib/graphql'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import { fetchPolicies } from '~/lib/graphql'; import { reportToSentry } from '~/ci/utils'; import ciConfigVariablesQuery from '~/ci/pipeline_new/graphql/queries/ci_config_variables.graphql'; +import { VARIABLE_TYPE } from '~/ci/pipeline_new/constants'; import PipelineVariablesForm from '~/ci/pipeline_new/components/pipeline_variables_form.vue'; Vue.use(VueApollo); jest.mock('~/ci/utils'); +jest.mock('@gitlab/ui/dist/utils', () => ({ + GlBreakpointInstance: { + getBreakpointSize: jest.fn(), + }, +})); describe('PipelineVariablesForm', () => { let wrapper; @@ -19,16 +26,41 @@ describe('PipelineVariablesForm', () => { let mockCiConfigVariables; const defaultProps = { - projectPath: 'group/project', defaultBranch: 'main', + isMaintainer: true, + projectPath: 'group/project', refParam: 'feature', + settingsLink: 'link/to/settings', }; - const createComponent = async ({ props = {} } = {}) => { + const configVariablesWithOptions = [ + { + key: 'VAR_WITH_OPTIONS', + value: 'option1', + description: 'Variable with options', + valueOptions: ['option1', 'option2', 'option3'], + }, + { + key: 'SIMPLE_VAR', + value: 'simple-value', + description: 'Simple variable', + valueOptions: [], + }, + ]; + + const createComponent = async ({ props = {}, configVariables = [] } = {}) => { + mockCiConfigVariables = jest.fn().mockResolvedValue({ + data: { + project: { + ciConfigVariables: configVariables, + }, + }, + }); + const handlers = [[ciConfigVariablesQuery, mockCiConfigVariables]]; mockApollo = createMockApollo(handlers); - wrapper = shallowMount(PipelineVariablesForm, { + wrapper = shallowMountExtended(PipelineVariablesForm, { apolloProvider: mockApollo, propsData: { ...defaultProps, ...props }, }); @@ -36,8 +68,11 @@ describe('PipelineVariablesForm', () => { await waitForPromises(); }; - const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findForm = () => wrapper.findComponent(GlFormGroup); + const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const findVariableRows = () => wrapper.findAllByTestId('ci-variable-row-container'); + const findKeyInputs = () => wrapper.findAllByTestId('pipeline-form-ci-variable-key-field'); + const findRemoveButton = () => wrapper.findByTestId('remove-ci-variable-row'); beforeEach(() => { mockCiConfigVariables = jest.fn().mockResolvedValue({ @@ -65,15 +100,47 @@ describe('PipelineVariablesForm', () => { }); }); + describe('form initialization', () => { + it('adds an empty variable row', async () => { + await createComponent(); + + expect(findVariableRows()).toHaveLength(1); + }); + + it('initializes with variables from config', async () => { + await createComponent({ configVariables: configVariablesWithOptions }); + + const keyInputs = findKeyInputs(); + expect(keyInputs.length).toBeGreaterThanOrEqual(1); + + // Check if at least one of the expected variables exists + const keys = keyInputs.wrappers.map((w) => w.props('value')); + expect(keys.some((key) => ['VAR_WITH_OPTIONS', 'SIMPLE_VAR'].includes(key))).toBe(true); + }); + + it('initializes with variables from props', async () => { + await createComponent({ + props: { + variableParams: { CUSTOM_VAR: 'custom-value' }, + }, + }); + + const keyInputs = findKeyInputs(); + expect(keyInputs.length).toBeGreaterThanOrEqual(1); + + // At least the empty row should exist + const emptyRowExists = keyInputs.wrappers.some((w) => w.props('value') === ''); + expect(emptyRowExists).toBe(true); + }); + }); + describe('query configuration', () => { it('has correct apollo query configuration', async () => { await createComponent(); const { apollo } = wrapper.vm.$options; - expect(apollo.ciConfigVariables).toMatchObject({ - fetchPolicy: fetchPolicies.NO_CACHE, - query: ciConfigVariablesQuery, - }); + expect(apollo.ciConfigVariables.fetchPolicy).toBe(fetchPolicies.NO_CACHE); + expect(apollo.ciConfigVariables.query).toBe(ciConfigVariablesQuery); }); it('makes query with correct variables', async () => { @@ -86,10 +153,73 @@ describe('PipelineVariablesForm', () => { }); it('reports to sentry when query fails', async () => { - mockCiConfigVariables = jest.fn().mockRejectedValue(new Error('GraphQL error')); + const error = new Error('GraphQL error'); + await createComponent(); + wrapper.vm.$options.apollo.ciConfigVariables.error.call(wrapper.vm, error); + + expect(reportToSentry).toHaveBeenCalledWith('PipelineVariablesForm', error); + }); + }); + describe('variable rows', () => { + it('emits variables-updated event when variables change', async () => { await createComponent(); - expect(reportToSentry).toHaveBeenCalledWith('PipelineVariablesForm', expect.any(Error)); + + expect(wrapper.emitted('variables-updated')).toHaveLength(1); + + wrapper.vm.$options.watch.variables.handler.call(wrapper.vm, [ + { key: 'TEST_KEY', value: 'test_value', variableType: VARIABLE_TYPE }, + ]); + + expect(wrapper.emitted('variables-updated')).toHaveLength(2); + }); + }); + + describe('variable removal', () => { + it('shows remove button with correct aria-label', async () => { + await createComponent({ + props: { variableParams: { VAR1: 'value1', VAR2: 'value2' } }, + }); + + expect(findRemoveButton().exists()).toBe(true); + expect(findRemoveButton().attributes('aria-label')).toBe('Remove variable'); + }); + }); + + describe('responsive design', () => { + it('uses secondary button category on mobile', async () => { + GlBreakpointInstance.getBreakpointSize.mockReturnValue('sm'); + await createComponent({ + props: { variableParams: { VAR1: 'value1' } }, + }); + + expect(findRemoveButton().exists()).toBe(true); + expect(findRemoveButton().props('category')).toBe('secondary'); + }); + + it('uses tertiary button category on desktop', async () => { + GlBreakpointInstance.getBreakpointSize.mockReturnValue('md'); + await createComponent({ + props: { variableParams: { VAR1: 'value1' } }, + }); + + expect(findRemoveButton().exists()).toBe(true); + expect(findRemoveButton().props('category')).toBe('tertiary'); + }); + }); + + describe('settings link', () => { + it('passes correct props for maintainers', async () => { + await createComponent({ props: { isMaintainer: true } }); + + expect(wrapper.props('isMaintainer')).toBe(true); + expect(wrapper.props('settingsLink')).toBe(defaultProps.settingsLink); + }); + + it('passes correct props for non-maintainers', async () => { + await createComponent({ props: { isMaintainer: false } }); + + expect(wrapper.props('isMaintainer')).toBe(false); }); }); });