Merge branch 'jivanvl-remove-ci-variables-table-feature-flag' into 'master'

Remove new_ci_variables_list feature flag

See merge request gitlab-org/gitlab!41412

app/assets/javascripts/ci_variable_list/ajax_variable_list.js

-import { escape } from 'lodash';
-import axios from '../lib/utils/axios_utils';
-import { s__ } from '../locale';
-import { deprecatedCreateFlash as Flash } from '../flash';
-import { parseBoolean } 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,
-    maskableRegex,
-  }) {
-    this.container = container;
-    this.saveButton = saveButton;
-    this.errorBox = errorBox;
-    this.saveEndpoint = saveEndpoint;
-    this.maskableRegex = maskableRegex;
-
-    this.variableList = new VariableList({
-      container: this.container,
-      formField,
-      maskableRegex,
-    });
-
-    this.bindEvents();
-    this.variableList.init();
-  }
-
-  bindEvents() {
-    this.saveButton.addEventListener('click', this.onSaveClicked.bind(this));
-  }
-
-  onSaveClicked() {
-    const loadingIcon = this.saveButton.querySelector('.js-ci-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);
-          this.variableList.hideValues();
-        } 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 occurred 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 (parseBoolean(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/components/ci_environments_dropdown.vue

@@ -60,7 +60,7 @@ export default {
 </script>
 <template>
   <gl-dropdown :text="value">
-    <gl-search-box-by-type v-model.trim="searchTerm" />
+    <gl-search-box-by-type v-model.trim="searchTerm" data-testid="ci-environment-search" />
     <gl-dropdown-item
       v-for="environment in filteredResults"
       :key="environment"
@@ -75,7 +75,7 @@ export default {
     }}</gl-dropdown-item>
     <template v-if="shouldRenderCreateButton">
       <gl-dropdown-divider />
-      <gl-dropdown-item @click="createClicked">
+      <gl-dropdown-item data-testid="create-wildcard-button" @click="createClicked">
         {{ composedCreateButtonLabel }}
       </gl-dropdown-item>
     </template>

app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue

@@ -236,6 +236,7 @@ export default {
           :label="__('Environment scope')"
           label-for="ci-variable-env"
           class="w-50"
+          data-testid="environment-scope"
         >
           <ci-environments-dropdown
             class="w-100"
@@ -247,7 +248,11 @@ export default {
       </div>
 
       <gl-form-group :label="__('Flags')" label-for="ci-variable-flags">
-        <gl-form-checkbox v-model="protected_variable" class="mb-0">
+        <gl-form-checkbox
+          v-model="protected_variable"
+          class="mb-0"
+          data-testid="ci-variable-protected-checkbox"
+        >
           {{ __('Protect variable') }}
           <gl-link target="_blank" :href="protectedEnvironmentVariablesLink">
             <gl-icon name="question" :size="12" />
@@ -261,6 +266,7 @@ export default {
           ref="masked-ci-variable"
           v-model="masked"
           data-qa-selector="ci_variable_masked_checkbox"
+          data-testid="ci-variable-masked-checkbox"
         >
           {{ __('Mask variable') }}
           <gl-link target="_blank" :href="maskedEnvironmentVariablesLink">

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

 import initSettingsPanels from '~/settings_panels';
-import AjaxVariableList from '~/ci_variable_list/ajax_variable_list';
 import initVariableList from '~/ci_variable_list';
 import initFilteredSearch from '~/pages/search/init_filtered_search';
 import GroupRunnersFilteredSearchTokenKeys from '~/filtered_search/group_runners_filtered_search_token_keys';
@@ -17,19 +16,6 @@ document.addEventListener('DOMContentLoaded', () => {
     useDefaultState: false,
   });
 
-  if (gon.features.newVariablesUi) {
-    initVariableList();
-  } else {
-    const variableListEl = document.querySelector('.js-ci-variable-list-section');
-    // eslint-disable-next-line no-new
-    new AjaxVariableList({
-      container: variableListEl,
-      saveButton: variableListEl.querySelector('.js-ci-variables-save-button'),
-      errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
-      saveEndpoint: variableListEl.dataset.saveEndpoint,
-      maskableRegex: variableListEl.dataset.maskableRegex,
-    });
-  }
-
   initSharedRunnersForm();
+  initVariableList();
 });

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';
 import registrySettingsApp from '~/registry/settings/registry_settings_bundle';
 import initVariableList from '~/ci_variable_list';
 import initDeployFreeze from '~/deploy_freeze';
@@ -18,19 +17,7 @@ document.addEventListener('DOMContentLoaded', () => {
     runnerTokenSecretValue.init();
   }
 
-  if (gon.features.newVariablesUi) {
   initVariableList();
-  } else {
-    const variableListEl = document.querySelector('.js-ci-variable-list-section');
-    // eslint-disable-next-line no-new
-    new AjaxVariableList({
-      container: variableListEl,
-      saveButton: variableListEl.querySelector('.js-ci-variables-save-button'),
-      errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
-      saveEndpoint: variableListEl.dataset.saveEndpoint,
-      maskableRegex: variableListEl.dataset.maskableRegex,
-    });
-  }
 
   // hide extra auto devops settings based checkbox state
   const autoDevOpsExtraSettings = document.querySelector('.js-extra-settings');

app/controllers/groups/settings/ci_cd_controller.rb

@@ -8,9 +8,6 @@ module Groups
       skip_cross_project_access_check :show
       before_action :authorize_admin_group!
       before_action :authorize_update_max_artifacts_size!, only: [:update]
-      before_action do
-        push_frontend_feature_flag(:new_variables_ui, @group, default_enabled: true)
-      end
       before_action :define_variables, only: [:show]
 
       feature_category :continuous_integration

app/controllers/projects/settings/ci_cd_controller.rb

@@ -8,7 +8,6 @@ module Projects
       before_action :authorize_admin_pipeline!
       before_action :define_variables
       before_action do
-        push_frontend_feature_flag(:new_variables_ui, @project, default_enabled: true)
         push_frontend_feature_flag(:ajax_new_deploy_token, @project)
       end
 

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

@@ -5,10 +5,9 @@
     - link_start = '<a href="%{url}">'.html_safe % { url: help_page_path('ci/variables/README', anchor: 'protect-a-custom-variable') }
     = s_('Environment variables are configured by your administrator to be %{link_start}protected%{link_end} by default').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
 
-- if Feature.enabled?(:new_variables_ui, @project || @group, default_enabled: true)
-  - is_group = !@group.nil?
+- is_group = !@group.nil?
 
-  #js-ci-project-variables{ data: { endpoint: save_endpoint,
+#js-ci-project-variables{ data: { endpoint: save_endpoint,
   project_id: @project&.id || '',
   group: is_group.to_s,
   maskable_regex: ci_variable_maskable_regex,
@@ -19,28 +18,7 @@
   aws_tip_learn_link: help_page_path('ci/cloud_deployment/index.md', anchor: 'aws'),
   protected_environment_variables_link: help_page_path('ci/variables/README', anchor: 'protect-a-custom-variable'),
   masked_environment_variables_link: help_page_path('ci/variables/README', anchor: 'mask-a-custom-variable'),
-  } }
-
-- else
-  .row
-    .col-lg-12.js-ci-variable-list-section{ data: { save_endpoint: save_endpoint, maskable_regex: ci_variable_maskable_regex } }
-      .hide.gl-alert.gl-alert-danger.js-ci-variable-error-box
-
-      %ul.ci-variable-list
-        = render 'ci/variables/variable_header'
-        - @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-ci-variables-save-button{ type: 'button' }
-          %span.hide.js-ci-variables-save-loading-icon
-            .spinner.spinner-light.mr-1
-          = _('Save variables')
-        %button.btn.btn-info.btn-inverted.gl-ml-3.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)
+} }
 
 - if !@group && @project.group
   .settings-header.border-top.prepend-top-20

changelogs/unreleased/jivanvl-remove-ci-variables-table-feature-flag.yml

+---
+title: Remove new_variables_ui feature flag
+merge_request: 41412
+author:
+type: other

config/feature_flags/development/new_variables_ui.yml

----
-name: new_variables_ui
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/25260
-rollout_issue_url:
-group: group::continuous integration
-type: development
-default_enabled: true

ee/spec/frontend/ci_variable_list/ci_variable_list_ee_spec.js

-import $ from 'jquery';
-import VariableList from '~/ci_variable_list/ci_variable_list';
-
-describe('VariableList (EE features)', () => {
-  preloadFixtures('projects/ci_cd_settings.html');
-
-  let $wrapper;
-  let variableList;
-
-  describe('with all inputs(key, value, protected, environment)', () => {
-    beforeEach(() => {
-      loadFixtures('projects/ci_cd_settings.html');
-      $wrapper = $('.js-ci-variable-list-section');
-
-      variableList = new VariableList({
-        container: $wrapper,
-        formField: 'variables',
-      });
-      variableList.init();
-    });
-
-    describe('environment dropdown', () => {
-      function addRowByNewEnvironment(newEnv) {
-        const $row = $wrapper.find('.js-row:last-child');
-
-        // Open the dropdown
-        $row.find('.js-variable-environment-toggle').click();
-
-        // Filter for the new item
-        $row
-          .find('.js-variable-environment-dropdown-wrapper .dropdown-input-field')
-          .val(newEnv)
-          .trigger('input');
-
-        // Create the new item
-        $row.find('.js-variable-environment-dropdown-wrapper .js-dropdown-create-new-item').click();
-      }
-
-      it('should add another row when editing the last rows environment dropdown', () => {
-        addRowByNewEnvironment('someenv');
-
-        jest.runOnlyPendingTimers();
-
-        expect($wrapper.find('.js-row')).toHaveLength(2);
-
-        // Check for the correct default in the new row
-        const $environmentInput = $wrapper
-          .find('.js-row:last-child')
-          .find('input[name="variables[variables_attributes][][environment_scope]"]');
-
-        expect($environmentInput.val()).toBe('*');
-      });
-
-      it('should update dropdown with new environment values and remove values when row is removed', () => {
-        addRowByNewEnvironment('someenv');
-
-        const $row = $wrapper.find('.js-row:last-child');
-        $row.find('.js-variable-environment-toggle').click();
-
-        jest.runOnlyPendingTimers();
-
-        const $dropdownItemsBeforeRemove = $row.find(
-          '.js-variable-environment-dropdown-wrapper .dropdown-content a',
-        );
-
-        expect($dropdownItemsBeforeRemove).toHaveLength(2);
-        expect($dropdownItemsBeforeRemove[0].textContent.trim()).toBe('someenv');
-        expect($dropdownItemsBeforeRemove[1].textContent.trim()).toBe('* (All environments)');
-
-        $wrapper.find('.js-row-remove-button').trigger('click');
-
-        expect($wrapper.find('.js-row')).toHaveLength(0);
-
-        jest.runOnlyPendingTimers();
-
-        const $dropdownItemsAfterRemove = $row.find(
-          '.js-variable-environment-dropdown-wrapper .dropdown-content a',
-        );
-
-        expect($dropdownItemsAfterRemove).toHaveLength(1);
-        expect($dropdownItemsAfterRemove[0].textContent.trim()).toBe('* (All environments)');
-      });
-    });
-  });
-});

locale/gitlab.pot

@@ -5321,9 +5321,6 @@ msgstr ""
 msgid "CiVariable|Create wildcard"
 msgstr ""
 
-msgid "CiVariable|Error occurred while saving variables"
-msgstr ""
-
 msgid "CiVariable|Masked"
 msgstr ""
 
@@ -5342,9 +5339,6 @@ msgstr ""
 msgid "CiVariable|Toggle protected"
 msgstr ""
 
-msgid "CiVariable|Validation failed"
-msgstr ""
-
 msgid "Classification Label (optional)"
 msgstr ""
 
@@ -23050,9 +23044,6 @@ msgstr ""
 msgid "Save space and find tags in the Container Registry more easily. Enable the cleanup policy to remove stale tags and keep only the ones you need."
 msgstr ""
 
-msgid "Save variables"
-msgstr ""
-
 msgid "Saved scan settings and target site settings which are reusable."
 msgstr ""
 

spec/features/group_variables_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe 'Group variables', :js do
   before do
     group.add_owner(user)
     gitlab_sign_in(user)
-    stub_feature_flags(new_variables_ui: false)
+    wait_for_requests
     visit page_path
   end
 

spec/features/project_group_variables_spec.rb

@@ -24,7 +24,6 @@ RSpec.describe 'Project group variables', :js do
     sign_in(user)
     project.add_maintainer(user)
     group.add_owner(user)
-    stub_feature_flags(new_variables_ui: false)
   end
 
   it 'project in group shows inherited vars from ancestor group' do
@@ -53,9 +52,13 @@ RSpec.describe 'Project group variables', :js do
 
   it 'project origin keys link to ancestor groups ci_cd settings' do
     visit project_path
+
     find('.group-origin-link').click
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-key').value).to eq(key1)
+
+    wait_for_requests
+
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:nth-child(1) [data-label="Key"]').text).to eq(key1)
     end
   end
 end

spec/features/project_variables_spec.rb

@@ -12,32 +12,29 @@ RSpec.describe 'Project variables', :js do
     sign_in(user)
     project.add_maintainer(user)
     project.variables << variable
-    stub_feature_flags(new_variables_ui: false)
     visit page_path
   end
 
   it_behaves_like 'variable list'
 
-  it 'adds new variable with a special environment scope' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('somekey')
-      find('.js-ci-variable-input-value').set('somevalue')
+  it 'adds a new variable with an environment scope' do
+    click_button('Add Variable')
 
-      find('.js-variable-environment-toggle').click
-      find('.js-variable-environment-dropdown-wrapper .dropdown-input-field').set('review/*')
-      find('.js-variable-environment-dropdown-wrapper .js-dropdown-create-new-item').click
+    page.within('#add-ci-variable') do
+      find('[data-qa-selector="ci_variable_key_field"] input').set('akey')
+      find('#ci-variable-value').set('akey_value')
+      find('[data-testid="environment-scope"]').click
+      find_button('clear').click
+      find('[data-testid="ci-environment-search"]').set('review/*')
+      find('[data-testid="create-wildcard-button"]').click
 
-      expect(find('input[name="variables[variables_attributes][][environment_scope]"]', visible: false).value).to eq('review/*')
+      click_button('Add variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
-
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-key').value).to eq('somekey')
-      expect(page).to have_content('review/*')
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:first-child [data-label="Environments"]').text).to eq('review/*')
     end
   end
 end

spec/features/projects/settings/registry_settings_spec.rb

@@ -18,7 +18,6 @@ RSpec.describe 'Project > Settings > CI/CD > Container registry tag expiration p
 
     sign_in(user)
     stub_container_registry_config(enabled: container_registry_enabled)
-    stub_feature_flags(new_variables_ui: false)
   end
 
   context 'as owner' do

spec/frontend/ci_variable_list/ci_variable_list/ajax_variable_list_spec.js

-import $ from 'jquery';
-import MockAdapter from 'axios-mock-adapter';
-import axios from '~/lib/utils/axios_utils';
-import AjaxFormVariableList from '~/ci_variable_list/ajax_variable_list';
-
-const VARIABLE_PATCH_ENDPOINT = 'http://test.host/frontend-fixtures/builds-project/-/variables';
-const HIDE_CLASS = 'hide';
-
-describe('AjaxFormVariableList', () => {
-  preloadFixtures('projects/ci_cd_settings.html');
-  preloadFixtures('projects/ci_cd_settings_with_variables.html');
-
-  let container;
-  let saveButton;
-  let errorBox;
-
-  let mock;
-  let ajaxVariableList;
-
-  beforeEach(() => {
-    loadFixtures('projects/ci_cd_settings.html');
-    container = document.querySelector('.js-ci-variable-list-section');
-
-    mock = new MockAdapter(axios);
-
-    const ajaxVariableListEl = document.querySelector('.js-ci-variable-list-section');
-    saveButton = ajaxVariableListEl.querySelector('.js-ci-variables-save-button');
-    errorBox = container.querySelector('.js-ci-variable-error-box');
-    ajaxVariableList = new AjaxFormVariableList({
-      container,
-      formField: 'variables',
-      saveButton,
-      errorBox,
-      saveEndpoint: container.dataset.saveEndpoint,
-      maskableRegex: container.dataset.maskableRegex,
-    });
-
-    jest.spyOn(ajaxVariableList, 'updateRowsWithPersistedVariables');
-    jest.spyOn(ajaxVariableList.variableList, 'toggleEnableRow');
-  });
-
-  afterEach(() => {
-    mock.restore();
-  });
-
-  describe('onSaveClicked', () => {
-    it('shows loading spinner while waiting for the request', () => {
-      const loadingIcon = saveButton.querySelector('.js-ci-variables-save-loading-icon');
-
-      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => {
-        expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(false);
-
-        return [200, {}];
-      });
-
-      expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
-
-      return ajaxVariableList.onSaveClicked().then(() => {
-        expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true);
-      });
-    });
-
-    it('calls `updateRowsWithPersistedVariables` with the persisted variables', () => {
-      const variablesResponse = [{ id: 1, key: 'foo', value: 'bar' }];
-      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {
-        variables: variablesResponse,
-      });
-
-      return ajaxVariableList.onSaveClicked().then(() => {
-        expect(ajaxVariableList.updateRowsWithPersistedVariables).toHaveBeenCalledWith(
-          variablesResponse,
-        );
-      });
-    });
-
-    it('hides any previous error box', () => {
-      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200);
-
-      expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
-
-      return ajaxVariableList.onSaveClicked().then(() => {
-        expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
-      });
-    });
-
-    it('disables remove buttons while waiting for the request', () => {
-      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => {
-        expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(false);
-
-        return [200, {}];
-      });
-
-      return ajaxVariableList.onSaveClicked().then(() => {
-        expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(true);
-      });
-    });
-
-    it('hides secret values', () => {
-      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {});
-
-      const row = container.querySelector('.js-row');
-      const valueInput = row.querySelector('.js-ci-variable-input-value');
-      const valuePlaceholder = row.querySelector('.js-secret-value-placeholder');
-
-      valueInput.value = 'bar';
-      $(valueInput).trigger('input');
-
-      expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(true);
-      expect(valueInput.classList.contains(HIDE_CLASS)).toBe(false);
-
-      return ajaxVariableList.onSaveClicked().then(() => {
-        expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(false);
-        expect(valueInput.classList.contains(HIDE_CLASS)).toBe(true);
-      });
-    });
-
-    it('shows error box with validation errors', () => {
-      const validationError = 'some validation error';
-      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(400, [validationError]);
-
-      expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
-
-      return ajaxVariableList.onSaveClicked().then(() => {
-        expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(false);
-        expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual(
-          `Validation failed ${validationError}`,
-        );
-      });
-    });
-
-    it('shows flash message when request fails', () => {
-      mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(500);
-
-      expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
-
-      return ajaxVariableList.onSaveClicked().then(() => {
-        expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true);
-      });
-    });
-  });
-
-  describe('updateRowsWithPersistedVariables', () => {
-    beforeEach(() => {
-      loadFixtures('projects/ci_cd_settings_with_variables.html');
-      container = document.querySelector('.js-ci-variable-list-section');
-
-      const ajaxVariableListEl = document.querySelector('.js-ci-variable-list-section');
-      saveButton = ajaxVariableListEl.querySelector('.js-ci-variables-save-button');
-      errorBox = container.querySelector('.js-ci-variable-error-box');
-      ajaxVariableList = new AjaxFormVariableList({
-        container,
-        formField: 'variables',
-        saveButton,
-        errorBox,
-        saveEndpoint: container.dataset.saveEndpoint,
-      });
-    });
-
-    it('removes variable that was removed', () => {
-      expect(container.querySelectorAll('.js-row').length).toBe(3);
-
-      container.querySelector('.js-row-remove-button').click();
-
-      expect(container.querySelectorAll('.js-row').length).toBe(3);
-
-      ajaxVariableList.updateRowsWithPersistedVariables([]);
-
-      expect(container.querySelectorAll('.js-row').length).toBe(2);
-    });
-
-    it('updates new variable row with persisted ID', () => {
-      const row = container.querySelector('.js-row:last-child');
-      const idInput = row.querySelector('.js-ci-variable-input-id');
-      const keyInput = row.querySelector('.js-ci-variable-input-key');
-      const valueInput = row.querySelector('.js-ci-variable-input-value');
-
-      keyInput.value = 'foo';
-      $(keyInput).trigger('input');
-      valueInput.value = 'bar';
-      $(valueInput).trigger('input');
-
-      expect(idInput.value).toEqual('');
-
-      ajaxVariableList.updateRowsWithPersistedVariables([
-        {
-          id: 3,
-          key: 'foo',
-          value: 'bar',
-        },
-      ]);
-
-      expect(idInput.value).toEqual('3');
-      expect(row.dataset.isPersisted).toEqual('true');
-    });
-  });
-
-  describe('maskableRegex', () => {
-    it('takes in the regex provided by the data attribute', () => {
-      expect(container.dataset.maskableRegex).toBe('^[a-zA-Z0-9_+=/@:.-]{8,}$');
-      expect(ajaxVariableList.maskableRegex).toBe(container.dataset.maskableRegex);
-    });
-  });
-});

spec/frontend/ci_variable_list/ci_variable_list/ci_variable_list_spec.js

 import $ from 'jquery';
-import waitForPromises from 'helpers/wait_for_promises';
 import VariableList from '~/ci_variable_list/ci_variable_list';
 
 const HIDE_CLASS = 'hide';
@@ -7,7 +6,6 @@ const HIDE_CLASS = 'hide';
 describe('VariableList', () => {
   preloadFixtures('pipeline_schedules/edit.html');
   preloadFixtures('pipeline_schedules/edit_with_variables.html');
-  preloadFixtures('projects/ci_cd_settings.html');
 
   let $wrapper;
   let variableList;
@@ -113,92 +111,6 @@ describe('VariableList', () => {
     });
   });
 
-  describe('with all inputs(key, value, protected)', () => {
-    beforeEach(() => {
-      loadFixtures('projects/ci_cd_settings.html');
-      $wrapper = $('.js-ci-variable-list-section');
-
-      $wrapper.find('.js-ci-variable-input-protected').attr('data-default', 'false');
-
-      variableList = new VariableList({
-        container: $wrapper,
-        formField: 'variables',
-      });
-      variableList.init();
-    });
-
-    it('should not add another row when editing the last rows protected checkbox', () => {
-      const $row = $wrapper.find('.js-row:last-child');
-      $row.find('.ci-variable-protected-item .js-project-feature-toggle').click();
-
-      return waitForPromises().then(() => {
-        expect($wrapper.find('.js-row').length).toBe(1);
-      });
-    });
-
-    it('should not add another row when editing the last rows masked checkbox', () => {
-      jest.spyOn(variableList, 'checkIfRowTouched');
-      const $row = $wrapper.find('.js-row:last-child');
-      $row.find('.ci-variable-masked-item .js-project-feature-toggle').click();
-
-      return waitForPromises().then(() => {
-        // This validates that we are checking after the event listener has run
-        expect(variableList.checkIfRowTouched).toHaveBeenCalled();
-        expect($wrapper.find('.js-row').length).toBe(1);
-      });
-    });
-
-    describe('validateMaskability', () => {
-      let $row;
-
-      const maskingErrorElement = '.js-row:last-child .masking-validation-error';
-      const clickToggle = () =>
-        $row.find('.ci-variable-masked-item .js-project-feature-toggle').click();
-
-      beforeEach(() => {
-        $row = $wrapper.find('.js-row:last-child');
-      });
-
-      it('has a regex provided via a data attribute', () => {
-        clickToggle();
-
-        expect($wrapper.attr('data-maskable-regex')).toBe('^[a-zA-Z0-9_+=/@:.-]{8,}$');
-      });
-
-      it('allows values that are 8 characters long', () => {
-        $row.find('.js-ci-variable-input-value').val('looooong');
-
-        clickToggle();
-
-        expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
-      });
-
-      it('rejects values that are shorter than 8 characters', () => {
-        $row.find('.js-ci-variable-input-value').val('short');
-
-        clickToggle();
-
-        expect($wrapper.find(maskingErrorElement)).toBeVisible();
-      });
-
-      it('allows values with base 64 characters', () => {
-        $row.find('.js-ci-variable-input-value').val('abcABC123_+=/-');
-
-        clickToggle();
-
-        expect($wrapper.find(maskingErrorElement)).toHaveClass('hide');
-      });
-
-      it('rejects values with other special characters', () => {
-        $row.find('.js-ci-variable-input-value').val('1234567$');
-
-        clickToggle();
-
-        expect($wrapper.find(maskingErrorElement)).toBeVisible();
-      });
-    });
-  });
-
   describe('toggleEnableRow method', () => {
     beforeEach(() => {
       loadFixtures('pipeline_schedules/edit_with_variables.html');
@@ -247,36 +159,4 @@ describe('VariableList', () => {
       expect($wrapper.find('.js-ci-variable-input-key:not([disabled])').length).toBe(3);
     });
   });
-
-  describe('hideValues', () => {
-    beforeEach(() => {
-      loadFixtures('projects/ci_cd_settings.html');
-      $wrapper = $('.js-ci-variable-list-section');
-
-      variableList = new VariableList({
-        container: $wrapper,
-        formField: 'variables',
-      });
-      variableList.init();
-    });
-
-    it('should hide value input and show placeholder stars', () => {
-      const $row = $wrapper.find('.js-row');
-      const $inputValue = $row.find('.js-ci-variable-input-value');
-      const $placeholder = $row.find('.js-secret-value-placeholder');
-
-      $row
-        .find('.js-ci-variable-input-value')
-        .val('foo')
-        .trigger('input');
-
-      expect($placeholder.hasClass(HIDE_CLASS)).toBe(true);
-      expect($inputValue.hasClass(HIDE_CLASS)).toBe(false);
-
-      variableList.hideValues();
-
-      expect($placeholder.hasClass(HIDE_CLASS)).toBe(false);
-      expect($inputValue.hasClass(HIDE_CLASS)).toBe(true);
-    });
-  });
 });

spec/frontend/fixtures/groups.rb

@@ -15,7 +15,6 @@ RSpec.describe 'Groups (JavaScript fixtures)', type: :controller do
   end
 
   before do
-    stub_feature_flags(new_variables_ui: false)
     group.add_maintainer(admin)
     sign_in(admin)
   end
@@ -27,12 +26,4 @@ RSpec.describe 'Groups (JavaScript fixtures)', type: :controller do
       expect(response).to be_successful
     end
   end
-
-  describe Groups::Settings::CiCdController, '(JavaScript fixtures)', type: :controller do
-    it 'groups/ci_cd_settings.html' do
-      get :show, params: { group_id: group }
-
-      expect(response).to be_successful
-    end
-  end
 end

spec/frontend/fixtures/projects.rb

@@ -20,7 +20,6 @@ RSpec.describe 'Projects (JavaScript fixtures)', type: :controller do
   end
 
   before do
-    stub_feature_flags(new_variables_ui: false)
     project.add_maintainer(admin)
     sign_in(admin)
     allow(SecureRandom).to receive(:hex).and_return('securerandomhex:thereisnospoon')
@@ -58,27 +57,4 @@ RSpec.describe 'Projects (JavaScript fixtures)', type: :controller do
       expect(response).to be_successful
     end
   end
-
-  describe Projects::Settings::CiCdController, '(JavaScript fixtures)', type: :controller do
-    it 'projects/ci_cd_settings.html' do
-      get :show, params: {
-        namespace_id: project.namespace.to_param,
-        project_id: project
-      }
-
-      expect(response).to be_successful
-    end
-
-    it 'projects/ci_cd_settings_with_variables.html' do
-      create(:ci_variable, project: project_variable_populated)
-      create(:ci_variable, project: project_variable_populated)
-
-      get :show, params: {
-        namespace_id: project_variable_populated.namespace.to_param,
-        project_id: project_variable_populated
-      }
-
-      expect(response).to be_successful
-    end
-  end
 end

spec/support/shared_examples/features/variable_list_shared_examples.rb

 # frozen_string_literal: true
 
 RSpec.shared_examples 'variable list' do
-  it 'shows list of variables' do
-    page.within('.js-ci-variable-list-section') do
-      expect(first('.js-ci-variable-input-key').value).to eq(variable.key)
+  it 'shows a list of variables' do
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Key"]').text).to eq(variable.key)
     end
   end
 
-  it 'adds new CI variable' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('key')
-      find('.js-ci-variable-input-value').set('key_value')
+  it 'adds a new CI variable' do
+    click_button('Add Variable')
+
+    fill_variable('key', 'key_value') do
+      click_button('Add variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
-
-    # We check the first row because it re-sorts to alphabetical order on refresh
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-key').value).to eq('key')
-      expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value')
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Key"]').text).to eq('key')
     end
   end
 
   it 'adds a new protected variable' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('key')
-      find('.js-ci-variable-input-value').set('key_value')
+    click_button('Add Variable')
 
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true')
+    fill_variable('key', 'key_value') do
+      click_button('Add variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
-
-    # We check the first row because it re-sorts to alphabetical order on refresh
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-key').value).to eq('key')
-      expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value')
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true')
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Key"]').text).to eq('key')
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Protected"] svg[data-testid="mobile-issue-close-icon"]')).to be_present
     end
   end
 
   it 'defaults to unmasked' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('key')
-      find('.js-ci-variable-input-value').set('key_value')
+    click_button('Add Variable')
 
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false')
+    fill_variable('key', 'key_value') do
+      click_button('Add variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
-
-    # We check the first row because it re-sorts to alphabetical order on refresh
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-key').value).to eq('key')
-      expect(find('.js-ci-variable-input-value', visible: false).value).to eq('key_value')
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false')
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Key"]').text).to eq('key')
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Masked"] svg[data-testid="close-icon"]')).to be_present
     end
   end
 
-  context 'defaults to the application setting' do
-    context 'application setting is true' do
-      before do
-        stub_application_setting(protected_ci_variables: true)
-
-        visit page_path
-      end
+  it 'reveals and hides variables' do
+    page.within('.ci-variable-table') do
+      expect(first('.js-ci-variable-row td[data-label="Key"]').text).to eq(variable.key)
+      expect(page).to have_content('*' * 17)
 
-      it 'defaults to protected' do
-        page.within('.js-ci-variable-list-section .js-row:last-child') do
-          find('.js-ci-variable-input-key').set('key')
-        end
+      click_button('Reveal value')
 
-        values = all('.js-ci-variable-input-protected', visible: false).map(&:value)
+      expect(first('.js-ci-variable-row td[data-label="Key"]').text).to eq(variable.key)
+      expect(first('.js-ci-variable-row td[data-label="Value"]').text).to eq(variable.value)
+      expect(page).not_to have_content('*' * 17)
 
-        expect(values).to eq %w(false true true)
-      end
+      click_button('Hide value')
 
-      it 'shows a message regarding the changed default' do
-        expect(page).to have_content 'Environment variables are configured by your administrator to be protected by default'
+      expect(first('.js-ci-variable-row td[data-label="Key"]').text).to eq(variable.key)
+      expect(page).to have_content('*' * 17)
     end
   end
 
-    context 'application setting is false' do
-      before do
-        stub_application_setting(protected_ci_variables: false)
+  it 'deletes a variable' do
+    expect(page).to have_selector('.js-ci-variable-row', count: 1)
 
-        visit page_path
+    page.within('.ci-variable-table') do
+      click_button('Edit')
     end
 
-      it 'defaults to unprotected' do
-        page.within('.js-ci-variable-list-section .js-row:last-child') do
-          find('.js-ci-variable-input-key').set('key')
+    page.within('#add-ci-variable') do
+      click_button('Delete variable')
     end
 
-        values = all('.js-ci-variable-input-protected', visible: false).map(&:value)
+    wait_for_requests
 
-        expect(values).to eq %w(false false false)
+    expect(first('.js-ci-variable-row').text).to eq('There are no variables yet.')
   end
 
-      it 'does not show a message regarding the default' do
-        expect(page).not_to have_content 'Environment variables are configured by your administrator to be protected by default'
-      end
-    end
+  it 'edits a variable' do
+    page.within('.ci-variable-table') do
+      click_button('Edit')
     end
 
-  it 'reveals and hides variables' do
-    page.within('.js-ci-variable-list-section') do
-      expect(first('.js-ci-variable-input-key').value).to eq(variable.key)
-      expect(first('.js-ci-variable-input-value', visible: false).value).to eq(variable.value)
-      expect(page).to have_content('*' * 17)
-
-      click_button('Reveal value')
-
-      expect(first('.js-ci-variable-input-key').value).to eq(variable.key)
-      expect(first('.js-ci-variable-input-value').value).to eq(variable.value)
-      expect(page).not_to have_content('*' * 17)
-
-      click_button('Hide value')
+    page.within('#add-ci-variable') do
+      find('[data-qa-selector="ci_variable_key_field"] input').set('new_key')
 
-      expect(first('.js-ci-variable-input-key').value).to eq(variable.key)
-      expect(first('.js-ci-variable-input-value', visible: false).value).to eq(variable.value)
-      expect(page).to have_content('*' * 17)
+      click_button('Update variable')
     end
-  end
-
-  it 'deletes variable' do
-    page.within('.js-ci-variable-list-section') do
-      expect(page).to have_selector('.js-row', count: 2)
 
-      first('.js-row-remove-button').click
-
-      click_button('Save variables')
     wait_for_requests
 
-      expect(page).to have_selector('.js-row', count: 1)
+    expect(first('.js-ci-variable-row td[data-label="Key"]').text).to eq('new_key')
   end
+
+  it 'edits a variable to be unmasked' do
+    page.within('.ci-variable-table') do
+      click_button('Edit')
     end
 
-  it 'edits variable' do
-    page.within('.js-ci-variable-list-section') do
-      click_button('Reveal value')
+    page.within('#add-ci-variable') do
+      find('[data-testid="ci-variable-protected-checkbox"]').click
+      find('[data-testid="ci-variable-masked-checkbox"]').click
 
-      page.within('.js-row:nth-child(2)') do
-        find('.js-ci-variable-input-key').set('new_key')
-        find('.js-ci-variable-input-value').set('new_value')
+      click_button('Update variable')
     end
 
-      click_button('Save variables')
     wait_for_requests
 
-      visit page_path
-
-      page.within('.js-row:nth-child(2)') do
-        expect(find('.js-ci-variable-input-key').value).to eq('new_key')
-        expect(find('.js-ci-variable-input-value', visible: false).value).to eq('new_value')
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Masked"] svg[data-testid="close-icon"]')).to be_present
     end
   end
+
+  it 'edits a variable to be masked' do
+    page.within('.ci-variable-table') do
+      click_button('Edit')
     end
 
-  it 'edits variable to be protected' do
-    # Create the unprotected variable
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('unprotected_key')
-      find('.js-ci-variable-input-value').set('unprotected_value')
-      find('.ci-variable-protected-item .js-project-feature-toggle').click
+    page.within('#add-ci-variable') do
+      find('[data-testid="ci-variable-masked-checkbox"]').click
 
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false')
+      click_button('Update variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
-
-    # We check the first row because it re-sorts to alphabetical order on refresh
-    page.within('.js-ci-variable-list-section .js-row:nth-child(3)') do
-      find('.ci-variable-protected-item .js-project-feature-toggle').click
-
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true')
+    page.within('.ci-variable-table') do
+      click_button('Edit')
     end
 
-    click_button('Save variables')
-    wait_for_requests
+    page.within('#add-ci-variable') do
+      find('[data-testid="ci-variable-masked-checkbox"]').click
 
-    visit page_path
+      click_button('Update variable')
+    end
 
-    # We check the first row because it re-sorts to alphabetical order on refresh
-    page.within('.js-ci-variable-list-section .js-row:nth-child(3)') do
-      expect(find('.js-ci-variable-input-key').value).to eq('unprotected_key')
-      expect(find('.js-ci-variable-input-value', visible: false).value).to eq('unprotected_value')
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true')
+    page.within('.ci-variable-table') do
+      expect(find('.js-ci-variable-row:nth-child(1) td[data-label="Masked"] svg[data-testid="mobile-issue-close-icon"]')).to be_present
     end
   end
 
-  it 'edits variable to be unprotected' do
-    # Create the protected variable
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('protected_key')
-      find('.js-ci-variable-input-value').set('protected_value')
+  it 'shows a validation error box about duplicate keys' do
+    click_button('Add Variable')
 
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true')
+    fill_variable('key', 'key_value') do
+      click_button('Add variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
-
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      find('.ci-variable-protected-item .js-project-feature-toggle').click
+    click_button('Add Variable')
 
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false')
+    fill_variable('key', 'key_value') do
+      click_button('Add variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
-
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-key').value).to eq('protected_key')
-      expect(find('.js-ci-variable-input-value', visible: false).value).to eq('protected_value')
-      expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false')
-    end
+    expect(find('.flash-container')).to be_present
+    expect(find('.flash-text').text).to have_content('Variables key (key) has already been taken')
   end
 
-  it 'edits variable to be unmasked' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('unmasked_key')
-      find('.js-ci-variable-input-value').set('unmasked_value')
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false')
+  it 'prevents a variable to be added if no values are provided when a variable is set to masked' do
+    click_button('Add Variable')
 
-      find('.ci-variable-masked-item .js-project-feature-toggle').click
+    page.within('#add-ci-variable') do
+      find('[data-qa-selector="ci_variable_key_field"] input').set('empty_mask_key')
+      find('[data-testid="ci-variable-protected-checkbox"]').click
+      find('[data-testid="ci-variable-masked-checkbox"]').click
 
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true')
+      expect(find_button('Add variable', disabled: true)).to be_present
+    end
   end
 
-    click_button('Save variables')
-    wait_for_requests
-
-    visit page_path
-
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true')
-
-      find('.ci-variable-masked-item .js-project-feature-toggle').click
+  it 'shows validation error box about unmaskable values' do
+    click_button('Add Variable')
 
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false')
+    fill_variable('empty_mask_key', '???', protected: true, masked: true) do
+      expect(page).to have_content('This variable can not be masked')
+      expect(find_button('Add variable', disabled: true)).to be_present
+    end
   end
 
-    click_button('Save variables')
-    wait_for_requests
-
-    visit page_path
+  it 'handles multiple edits and a deletion' do
+    # Create two variables
+    click_button('Add Variable')
 
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false')
-    end
+    fill_variable('akey', 'akeyvalue') do
+      click_button('Add variable')
     end
 
-  it 'edits variable to be masked' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('masked_key')
-      find('.js-ci-variable-input-value').set('masked_value')
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('false')
+    wait_for_requests
 
-      find('.ci-variable-masked-item .js-project-feature-toggle').click
+    click_button('Add Variable')
 
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true')
+    fill_variable('zkey', 'zkeyvalue') do
+      click_button('Add variable')
     end
 
-    click_button('Save variables')
     wait_for_requests
 
-    visit page_path
+    expect(page).to have_selector('.js-ci-variable-row', count: 3)
 
-    page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do
-      expect(find('.js-ci-variable-input-masked', visible: false).value).to eq('true')
+    # Remove the `akey` variable
+    page.within('.ci-variable-table') do
+      page.within('.js-ci-variable-row:first-child') do
+        click_button('Edit')
       end
     end
 
-  it 'handles multiple edits and deletion in the middle' do
-    page.within('.js-ci-variable-list-section') do
-      # Create 2 variables
-      page.within('.js-row:last-child') do
-        find('.js-ci-variable-input-key').set('akey')
-        find('.js-ci-variable-input-value').set('akeyvalue')
-      end
-      page.within('.js-row:last-child') do
-        find('.js-ci-variable-input-key').set('zkey')
-        find('.js-ci-variable-input-value').set('zkeyvalue')
+    page.within('#add-ci-variable') do
+      click_button('Delete variable')
     end
 
-      click_button('Save variables')
     wait_for_requests
 
-      expect(page).to have_selector('.js-row', count: 4)
+    # Add another variable
+    click_button('Add Variable')
 
-      # Remove the `akey` variable
-      page.within('.js-row:nth-child(3)') do
-        first('.js-row-remove-button').click
+    fill_variable('ckey', 'ckeyvalue') do
+      click_button('Add variable')
     end
 
-      # Add another variable
-      page.within('.js-row:last-child') do
-        find('.js-ci-variable-input-key').set('ckey')
-        find('.js-ci-variable-input-value').set('ckeyvalue')
+    wait_for_requests
+
+    # expect to find 3 rows of variables in alphabetical order
+    expect(page).to have_selector('.js-ci-variable-row', count: 3)
+    rows = all('.js-ci-variable-row')
+    expect(rows[0].find('td[data-label="Key"]').text).to eq('ckey')
+    expect(rows[1].find('td[data-label="Key"]').text).to eq('test_key')
+    expect(rows[2].find('td[data-label="Key"]').text).to eq('zkey')
   end
 
-      click_button('Save variables')
-      wait_for_requests
+  context 'defaults to the application setting' do
+    context 'application setting is true' do
+      before do
+        stub_application_setting(protected_ci_variables: true)
 
         visit page_path
-
-      # Expect to find 3 variables(4 rows) in alphbetical order
-      expect(page).to have_selector('.js-row', count: 4)
-      row_keys = all('.js-ci-variable-input-key')
-      expect(row_keys[0].value).to eq('ckey')
-      expect(row_keys[1].value).to eq('test_key')
-      expect(row_keys[2].value).to eq('zkey')
-      expect(row_keys[3].value).to eq('')
-    end
       end
 
-  it 'shows validation error box about duplicate keys' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('samekey')
-      find('.js-ci-variable-input-value').set('value123')
+      it 'defaults to protected' do
+        click_button('Add Variable')
+
+        page.within('#add-ci-variable') do
+          expect(find('[data-testid="ci-variable-protected-checkbox"]')).to be_checked
         end
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('samekey')
-      find('.js-ci-variable-input-value').set('value456')
       end
 
-    click_button('Save variables')
-    wait_for_requests
-
-    expect(all('.js-ci-variable-list-section .js-ci-variable-error-box ul li').count).to eq(1)
-
-    # We check the first row because it re-sorts to alphabetical order on refresh
-    page.within('.js-ci-variable-list-section') do
-      expect(find('.js-ci-variable-error-box')).to have_content(/Validation failed Variables have duplicate values \(.+\)/)
+      it 'shows a message regarding the changed default' do
+        expect(page).to have_content 'Environment variables are configured by your administrator to be protected by default'
       end
     end
 
-  it 'shows validation error box about masking empty values' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('empty_value')
-      find('.js-ci-variable-input-value').set('')
-      find('.ci-variable-masked-item .js-project-feature-toggle').click
+    context 'application setting is false' do
+      before do
+        stub_application_setting(protected_ci_variables: false)
+
+        visit page_path
       end
 
-    click_button('Save variables')
-    wait_for_requests
+      it 'defaults to unprotected' do
+        click_button('Add Variable')
 
-    page.within('.js-ci-variable-list-section') do
-      expect(all('.js-ci-variable-error-box ul li').count).to eq(1)
-      expect(find('.js-ci-variable-error-box')).to have_content(/Validation failed Variables value is invalid/)
+        page.within('#add-ci-variable') do
+          expect(find('[data-testid="ci-variable-protected-checkbox"]')).not_to be_checked
         end
       end
 
-  it 'shows validation error box about unmaskable values' do
-    page.within('.js-ci-variable-list-section .js-row:last-child') do
-      find('.js-ci-variable-input-key').set('unmaskable_value')
-      find('.js-ci-variable-input-value').set('???')
-      find('.ci-variable-masked-item .js-project-feature-toggle').click
+      it 'does not show a message regarding the default' do
+        expect(page).not_to have_content 'Environment variables are configured by your administrator to be protected by default'
+      end
+    end
   end
 
-    click_button('Save variables')
-    wait_for_requests
+  def fill_variable(key, value, protected: false, masked: false)
+    page.within('#add-ci-variable') do
+      find('[data-qa-selector="ci_variable_key_field"] input').set(key)
+      find('[data-qa-selector="ci_variable_value_field"]').set(value) if value.present?
+      find('[data-testid="ci-variable-protected-checkbox"]').click if protected
+      find('[data-testid="ci-variable-masked-checkbox"]').click if masked
 
-    page.within('.js-ci-variable-list-section') do
-      expect(all('.js-ci-variable-error-box ul li').count).to eq(1)
-      expect(find('.js-ci-variable-error-box')).to have_content(/Validation failed Variables value is invalid/)
+      yield
     end
   end
 end