From 33d4e66de0decee8bdc8225b13ab69d45ac6dcb1 Mon Sep 17 00:00:00 2001
From: Miguel Rincon <mrincon@gitlab.com>
Date: Fri, 16 Oct 2020 13:40:32 +0000
Subject: [PATCH] Load configured variables when ref changes
This changes loads the variables from the CI file and puts them in
the "Run pipeline" form as default variables.
---
.../components/pipeline_new_form.vue | 203 ++++++++++++------
app/assets/javascripts/pipeline_new/index.js | 2 +
.../projects/pipelines_controller.rb | 1 +
app/views/projects/pipelines/new.html.haml | 10 +-
.../new_pipeline_form_prefilled_vars.yml | 7 +
.../components/pipeline_new_form_spec.js | 165 +++++++++++++-
6 files changed, 313 insertions(+), 75 deletions(-)
create mode 100644 config/feature_flags/development/new_pipeline_form_prefilled_vars.yml
diff --git a/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue b/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue
index b05cf080aea6..5841716c8c5f 100644
--- a/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue
+++ b/app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue
@@ -1,4 +1,5 @@
<script>
+import Vue from 'vue';
import { uniqueId } from 'lodash';
import {
GlAlert,
@@ -50,6 +51,10 @@ export default {
type: String,
required: true,
},
+ configVariablesPath: {
+ type: String,
+ required: true,
+ },
projectId: {
type: String,
required: true,
@@ -86,7 +91,7 @@ export default {
return {
searchTerm: '',
refValue: this.refParam,
- variables: [],
+ form: {},
error: null,
warnings: [],
totalWarnings: 0,
@@ -110,60 +115,122 @@ export default {
shouldShowWarning() {
return this.warnings.length > 0 && !this.isWarningDismissed;
},
+ variables() {
+ return this.form[this.refValue]?.variables ?? [];
+ },
+ descriptions() {
+ return this.form[this.refValue]?.descriptions ?? {};
+ },
},
created() {
- this.addEmptyVariable();
-
- if (this.variableParams) {
- this.setVariableParams(VARIABLE_TYPE, this.variableParams);
- }
-
- if (this.fileParams) {
- this.setVariableParams(FILE_TYPE, this.fileParams);
- }
+ this.setRefSelected(this.refValue);
},
methods: {
- setVariable(type, key, value) {
- const variable = this.variables.find(v => v.key === key);
+ 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}`),
+ variable_type: VARIABLE_TYPE,
+ key: '',
+ value: '',
+ });
+ },
+ setVariable(refValue, type, key, value) {
+ const { variables } = this.form[refValue];
+
+ const variable = variables.find(v => v.key === key);
if (variable) {
variable.type = type;
variable.value = value;
} else {
- // insert before the empty variable
- this.variables.splice(this.variables.length - 1, 0, {
- uniqueId: uniqueId('var'),
+ variables.push({
+ uniqueId: uniqueId(`var-${refValue}`),
key,
value,
variable_type: type,
});
}
},
- setVariableParams(type, paramsObj) {
+ setVariableParams(refValue, type, paramsObj) {
Object.entries(paramsObj).forEach(([key, value]) => {
- this.setVariable(type, key, value);
+ this.setVariable(refValue, type, key, value);
});
},
- setRefSelected(ref) {
- this.refValue = ref;
+ setRefSelected(refValue) {
+ this.refValue = refValue;
+
+ if (!this.form[refValue]) {
+ this.fetchConfigVariables(refValue)
+ .then(({ descriptions, params }) => {
+ Vue.set(this.form, refValue, {
+ variables: [],
+ descriptions,
+ });
+
+ // Add default variables from yml
+ this.setVariableParams(refValue, VARIABLE_TYPE, params);
+ })
+ .catch(() => {
+ Vue.set(this.form, refValue, {
+ variables: [],
+ descriptions: {},
+ });
+ })
+ .finally(() => {
+ // Add/update variables, e.g. from query string
+ if (this.variableParams) {
+ this.setVariableParams(refValue, VARIABLE_TYPE, this.variableParams);
+ }
+ if (this.fileParams) {
+ this.setVariableParams(refValue, FILE_TYPE, this.fileParams);
+ }
+
+ // Adds empty var at the end of the form
+ this.addEmptyVariable(refValue);
+ });
+ }
},
+
isSelected(ref) {
return ref === this.refValue;
},
- addEmptyVariable() {
- this.variables.push({
- uniqueId: uniqueId('var'),
- variable_type: VARIABLE_TYPE,
- key: '',
- value: '',
- });
- },
removeVariable(index) {
this.variables.splice(index, 1);
},
-
canRemove(index) {
return index < this.variables.length - 1;
},
+
+ fetchConfigVariables(refValue) {
+ if (gon?.features?.newPipelineFormPrefilledVars) {
+ return axios
+ .get(this.configVariablesPath, {
+ params: {
+ sha: refValue,
+ },
+ })
+ .then(({ data }) => {
+ const params = {};
+ const descriptions = {};
+
+ Object.entries(data).forEach(([key, { value, description }]) => {
+ if (description !== null) {
+ params[key] = value;
+ descriptions[key] = description;
+ }
+ });
+
+ return { params, descriptions };
+ });
+ }
+ return Promise.resolve({ params: {}, descriptions: {} });
+ },
createPipeline() {
const filteredVariables = this.variables
.filter(({ key, value }) => key !== '' && value !== '')
@@ -261,45 +328,53 @@ export default {
<div
v-for="(variable, index) in variables"
:key="variable.uniqueId"
- class="gl-display-flex gl-align-items-stretch gl-align-items-center gl-mb-4 gl-ml-n3 gl-pb-2 gl-border-b-solid gl-border-gray-200 gl-border-b-1 gl-flex-direction-column gl-md-flex-direction-row"
+ class="gl-mb-3 gl-ml-n3 gl-pb-2"
data-testid="ci-variable-row"
>
- <gl-form-select
- v-model="variable.variable_type"
- :class="$options.formElementClasses"
- :options="$options.typeOptions"
- />
- <gl-form-input
- v-model="variable.key"
- :placeholder="s__('CiVariables|Input variable key')"
- :class="$options.formElementClasses"
- data-testid="pipeline-form-ci-variable-key"
- @change.once="addEmptyVariable()"
- />
- <gl-form-input
- v-model="variable.value"
- :placeholder="s__('CiVariables|Input variable value')"
- class="gl-mb-3"
- />
-
- <template v-if="variables.length > 1">
- <gl-button
- v-if="canRemove(index)"
- class="gl-md-ml-3 gl-mb-3"
- data-testid="remove-ci-variable-row"
- variant="danger"
- category="secondary"
- @click="removeVariable(index)"
- >
- <gl-icon class="gl-mr-0! gl-display-none gl-display-md-block" name="clear" />
- <span class="gl-display-md-none">{{ s__('CiVariables|Remove variable') }}</span>
- </gl-button>
- <gl-button
- v-else
- class="gl-md-ml-3 gl-mb-3 gl-display-none gl-display-md-block gl-visibility-hidden"
- icon="clear"
+ <div
+ class="gl-display-flex gl-align-items-stretch gl-flex-direction-column gl-md-flex-direction-row"
+ >
+ <gl-form-select
+ v-model="variable.variable_type"
+ :class="$options.formElementClasses"
+ :options="$options.typeOptions"
+ />
+ <gl-form-input
+ v-model="variable.key"
+ :placeholder="s__('CiVariables|Input variable key')"
+ :class="$options.formElementClasses"
+ data-testid="pipeline-form-ci-variable-key"
+ @change="addEmptyVariable(refValue)"
+ />
+ <gl-form-input
+ v-model="variable.value"
+ :placeholder="s__('CiVariables|Input variable value')"
+ class="gl-mb-3"
+ data-testid="pipeline-form-ci-variable-value"
/>
- </template>
+
+ <template v-if="variables.length > 1">
+ <gl-button
+ v-if="canRemove(index)"
+ class="gl-md-ml-3 gl-mb-3"
+ data-testid="remove-ci-variable-row"
+ variant="danger"
+ category="secondary"
+ @click="removeVariable(index)"
+ >
+ <gl-icon class="gl-mr-0! gl-display-none gl-display-md-block" name="clear" />
+ <span class="gl-display-md-none">{{ s__('CiVariables|Remove variable') }}</span>
+ </gl-button>
+ <gl-button
+ v-else
+ class="gl-md-ml-3 gl-mb-3 gl-display-none gl-display-md-block gl-visibility-hidden"
+ icon="clear"
+ />
+ </template>
+ </div>
+ <div v-if="descriptions[variable.key]" class="gl-text-gray-500 gl-mb-3">
+ {{ descriptions[variable.key] }}
+ </div>
</div>
<template #description
diff --git a/app/assets/javascripts/pipeline_new/index.js b/app/assets/javascripts/pipeline_new/index.js
index f1ea86f8c5fb..ff4f677654e7 100644
--- a/app/assets/javascripts/pipeline_new/index.js
+++ b/app/assets/javascripts/pipeline_new/index.js
@@ -6,6 +6,7 @@ export default () => {
const {
projectId,
pipelinesPath,
+ configVariablesPath,
refParam,
varParam,
fileParam,
@@ -25,6 +26,7 @@ export default () => {
props: {
projectId,
pipelinesPath,
+ configVariablesPath,
refParam,
variableParams,
fileParams,
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 39dd7a9899d9..953dce4d63c8 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -17,6 +17,7 @@ class Projects::PipelinesController < Projects::ApplicationController
push_frontend_feature_flag(:pipelines_security_report_summary, project)
push_frontend_feature_flag(:new_pipeline_form, project)
push_frontend_feature_flag(:graphql_pipeline_header, project, type: :development, default_enabled: false)
+ push_frontend_feature_flag(:new_pipeline_form_prefilled_vars, project, type: :development)
end
before_action :ensure_pipeline, only: [:show]
diff --git a/app/views/projects/pipelines/new.html.haml b/app/views/projects/pipelines/new.html.haml
index 29e5f2cf5b48..cb5401cd3293 100644
--- a/app/views/projects/pipelines/new.html.haml
+++ b/app/views/projects/pipelines/new.html.haml
@@ -7,7 +7,15 @@
%hr
- if Feature.enabled?(:new_pipeline_form, @project)
- #js-new-pipeline{ data: { project_id: @project.id, pipelines_path: project_pipelines_path(@project), ref_param: params[:ref] || @project.default_branch, var_param: params[:var].to_json, file_param: params[:file_var].to_json, ref_names: @project.repository.ref_names.to_json.html_safe, settings_link: project_settings_ci_cd_path(@project), max_warnings: ::Gitlab::Ci::Warnings::MAX_LIMIT } }
+ #js-new-pipeline{ data: { project_id: @project.id,
+ pipelines_path: project_pipelines_path(@project),
+ config_variables_path: config_variables_namespace_project_pipelines_path(@project.namespace, @project),
+ ref_param: params[:ref] || @project.default_branch,
+ var_param: params[:var].to_json,
+ file_param: params[:file_var].to_json,
+ ref_names: @project.repository.ref_names.to_json.html_safe,
+ settings_link: project_settings_ci_cd_path(@project),
+ max_warnings: ::Gitlab::Ci::Warnings::MAX_LIMIT } }
- else
= form_for @pipeline, as: :pipeline, url: project_pipelines_path(@project), html: { id: "new-pipeline-form", class: "js-new-pipeline-form js-requires-input" } do |f|
diff --git a/config/feature_flags/development/new_pipeline_form_prefilled_vars.yml b/config/feature_flags/development/new_pipeline_form_prefilled_vars.yml
new file mode 100644
index 000000000000..6b821e7fd9e7
--- /dev/null
+++ b/config/feature_flags/development/new_pipeline_form_prefilled_vars.yml
@@ -0,0 +1,7 @@
+---
+name: new_pipeline_form_prefilled_vars
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44120
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/263276
+type: development
+group: group::continuous integration
+default_enabled: false
diff --git a/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js b/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js
index 97a92778f1a9..040c0fbecc59 100644
--- a/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js
+++ b/spec/frontend/pipeline_new/components/pipeline_new_form_spec.js
@@ -2,6 +2,7 @@ import { mount, shallowMount } from '@vue/test-utils';
import { GlDropdown, GlDropdownItem, GlForm, GlSprintf } from '@gitlab/ui';
import MockAdapter from 'axios-mock-adapter';
import waitForPromises from 'helpers/wait_for_promises';
+import httpStatusCodes from '~/lib/utils/http_status';
import axios from '~/lib/utils/axios_utils';
import PipelineNewForm from '~/pipeline_new/components/pipeline_new_form.vue';
import { mockRefs, mockParams, mockPostParams, mockProjectId, mockError } from '../mock_data';
@@ -11,7 +12,8 @@ jest.mock('~/lib/utils/url_utility', () => ({
redirectTo: jest.fn(),
}));
-const pipelinesPath = '/root/project/-/pipleines';
+const pipelinesPath = '/root/project/-/pipelines';
+const configVariablesPath = '/root/project/-/pipelines/config_variables';
const postResponse = { id: 1 };
describe('Pipeline New Form', () => {
@@ -28,6 +30,7 @@ describe('Pipeline New Form', () => {
const findVariableRows = () => wrapper.findAll('[data-testid="ci-variable-row"]');
const findRemoveIcons = () => wrapper.findAll('[data-testid="remove-ci-variable-row"]');
const findKeyInputs = () => wrapper.findAll('[data-testid="pipeline-form-ci-variable-key"]');
+ const findValueInputs = () => wrapper.findAll('[data-testid="pipeline-form-ci-variable-value"]');
const findErrorAlert = () => wrapper.find('[data-testid="run-pipeline-error-alert"]');
const findWarningAlert = () => wrapper.find('[data-testid="run-pipeline-warning-alert"]');
const findWarningAlertSummary = () => findWarningAlert().find(GlSprintf);
@@ -39,6 +42,7 @@ describe('Pipeline New Form', () => {
propsData: {
projectId: mockProjectId,
pipelinesPath,
+ configVariablesPath,
refs: mockRefs,
defaultBranch: 'master',
settingsLink: '',
@@ -55,6 +59,7 @@ describe('Pipeline New Form', () => {
beforeEach(() => {
mock = new MockAdapter(axios);
+ mock.onGet(configVariablesPath).reply(httpStatusCodes.OK, {});
});
afterEach(() => {
@@ -66,7 +71,7 @@ describe('Pipeline New Form', () => {
describe('Dropdown with branches and tags', () => {
beforeEach(() => {
- mock.onPost(pipelinesPath).reply(200, postResponse);
+ mock.onPost(pipelinesPath).reply(httpStatusCodes.OK, postResponse);
});
it('displays dropdown with all branches and tags', () => {
@@ -87,17 +92,27 @@ describe('Pipeline New Form', () => {
});
describe('Form', () => {
- beforeEach(() => {
+ beforeEach(async () => {
createComponent('', mockParams, mount);
- mock.onPost(pipelinesPath).reply(200, postResponse);
+ mock.onPost(pipelinesPath).reply(httpStatusCodes.OK, postResponse);
+
+ await waitForPromises();
});
+
it('displays the correct values for the provided query params', async () => {
expect(findDropdown().props('text')).toBe('tag-1');
+ expect(findVariableRows()).toHaveLength(3);
+ });
- await wrapper.vm.$nextTick();
+ it('displays a variable from provided query params', () => {
+ expect(findKeyInputs().at(0).element.value).toBe('test_var');
+ expect(findValueInputs().at(0).element.value).toBe('test_var_val');
+ });
- expect(findVariableRows()).toHaveLength(3);
+ it('displays an empty variable for the user to fill out', async () => {
+ expect(findKeyInputs().at(2).element.value).toBe('');
+ expect(findValueInputs().at(2).element.value).toBe('');
});
it('does not display remove icon for last row', () => {
@@ -124,13 +139,143 @@ describe('Pipeline New Form', () => {
});
it('creates blank variable on input change event', async () => {
- findKeyInputs()
- .at(2)
- .trigger('change');
+ const input = findKeyInputs().at(2);
+ input.element.value = 'test_var_2';
+ input.trigger('change');
await wrapper.vm.$nextTick();
expect(findVariableRows()).toHaveLength(4);
+ expect(findKeyInputs().at(3).element.value).toBe('');
+ expect(findValueInputs().at(3).element.value).toBe('');
+ });
+
+ describe('when the form has been modified', () => {
+ const selectRef = i =>
+ findDropdownItems()
+ .at(i)
+ .vm.$emit('click');
+
+ beforeEach(async () => {
+ const input = findKeyInputs().at(0);
+ input.element.value = 'test_var_2';
+ input.trigger('change');
+
+ findRemoveIcons()
+ .at(1)
+ .trigger('click');
+
+ await wrapper.vm.$nextTick();
+ });
+
+ it('form values are restored when the ref changes', async () => {
+ expect(findVariableRows()).toHaveLength(2);
+
+ selectRef(1);
+ await waitForPromises();
+
+ expect(findVariableRows()).toHaveLength(3);
+ expect(findKeyInputs().at(0).element.value).toBe('test_var');
+ });
+
+ it('form values are restored again when the ref is reverted', async () => {
+ selectRef(1);
+ await waitForPromises();
+
+ selectRef(2);
+ await waitForPromises();
+
+ expect(findVariableRows()).toHaveLength(2);
+ expect(findKeyInputs().at(0).element.value).toBe('test_var_2');
+ });
+ });
+ });
+
+ describe('when feature flag new_pipeline_form_prefilled_vars is enabled', () => {
+ let origGon;
+
+ const mockYmlKey = 'yml_var';
+ const mockYmlValue = 'yml_var_val';
+ const mockYmlDesc = 'A var from yml.';
+
+ beforeAll(() => {
+ origGon = window.gon;
+ window.gon = { features: { newPipelineFormPrefilledVars: true } };
+ });
+
+ afterAll(() => {
+ window.gon = origGon;
+ });
+
+ describe('when yml defines a variable with description', () => {
+ beforeEach(async () => {
+ createComponent('', mockParams, mount);
+
+ mock.onGet(configVariablesPath).reply(httpStatusCodes.OK, {
+ [mockYmlKey]: {
+ value: mockYmlValue,
+ description: mockYmlDesc,
+ },
+ });
+
+ await waitForPromises();
+ });
+
+ it('displays all the variables', async () => {
+ expect(findVariableRows()).toHaveLength(4);
+ });
+
+ it('displays a variable from yml', () => {
+ expect(findKeyInputs().at(0).element.value).toBe(mockYmlKey);
+ expect(findValueInputs().at(0).element.value).toBe(mockYmlValue);
+ });
+
+ it('displays a variable from provided query params', () => {
+ expect(findKeyInputs().at(1).element.value).toBe('test_var');
+ expect(findValueInputs().at(1).element.value).toBe('test_var_val');
+ });
+
+ it('adds a description to the first variable from yml', () => {
+ expect(
+ findVariableRows()
+ .at(0)
+ .text(),
+ ).toContain(mockYmlDesc);
+ });
+
+ it('removes the description when a variable key changes', async () => {
+ findKeyInputs().at(0).element.value = 'yml_var_modified';
+ findKeyInputs()
+ .at(0)
+ .trigger('change');
+
+ await wrapper.vm.$nextTick();
+
+ expect(
+ findVariableRows()
+ .at(0)
+ .text(),
+ ).not.toContain(mockYmlDesc);
+ });
+ });
+
+ describe('when yml defines a variable without description', () => {
+ beforeEach(async () => {
+ createComponent('', mockParams, mount);
+
+ mock.onGet(configVariablesPath).reply(httpStatusCodes.OK, {
+ [mockYmlKey]: {
+ value: mockYmlValue,
+ description: null,
+ },
+ });
+
+ await waitForPromises();
+ });
+
+ it('displays all the variables', async () => {
+ expect(findVariableRows()).toHaveLength(3);
+ });
});
});
@@ -138,7 +283,7 @@ describe('Pipeline New Form', () => {
beforeEach(() => {
createComponent();
- mock.onPost(pipelinesPath).reply(400, mockError);
+ mock.onPost(pipelinesPath).reply(httpStatusCodes.BAD_REQUEST, mockError);
findForm().vm.$emit('submit', dummySubmitEvent);
--
GitLab