Merge branch '30101-frontend-prefilled-variables' into 'master'

Load configured variables when `ref` changes

See merge request gitlab-org/gitlab!44120

app/assets/javascripts/pipeline_new/components/pipeline_new_form.vue

 <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

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,

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]
 

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|

config/feature_flags/development/new_pipeline_form_prefilled_vars.yml

+---
+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

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);