Use dynamic variable list in scheduled pipelines and group/project CI secret variables

See https://gitlab.com/gitlab-org/gitlab-ce/issues/39118

Conflicts:
	app/views/ci/variables/_form.html.haml
	app/views/ci/variables/_table.html.haml
	ee/app/views/ci/variables/_environment_scope.html.haml
	spec/javascripts/ci_variable_list/ci_variable_list_ee_spec.js
	spec/javascripts/fixtures/projects.rb

app/assets/javascripts/ci_variable_list/ajax_variable_list.js

+import _ from 'underscore';
+import axios from '../lib/utils/axios_utils';
+import { s__ } from '../locale';
+import Flash from '../flash';
+import { convertPermissionToBoolean } from '../lib/utils/common_utils';
+import statusCodes from '../lib/utils/http_status';
+import VariableList from './ci_variable_list';
+
+function generateErrorBoxContent(errors) {
+  const errorList = [].concat(errors).map(errorString => `
+    <li>
+      ${_.escape(errorString)}
+    </li>
+  `);
+
+  return `
+    <p>
+      ${s__('CiVariable|Validation failed')}
+    </p>
+    <ul>
+      ${errorList.join('')}
+    </ul>
+  `;
+}
+
+// Used for the variable list on CI/CD projects/groups settings page
+export default class AjaxVariableList {
+  constructor({
+    container,
+    saveButton,
+    errorBox,
+    formField = 'variables',
+    saveEndpoint,
+  }) {
+    this.container = container;
+    this.saveButton = saveButton;
+    this.errorBox = errorBox;
+    this.saveEndpoint = saveEndpoint;
+
+    this.variableList = new VariableList({
+      container: this.container,
+      formField,
+    });
+
+    this.bindEvents();
+    this.variableList.init();
+  }
+
+  bindEvents() {
+    this.saveButton.addEventListener('click', this.onSaveClicked.bind(this));
+  }
+
+  onSaveClicked() {
+    const loadingIcon = this.saveButton.querySelector('.js-secret-variables-save-loading-icon');
+    loadingIcon.classList.toggle('hide', false);
+    this.errorBox.classList.toggle('hide', true);
+    // We use this to prevent a user from changing a key before we have a chance
+    // to match it up in `updateRowsWithPersistedVariables`
+    this.variableList.toggleEnableRow(false);
+
+    return axios.patch(this.saveEndpoint, {
+      variables_attributes: this.variableList.getAllData(),
+    }, {
+      // We want to be able to process the `res.data` from a 400 error response
+      // and print the validation messages such as duplicate variable keys
+      validateStatus: status => (
+          status >= statusCodes.OK &&
+          status < statusCodes.MULTIPLE_CHOICES
+        ) ||
+        status === statusCodes.BAD_REQUEST,
+    })
+      .then((res) => {
+        loadingIcon.classList.toggle('hide', true);
+        this.variableList.toggleEnableRow(true);
+
+        if (res.status === statusCodes.OK && res.data) {
+          this.updateRowsWithPersistedVariables(res.data.variables);
+        } else if (res.status === statusCodes.BAD_REQUEST) {
+          // Validation failed
+          this.errorBox.innerHTML = generateErrorBoxContent(res.data);
+          this.errorBox.classList.toggle('hide', false);
+        }
+      })
+      .catch(() => {
+        loadingIcon.classList.toggle('hide', true);
+        this.variableList.toggleEnableRow(true);
+        Flash(s__('CiVariable|Error occured while saving variables'));
+      });
+  }
+
+  updateRowsWithPersistedVariables(persistedVariables = []) {
+    const persistedVariableMap = [].concat(persistedVariables).reduce((variableMap, variable) => ({
+      ...variableMap,
+      [variable.key]: variable,
+    }), {});
+
+    this.container.querySelectorAll('.js-row').forEach((row) => {
+      // If we submitted a row that was destroyed, remove it so we don't try
+      // to destroy it again which would cause a BE error
+      const destroyInput = row.querySelector('.js-ci-variable-input-destroy');
+      if (convertPermissionToBoolean(destroyInput.value)) {
+        row.remove();
+      // Update the ID input so any future edits and `_destroy` will apply on the BE
+      } else {
+        const key = row.querySelector('.js-ci-variable-input-key').value;
+        const persistedVariable = persistedVariableMap[key];
+
+        if (persistedVariable) {
+          // eslint-disable-next-line no-param-reassign
+          row.querySelector('.js-ci-variable-input-id').value = persistedVariable.id;
+          row.setAttribute('data-is-persisted', 'true');
+        }
+      }
+    });
+  }
+}

app/assets/javascripts/ci_variable_list/ci_variable_list.js

@@ -11,7 +11,7 @@ function createEnvironmentItem(value) {
   return {
     title: value === '*' ? ALL_ENVIRONMENTS_STRING : value,
     id: value,
-    text: value,
+    text: value === '*' ? s__('CiVariable|* (All environments)') : value,
   };
 }
@@ -41,11 +41,11 @@ export default class VariableList {
         selector: '.js-ci-variable-input-protected',
         default: 'true',
       },
-      environment: {
+      environment_scope: {
         // We can't use a `.js-` class here because
         // gl_dropdown replaces the <input> and doesn't copy over the class
         // See https://gitlab.com/gitlab-org/gitlab-ce/issues/42458
-        selector: `input[name="${this.formField}[variables_attributes][][environment]"]`,
+        selector: `input[name="${this.formField}[variables_attributes][][environment_scope]"]`,
         default: '*',
       },
       _destroy: {
@@ -104,12 +104,15 @@ export default class VariableList {
     setupToggleButtons($row[0]);
+    // Reset the resizable textarea
+    $row.find(this.inputMap.value.selector).css('height', '');
+
     const $environmentSelect = $row.find('.js-variable-environment-toggle');
     if ($environmentSelect.length) {
       const createItemDropdown = new CreateItemDropdown({
         $dropdown: $environmentSelect,
         defaultToggleLabel: ALL_ENVIRONMENTS_STRING,
-        fieldName: `${this.formField}[variables_attributes][][environment]`,
+        fieldName: `${this.formField}[variables_attributes][][environment_scope]`,
         getData: (term, callback) => callback(this.getEnvironmentValues()),
         createNewItemFromValue: createEnvironmentItem,
         onSelect: () => {
@@ -117,7 +120,7 @@ export default class VariableList {
           // so they have the new value we just picked
           this.refreshDropdownData();
-          $row.find(this.inputMap.environment.selector).trigger('trigger-change');
+          $row.find(this.inputMap.environment_scope.selector).trigger('trigger-change');
         },
       });
@@ -143,7 +146,8 @@ export default class VariableList {
     $row.after($rowClone);
   }
-  removeRow($row) {
+  removeRow(row) {
+    const $row = $(row);
     const isPersisted = convertPermissionToBoolean($row.attr('data-is-persisted'));
     if (isPersisted) {
@@ -155,6 +159,10 @@ export default class VariableList {
     } else {
       $row.remove();
     }
+
+    // Refresh the other dropdowns in the variable list
+    // so any value with the variable deleted is gone
+    this.refreshDropdownData();
   }
   checkIfRowTouched($row) {
@@ -165,6 +173,11 @@ export default class VariableList {
     });
   }
+  toggleEnableRow(isEnabled = true) {
+    this.$container.find(this.inputMap.key.selector).attr('disabled', !isEnabled);
+    this.$container.find('.js-row-remove-button').attr('disabled', !isEnabled);
+  }
+
   getAllData() {
     // Ignore the last empty row because we don't want to try persist
     // a blank variable and run into validation problems.
@@ -185,7 +198,7 @@ export default class VariableList {
   }
   getEnvironmentValues() {
-    const valueMap = this.$container.find(this.inputMap.environment.selector).toArray()
+    const valueMap = this.$container.find(this.inputMap.environment_scope.selector).toArray()
       .reduce((prevValueMap, envInput) => ({
         ...prevValueMap,
         [envInput.value]: envInput.value,

app/assets/javascripts/commons/polyfills/element.js

@@ -18,3 +18,22 @@ Element.prototype.matches = Element.prototype.matches ||
     while (i >= 0 && elms.item(i) !== this) { i -= 1; }
     return i > -1;
   };
+
+// From the polyfill on MDN, https://developer.mozilla.org/en-US/docs/Web/API/ChildNode/remove#Polyfill
+((arr) => {
+  arr.forEach((item) => {
+    if (Object.prototype.hasOwnProperty.call(item, 'remove')) {
+      return;
+    }
+    Object.defineProperty(item, 'remove', {
+      configurable: true,
+      enumerable: true,
+      writable: true,
+      value: function remove() {
+        if (this.parentNode !== null) {
+          this.parentNode.removeChild(this);
+        }
+      },
+    });
+  });
+})([Element.prototype, CharacterData.prototype, DocumentType.prototype]);

app/assets/javascripts/lib/utils/http_status.js

@@ -6,4 +6,6 @@ export default {
   ABORTED: 0,
   NO_CONTENT: 204,
   OK: 200,
+  MULTIPLE_CHOICES: 300,
+  BAD_REQUEST: 400,
 };

app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js

-import SecretValues from '~/behaviors/secret_values';
+import AjaxVariableList from '~/ci_variable_list/ajax_variable_list';
 export default () => {
-  const secretVariableTable = document.querySelector('.js-secret-variable-table');
-  if (secretVariableTable) {
-    const secretVariableTableValues = new SecretValues({
-      container: secretVariableTable,
-    });
-    secretVariableTableValues.init();
-  }
+  const variableListEl = document.querySelector('.js-ci-variable-list-section');
+  // eslint-disable-next-line no-new
+  new AjaxVariableList({
+    container: variableListEl,
+    saveButton: variableListEl.querySelector('.js-secret-variables-save-button'),
+    errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
+    saveEndpoint: variableListEl.dataset.saveEndpoint,
+  });
 };

app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js

 import initSettingsPanels from '~/settings_panels';
 import SecretValues from '~/behaviors/secret_values';
+import AjaxVariableList from '~/ci_variable_list/ajax_variable_list';
 export default function () {
   // Initialize expandable settings panels
   initSettingsPanels();
+
   const runnerToken = document.querySelector('.js-secret-runner-token');
   if (runnerToken) {
     const runnerTokenSecretValue = new SecretValues({
@@ -12,11 +14,12 @@ export default function () {
     runnerTokenSecretValue.init();
   }
-  const secretVariableTable = document.querySelector('.js-secret-variable-table');
-  if (secretVariableTable) {
-    const secretVariableTableValues = new SecretValues({
-      container: secretVariableTable,
-    });
-    secretVariableTableValues.init();
-  }
+  const variableListEl = document.querySelector('.js-ci-variable-list-section');
+  // eslint-disable-next-line no-new
+  new AjaxVariableList({
+    container: variableListEl,
+    saveButton: variableListEl.querySelector('.js-secret-variables-save-button'),
+    errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
+    saveEndpoint: variableListEl.dataset.saveEndpoint,
+  });
 }

app/assets/stylesheets/framework/ci_variable_list.scss

@@ -8,7 +8,11 @@
 .ci-variable-row {
   display: flex;
-  align-items: flex-end;
+  align-items: flex-start;
+
+  @media (max-width: $screen-xs-max) {
+    align-items: flex-end;
+  }
   &:not(:last-child) {
     margin-bottom: $gl-btn-padding;
@@ -41,6 +45,7 @@
 .ci-variable-row-body {
   display: flex;
+  align-items: flex-start;
   width: 100%;
   @media (max-width: $screen-xs-max) {
@@ -85,4 +90,8 @@
     outline: none;
     color: $gl-text-color;
   }
+
+  &[disabled] {
+    color: $gl-text-color-disabled;
+  }
 }

app/views/ci/variables/_content.html.haml

-%p.append-bottom-default
-  Variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags.
-  You can use variables for passwords, secret keys, or whatever you want.
+= _('Variables are applied to environments via the runner. They can be protected by only exposing them to protected branches or tags. You can use variables for passwords, secret keys, or whatever you want.')

app/views/ci/variables/_form.html.haml

-= form_for @variable, as: :variable, url: @variable.form_path do |f|
-  = form_errors(@variable)
-
-  .form-group
-    = f.label :key, "Key", class: "label-light"
-    = f.text_field :key, class: "form-control", placeholder: @variable.placeholder, required: true
-  .form-group
-    = f.label :value, "Value", class: "label-light"
-    = f.text_area :value, class: "form-control", placeholder: @variable.placeholder
-  .form-group
-    .checkbox
-      = f.label :protected do
-        = f.check_box :protected
-        %strong Protected
-      .help-block
-        This variable will be passed only to pipelines running on protected branches and tags
-        = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'protected-secret-variables'), target: '_blank'
-
-  = f.submit btn_text, class: "btn btn-save"

app/views/ci/variables/_index.html.haml

-.row.prepend-top-default.append-bottom-default
-  .col-lg-12
-    %h5.prepend-top-0
-      Add a variable
-    = render "ci/variables/form", btn_text: "Add new variable"
-    %hr
-    %h5.prepend-top-0
-      Your variables (#{@variables.size})
-    - if @variables.empty?
-      %p.settings-message.text-center.append-bottom-0
-        No variables found, add one with the form above.
-    - else
-      .js-secret-variable-table
-        = render "ci/variables/table"
-        %button.btn.btn-info.js-secret-value-reveal-button{ data: { secret_reveal_status: 'false' } }
+- save_endpoint = local_assigns.fetch(:save_endpoint, nil)
+
+.row
+  .col-lg-12.js-ci-variable-list-section{ data: { save_endpoint: save_endpoint } }
+    .hide.alert.alert-danger.js-ci-variable-error-box
+
+    %ul.ci-variable-list
+      - @variables.each.each do |variable|
+        = render 'ci/variables/variable_row', form_field: 'variables', variable: variable
+      = render 'ci/variables/variable_row', form_field: 'variables'
+    .prepend-top-20
+      %button.btn.btn-success.js-secret-variables-save-button{ type: 'button' }
+        %span.hide.js-secret-variables-save-loading-icon
+          = icon('spinner spin')
+        = _('Save variables')
+      %button.btn.btn-info.btn-inverted.prepend-left-10.js-secret-value-reveal-button{ type: 'button', data: { secret_reveal_status: "#{@variables.size == 0}" } }
+        - if @variables.size == 0
+          = n_('Hide value', 'Hide values', @variables.size)
+        - else
           = n_('Reveal value', 'Reveal values', @variables.size)

app/views/ci/variables/_show.html.haml

-- page_title "Variables"
-
-.row.prepend-top-default.append-bottom-default
-  .col-lg-3
-    = render "ci/variables/content"
-  .col-lg-9
-    %h4.prepend-top-0
-      Update variable
-    = render "ci/variables/form", btn_text: "Save variable"

app/views/ci/variables/_table.html.haml

-.table-responsive.variables-table
-  %table.table
-    %colgroup
-      %col
-      %col
-      %col
-      %col{ width: 100 }
-    %thead
-      %th Key
-      %th Value
-      %th Protected
-      %th
-    %tbody
-      - @variables.each do |variable|
-        - if variable.id?
-          %tr
-            %td.variable-key= variable.key
-            %td.variable-value
-              %span.js-secret-value-placeholder
-                = '*' * 6
-              %span.hide.js-secret-value
-                = variable.value
-            %td.variable-protected= Gitlab::Utils.boolean_to_yes_no(variable.protected)
-            %td.variable-menu
-              = link_to variable.edit_path, class: "btn btn-transparent btn-variable-edit" do
-                %span.sr-only
-                  Update
-                = icon("pencil")
-              = link_to variable.delete_path, class: "btn btn-transparent btn-variable-delete", method: :delete, data: { confirm: "Are you sure?" } do
-                %span.sr-only
-                  Remove
-                = icon("trash")

app/views/groups/settings/ci_cd/show.html.haml

 - breadcrumb_title "CI / CD Settings"
 - page_title "CI / CD"
-= render 'ci/variables/index'
+%h4
+  = _('Secret variables')
+  = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'secret-variables'), target: '_blank', rel: 'noopener noreferrer'
+
+%p
+  = render "ci/variables/content"
+
+= render 'ci/variables/index', save_endpoint: group_variables_path

app/views/groups/variables/show.html.haml

-= render 'ci/variables/show'

app/views/projects/settings/ci_cd/show.html.haml

@@ -29,14 +29,14 @@
 %section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
-      Secret variables
-      = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'secret-variables'), target: '_blank'
+      = _('Secret variables')
+      = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'secret-variables'), target: '_blank', rel: 'noopener noreferrer'
     %button.btn.js-settings-toggle
       = expanded ? 'Collapse' : 'Expand'
-    %p
+    %p.append-bottom-0
       = render "ci/variables/content"
   .settings-content
-    = render 'ci/variables/index'
+    = render 'ci/variables/index', save_endpoint: project_variables_path(@project)
 %section.settings.no-animate{ class: ('expanded' if expanded) }
   .settings-header

app/views/projects/variables/show.html.haml

-= render 'ci/variables/show'

changelogs/unreleased-ee/39118-dynamic-pipeline-variables-fe.yml

+---
+title: Update CI/CD secret variables list to be dynamic and save without reloading
+  the page
+merge_request: 4110
+author:
+type: added

qa/qa/factory/resource/secret_variable.rb

@@ -31,7 +31,7 @@ module QA
               page.fill_variable_key(key)
               page.fill_variable_value(value)
-              page.add_variable
+              page.save_variables
             end
           end
         end

qa/qa/page/project/settings/secret_variables.rb

@@ -5,49 +5,40 @@ module QA
         class SecretVariables < Page::Base
           include Common
-          view 'app/views/ci/variables/_table.html.haml' do
-            element :variable_key, '.variable-key'
-            element :variable_value, '.variable-value'
+          view 'app/views/ci/variables/_variable_row.html.haml' do
+            element :variable_key, '.js-ci-variable-input-key'
+            element :variable_value, '.js-ci-variable-input-value'
           end
           view 'app/views/ci/variables/_index.html.haml' do
-            element :add_new_variable, 'btn_text: "Add new variable"'
-          end
-
-          view 'app/assets/javascripts/behaviors/secret_values.js' do
-            element :reveal_value, 'Reveal value'
-            element :hide_value, 'Hide value'
+            element :save_variables, '.js-secret-variables-save-button'
           end
           def fill_variable_key(key)
-            fill_in 'variable_key', with: key
+            page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do
+              page.find('.js-ci-variable-input-key').set(key)
+            end
           end
           def fill_variable_value(value)
-            fill_in 'variable_value', with: value
+            page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do
+              page.find('.js-ci-variable-input-value').set(value)
+            end
           end
-          def add_variable
-            click_on 'Add new variable'
+          def save_variables
+            click_button('Save variables')
           end
           def variable_key
-            page.find('.variable-key').text
-          end
-
-          def variable_value
-            reveal_value do
-              page.find('.variable-value').text
+            page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do
+              page.find('.js-ci-variable-input-key').value
             end
           end
-          private
-
-          def reveal_value
-            click_button('Reveal value')
-
-            yield.tap do
-              click_button('Hide value')
+          def variable_value
+            page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do
+              page.find('.js-ci-variable-input-value').value
             end
           end
         end