diff --git a/app/assets/javascripts/ci_variable_list/ajax_variable_list.js b/app/assets/javascripts/ci_variable_list/ajax_variable_list.js new file mode 100644 index 0000000000000000000000000000000000000000..76f93e5c6bdd4cb4767b41a2d4a0c79fc3cd2423 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/ajax_variable_list.js @@ -0,0 +1,116 @@ +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 => ` +
  • + ${_.escape(errorString)} +
  • + `); + + return ` +

    + ${s__('CiVariable|Validation failed')} +

    + + `; +} + +// 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'); + } + } + }); + } +} diff --git a/app/assets/javascripts/ci_variable_list/ci_variable_list.js b/app/assets/javascripts/ci_variable_list/ci_variable_list.js index e46478ddb98bae90835cb45198187a54a1953e6d..d91789c21923ab560c1fd30825c4bcfbd96f0dba 100644 --- a/app/assets/javascripts/ci_variable_list/ci_variable_list.js +++ b/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 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, diff --git a/app/assets/javascripts/commons/polyfills/element.js b/app/assets/javascripts/commons/polyfills/element.js index 9a1f73bf2ac432928907c8eee3e54760fe70cbe8..b593bde6aa2b8cd641bf3d762135bc6d5be30d6f 100644 --- a/app/assets/javascripts/commons/polyfills/element.js +++ b/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]); diff --git a/app/assets/javascripts/lib/utils/http_status.js b/app/assets/javascripts/lib/utils/http_status.js index 625e53ee9def46f6fe76f0c88dc46adbab21e825..bb15192943169d584a47831a7dac6f6e783a0689 100644 --- a/app/assets/javascripts/lib/utils/http_status.js +++ b/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, }; diff --git a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js index f26c7360fbe02619fc3420e11cd7b4c2914e6dfc..ad79f7e09ac9d1cd304dc5144fb63c9eb069d7d5 100644 --- a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js +++ b/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js @@ -1,11 +1,12 @@ -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, + }); }; diff --git a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js index 18dc1dc03a55417b0e30c20e3ee85f661332724d..a563d0f9961e6ad1ce06f72818693a536cdf3c52 100644 --- a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js +++ b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js @@ -1,9 +1,11 @@ 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, + }); } diff --git a/app/assets/stylesheets/framework/ci_variable_list.scss b/app/assets/stylesheets/framework/ci_variable_list.scss index 8f654ab363ce495f25e86eb26e1ce38e2db80875..ccd36af071f9882d1418ca31c5b0138839811a46 100644 --- a/app/assets/stylesheets/framework/ci_variable_list.scss +++ b/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; + } } diff --git a/app/views/ci/variables/_content.html.haml b/app/views/ci/variables/_content.html.haml index fbfe3e5658892d861a71efdb31ec8af02b3e8d57..d355e7799df5399b558ea10128c8537a49785420 100644 --- a/app/views/ci/variables/_content.html.haml +++ b/app/views/ci/variables/_content.html.haml @@ -1,3 +1 @@ -%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.') diff --git a/app/views/ci/variables/_form.html.haml b/app/views/ci/variables/_form.html.haml deleted file mode 100644 index eebd0955c801061d974cc38d95bc3374c4a05656..0000000000000000000000000000000000000000 --- a/app/views/ci/variables/_form.html.haml +++ /dev/null @@ -1,19 +0,0 @@ -= 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" diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index 6e399fc739290dfc7ef3736e30768a65caf16f79..e402801a7767cd192862a681eee0dfc43caf2f19 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/app/views/ci/variables/_index.html.haml @@ -1,16 +1,20 @@ -.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) diff --git a/app/views/ci/variables/_show.html.haml b/app/views/ci/variables/_show.html.haml deleted file mode 100644 index 6d75ae96124f0f4b82e1bd9c704d5ede8e2a3ebf..0000000000000000000000000000000000000000 --- a/app/views/ci/variables/_show.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -- 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" diff --git a/app/views/ci/variables/_table.html.haml b/app/views/ci/variables/_table.html.haml deleted file mode 100644 index 2298930d0c76ec185109b501159e56916d0a91f9..0000000000000000000000000000000000000000 --- a/app/views/ci/variables/_table.html.haml +++ /dev/null @@ -1,32 +0,0 @@ -.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") diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml index 472da2a6a720008acea3822ee5d8f4dc003f8ef8..dd82922ec550a6eedef8cde3106bea781e851a38 100644 --- a/app/views/groups/settings/ci_cd/show.html.haml +++ b/app/views/groups/settings/ci_cd/show.html.haml @@ -1,4 +1,11 @@ - 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 diff --git a/app/views/groups/variables/show.html.haml b/app/views/groups/variables/show.html.haml deleted file mode 100644 index df533952b76ac2624929392f6bdab78347ee0679..0000000000000000000000000000000000000000 --- a/app/views/groups/variables/show.html.haml +++ /dev/null @@ -1 +0,0 @@ -= render 'ci/variables/show' diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index 664a455469238cf5caa6fe52a1532abaa876c1e6..756f31f91d942a461e154a46284149f36f815ffc 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/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 diff --git a/app/views/projects/variables/show.html.haml b/app/views/projects/variables/show.html.haml deleted file mode 100644 index df533952b76ac2624929392f6bdab78347ee0679..0000000000000000000000000000000000000000 --- a/app/views/projects/variables/show.html.haml +++ /dev/null @@ -1 +0,0 @@ -= render 'ci/variables/show' diff --git a/changelogs/unreleased-ee/39118-dynamic-pipeline-variables-fe.yml b/changelogs/unreleased-ee/39118-dynamic-pipeline-variables-fe.yml new file mode 100644 index 0000000000000000000000000000000000000000..a38b447e345dbe13ccb1f3d6f2276bd8f0fa4801 --- /dev/null +++ b/changelogs/unreleased-ee/39118-dynamic-pipeline-variables-fe.yml @@ -0,0 +1,6 @@ +--- +title: Update CI/CD secret variables list to be dynamic and save without reloading + the page +merge_request: 4110 +author: +type: added diff --git a/doc/ci/variables/img/secret_variables.png b/doc/ci/variables/img/secret_variables.png index f70935069d9604009d5e0458b63343da0dbf7e8c..3c1aa361dc2b38759fa6619dcd2c8ffc788b8929 100644 Binary files a/doc/ci/variables/img/secret_variables.png and b/doc/ci/variables/img/secret_variables.png differ diff --git a/qa/qa/factory/resource/secret_variable.rb b/qa/qa/factory/resource/secret_variable.rb index 54ef4d8d9643493500509a1406477ad7ec84a767..af0fa8af2df58b0bf745d53ff2f7a08d1141b473 100644 --- a/qa/qa/factory/resource/secret_variable.rb +++ b/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 diff --git a/qa/qa/page/project/settings/secret_variables.rb b/qa/qa/page/project/settings/secret_variables.rb index e3bfbfcf0802fe20ebb511453da7e98ec2387a7e..fea4acb389a0d3c32fce9522beadadfd1beaa949 100644 --- a/qa/qa/page/project/settings/secret_variables.rb +++ b/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 diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb index e9b375f4c9481f1190368fa9bc73d07fd9855492..f786380757243ed15ced3a8db982b698be924f21 100644 --- a/spec/features/group_variables_spec.rb +++ b/spec/features/group_variables_spec.rb @@ -3,76 +3,15 @@ require 'spec_helper' feature 'Group variables', :js do let(:user) { create(:user) } let(:group) { create(:group) } + let!(:variable) { create(:ci_group_variable, key: 'test_key', value: 'test value', group: group) } + let(:page_path) { group_settings_ci_cd_path(group) } background do group.add_master(user) gitlab_sign_in(user) - end - - context 'when user creates a new variable' do - background do - visit group_settings_ci_cd_path(group) - fill_in 'variable_key', with: 'AAA' - fill_in 'variable_value', with: 'AAA123' - find(:css, "#variable_protected").set(true) - click_on 'Add new variable' - end - - scenario 'user sees the created variable' do - page.within('.variables-table') do - expect(find(".variable-key")).to have_content('AAA') - expect(find(".variable-value")).to have_content('******') - expect(find(".variable-protected")).to have_content('Yes') - end - click_on 'Reveal value' - page.within('.variables-table') do - expect(find(".variable-value")).to have_content('AAA123') - end - end - end - - context 'when user edits a variable' do - background do - create(:ci_group_variable, key: 'AAA', value: 'AAA123', protected: true, - group: group) - - visit group_settings_ci_cd_path(group) - page.within('.variable-menu') do - click_on 'Update' - end - - fill_in 'variable_key', with: 'BBB' - fill_in 'variable_value', with: 'BBB123' - find(:css, "#variable_protected").set(false) - click_on 'Save variable' - end - - scenario 'user sees the updated variable' do - page.within('.variables-table') do - expect(find(".variable-key")).to have_content('BBB') - expect(find(".variable-value")).to have_content('******') - expect(find(".variable-protected")).to have_content('No') - end - end + visit page_path end - context 'when user deletes a variable' do - background do - create(:ci_group_variable, key: 'BBB', value: 'BBB123', protected: false, - group: group) - - visit group_settings_ci_cd_path(group) - - page.within('.variable-menu') do - page.accept_alert 'Are you sure?' do - click_on 'Remove' - end - end - end - - scenario 'user does not see the deleted variable' do - expect(page).to have_no_css('.variables-table') - end - end + it_behaves_like 'variable list' end diff --git a/spec/features/project_variables_spec.rb b/spec/features/project_variables_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0ba2224359ac1786b5e9fb86006a2d8b40ce7181 --- /dev/null +++ b/spec/features/project_variables_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe 'Project variables', :js do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:variable) { create(:ci_variable, key: 'test_key', value: 'test value') } + let(:page_path) { project_settings_ci_cd_path(project) } + + before do + sign_in(user) + project.add_master(user) + project.variables << variable + + visit page_path + end + + it_behaves_like 'variable list' +end diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb deleted file mode 100644 index 79ca2b4bb4a26de32802f6b000dfaece11f966b9..0000000000000000000000000000000000000000 --- a/spec/features/variables_spec.rb +++ /dev/null @@ -1,145 +0,0 @@ -require 'spec_helper' - -describe 'Project variables', :js do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:variable) { create(:ci_variable, key: 'test_key', value: 'test value') } - - before do - sign_in(user) - project.add_master(user) - project.variables << variable - - visit project_settings_ci_cd_path(project) - end - - it 'shows list of variables' do - page.within('.variables-table') do - expect(page).to have_content(variable.key) - end - end - - it 'adds new secret variable' do - fill_in('variable_key', with: 'key') - fill_in('variable_value', with: 'key value') - click_button('Add new variable') - - expect(page).to have_content('Variable was successfully created.') - page.within('.variables-table') do - expect(page).to have_content('key') - expect(page).to have_content('No') - end - end - - it 'adds empty variable' do - fill_in('variable_key', with: 'new_key') - fill_in('variable_value', with: '') - click_button('Add new variable') - - expect(page).to have_content('Variable was successfully created.') - page.within('.variables-table') do - expect(page).to have_content('new_key') - end - end - - it 'adds new protected variable' do - fill_in('variable_key', with: 'key') - fill_in('variable_value', with: 'value') - check('Protected') - click_button('Add new variable') - - expect(page).to have_content('Variable was successfully created.') - page.within('.variables-table') do - expect(page).to have_content('key') - expect(page).to have_content('Yes') - end - end - - it 'reveals and hides new variable' do - fill_in('variable_key', with: 'key') - fill_in('variable_value', with: 'key value') - click_button('Add new variable') - - page.within('.variables-table') do - expect(page).to have_content('key') - expect(page).to have_content('******') - end - - click_button('Reveal values') - - page.within('.variables-table') do - expect(page).to have_content('key') - expect(page).to have_content('key value') - end - - click_button('Hide values') - - page.within('.variables-table') do - expect(page).to have_content('key') - expect(page).to have_content('******') - end - end - - it 'deletes variable' do - page.within('.variables-table') do - accept_confirm { click_on 'Remove' } - end - - expect(page).not_to have_selector('variables-table') - end - - it 'edits variable' do - page.within('.variables-table') do - click_on 'Update' - end - - expect(page).to have_content('Update variable') - fill_in('variable_key', with: 'key') - fill_in('variable_value', with: 'key value') - click_button('Save variable') - - expect(page).to have_content('Variable was successfully updated.') - expect(project.variables(true).first.value).to eq('key value') - end - - it 'edits variable with empty value' do - page.within('.variables-table') do - click_on 'Update' - end - - expect(page).to have_content('Update variable') - fill_in('variable_value', with: '') - click_button('Save variable') - - expect(page).to have_content('Variable was successfully updated.') - expect(project.variables(true).first.value).to eq('') - end - - it 'edits variable to be protected' do - page.within('.variables-table') do - click_on 'Update' - end - - expect(page).to have_content('Update variable') - check('Protected') - click_button('Save variable') - - expect(page).to have_content('Variable was successfully updated.') - expect(project.variables(true).first).to be_protected - end - - it 'edits variable to be unprotected' do - project.variables.first.update(protected: true) - - page.within('.variables-table') do - click_on 'Update' - end - - expect(page).to have_content('Update variable') - uncheck('Protected') - click_button('Save variable') - - expect(page).to have_content('Variable was successfully updated.') - expect(project.variables(true).first).not_to be_protected - end -end diff --git a/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js b/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..5b9cdceee7116c6ff9e62b405c4a3d3e78551d6e --- /dev/null +++ b/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js @@ -0,0 +1,189 @@ +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'; + +describe('AjaxFormVariableList', () => { + preloadFixtures('projects/ci_cd_settings.html.raw'); + preloadFixtures('projects/ci_cd_settings_with_variables.html.raw'); + + let container; + let saveButton; + let errorBox; + + let mock; + let ajaxVariableList; + + beforeEach(() => { + loadFixtures('projects/ci_cd_settings.html.raw'); + 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-secret-variables-save-button'); + errorBox = container.querySelector('.js-ci-variable-error-box'); + ajaxVariableList = new AjaxFormVariableList({ + container, + formField: 'variables', + saveButton, + errorBox, + saveEndpoint: container.dataset.saveEndpoint, + }); + + spyOn(ajaxVariableList, 'updateRowsWithPersistedVariables').and.callThrough(); + spyOn(ajaxVariableList.variableList, 'toggleEnableRow').and.callThrough(); + }); + + afterEach(() => { + mock.restore(); + }); + + describe('onSaveClicked', () => { + it('shows loading spinner while waiting for the request', (done) => { + const loadingIcon = saveButton.querySelector('.js-secret-variables-save-loading-icon'); + + mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => { + expect(loadingIcon.classList.contains('hide')).toEqual(false); + + return [200, {}]; + }); + + expect(loadingIcon.classList.contains('hide')).toEqual(true); + + ajaxVariableList.onSaveClicked() + .then(() => { + expect(loadingIcon.classList.contains('hide')).toEqual(true); + }) + .then(done) + .catch(done.fail); + }); + + it('calls `updateRowsWithPersistedVariables` with the persisted variables', (done) => { + const variablesResponse = [{ id: 1, key: 'foo', value: 'bar' }]; + mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, { + variables: variablesResponse, + }); + + ajaxVariableList.onSaveClicked() + .then(() => { + expect(ajaxVariableList.updateRowsWithPersistedVariables) + .toHaveBeenCalledWith(variablesResponse); + }) + .then(done) + .catch(done.fail); + }); + + it('hides any previous error box', (done) => { + mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200); + + expect(errorBox.classList.contains('hide')).toEqual(true); + + ajaxVariableList.onSaveClicked() + .then(() => { + expect(errorBox.classList.contains('hide')).toEqual(true); + }) + .then(done) + .catch(done.fail); + }); + + it('disables remove buttons while waiting for the request', (done) => { + mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => { + expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(false); + + return [200, {}]; + }); + + ajaxVariableList.onSaveClicked() + .then(() => { + expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(true); + }) + .then(done) + .catch(done.fail); + }); + + it('shows error box with validation errors', (done) => { + const validationError = 'some validation error'; + mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(400, [ + validationError, + ]); + + expect(errorBox.classList.contains('hide')).toEqual(true); + + ajaxVariableList.onSaveClicked() + .then(() => { + expect(errorBox.classList.contains('hide')).toEqual(false); + expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual(`Validation failed ${validationError}`); + }) + .then(done) + .catch(done.fail); + }); + + it('shows flash message when request fails', (done) => { + mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(500); + + expect(errorBox.classList.contains('hide')).toEqual(true); + + ajaxVariableList.onSaveClicked() + .then(() => { + expect(errorBox.classList.contains('hide')).toEqual(true); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('updateRowsWithPersistedVariables', () => { + beforeEach(() => { + loadFixtures('projects/ci_cd_settings_with_variables.html.raw'); + container = document.querySelector('.js-ci-variable-list-section'); + + const ajaxVariableListEl = document.querySelector('.js-ci-variable-list-section'); + saveButton = ajaxVariableListEl.querySelector('.js-secret-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.dispatchEvent(new Event('input')); + valueInput.value = 'bar'; + valueInput.dispatchEvent(new Event('input')); + + expect(idInput.value).toEqual(''); + + ajaxVariableList.updateRowsWithPersistedVariables([{ + id: 3, + key: 'foo', + value: 'bar', + }]); + + expect(idInput.value).toEqual('3'); + expect(row.dataset.isPersisted).toEqual('true'); + }); + }); +}); diff --git a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js index 0170ab458d4ff42a58224554c1500f14073dc7ba..6ab7b50e035e8a505e17d3d3db837d2b5323bc65 100644 --- a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js +++ b/spec/javascripts/ci_variable_list/ci_variable_list_spec.js @@ -4,6 +4,7 @@ import getSetTimeoutPromise from '../helpers/set_timeout_promise_helper'; describe('VariableList', () => { preloadFixtures('pipeline_schedules/edit.html.raw'); preloadFixtures('pipeline_schedules/edit_with_variables.html.raw'); + preloadFixtures('projects/ci_cd_settings.html.raw'); let $wrapper; let variableList; @@ -105,37 +106,8 @@ describe('VariableList', () => { describe('with all inputs(key, value, protected)', () => { beforeEach(() => { - // This markup will be replaced with a fixture when we can render the - // CI/CD settings page with the new dynamic variable list in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/4110 - $wrapper = $(`
    - - -
    `); + loadFixtures('projects/ci_cd_settings.html.raw'); + $wrapper = $('.js-ci-variable-list-section'); variableList = new VariableList({ container: $wrapper, @@ -160,4 +132,51 @@ describe('VariableList', () => { .catch(done.fail); }); }); + + describe('toggleEnableRow method', () => { + beforeEach(() => { + loadFixtures('pipeline_schedules/edit_with_variables.html.raw'); + $wrapper = $('.js-ci-variable-list-section'); + + variableList = new VariableList({ + container: $wrapper, + formField: 'variables', + }); + variableList.init(); + }); + + it('should disable all key inputs', () => { + expect($wrapper.find('.js-ci-variable-input-key:not([disabled])').length).toBe(3); + + variableList.toggleEnableRow(false); + + expect($wrapper.find('.js-ci-variable-input-key[disabled]').length).toBe(3); + }); + + it('should disable all remove buttons', () => { + expect($wrapper.find('.js-row-remove-button:not([disabled])').length).toBe(3); + + variableList.toggleEnableRow(false); + + expect($wrapper.find('.js-row-remove-button[disabled]').length).toBe(3); + }); + + it('should enable all remove buttons', () => { + variableList.toggleEnableRow(false); + expect($wrapper.find('.js-row-remove-button[disabled]').length).toBe(3); + + variableList.toggleEnableRow(true); + + expect($wrapper.find('.js-row-remove-button:not([disabled])').length).toBe(3); + }); + + it('should enable all key inputs', () => { + variableList.toggleEnableRow(false); + expect($wrapper.find('.js-ci-variable-input-key[disabled]').length).toBe(3); + + variableList.toggleEnableRow(true); + + expect($wrapper.find('.js-ci-variable-input-key:not([disabled])').length).toBe(3); + }); + }); }); diff --git a/spec/javascripts/fixtures/groups.rb b/spec/javascripts/fixtures/groups.rb new file mode 100644 index 0000000000000000000000000000000000000000..35be52fbf97490912f2c70771acd1e343a4a6d56 --- /dev/null +++ b/spec/javascripts/fixtures/groups.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe 'Groups (JavaScript fixtures)', type: :controller do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:group) { create(:group, name: 'frontend-fixtures-group' )} + + render_views + + before(:all) do + clean_frontend_fixtures('groups/') + end + + before do + group.add_master(admin) + sign_in(admin) + end + + describe Groups::Settings::CiCdController, '(JavaScript fixtures)', type: :controller do + it 'groups/ci_cd_settings.html.raw' do |example| + get :show, + group_id: group + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end + end +end diff --git a/spec/javascripts/fixtures/projects.rb b/spec/javascripts/fixtures/projects.rb index 2a100e7fab55761a4e52ffe1ccead11382ac4754..b344b389241d5c6e2250178fac59c179307fb8ef 100644 --- a/spec/javascripts/fixtures/projects.rb +++ b/spec/javascripts/fixtures/projects.rb @@ -1,11 +1,14 @@ require 'spec_helper' -describe ProjectsController, '(JavaScript fixtures)', type: :controller do +describe 'Projects (JavaScript fixtures)', type: :controller do include JavaScriptFixturesHelpers let(:admin) { create(:admin) } let(:namespace) { create(:namespace, name: 'frontend-fixtures' )} let(:project) { create(:project, namespace: namespace, path: 'builds-project') } + let(:project_variable_populated) { create(:project, namespace: namespace, path: 'builds-project2') } + let!(:variable1) { create(:ci_variable, project: project_variable_populated) } + let!(:variable2) { create(:ci_variable, project: project_variable_populated) } render_views @@ -14,6 +17,9 @@ describe ProjectsController, '(JavaScript fixtures)', type: :controller do end before do + # EE-specific start + # EE specific end + project.add_master(admin) sign_in(admin) end @@ -21,12 +27,43 @@ describe ProjectsController, '(JavaScript fixtures)', type: :controller do remove_repository(project) end - it 'projects/dashboard.html.raw' do |example| - get :show, - namespace_id: project.namespace.to_param, - id: project + describe ProjectsController, '(JavaScript fixtures)', type: :controller do + it 'projects/dashboard.html.raw' do |example| + get :show, + namespace_id: project.namespace.to_param, + id: project - expect(response).to be_success - store_frontend_fixture(response, example.description) + expect(response).to be_success + store_frontend_fixture(response, example.description) + end + + it 'projects/edit.html.raw' do |example| + get :edit, + namespace_id: project.namespace.to_param, + id: project + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end + end + + describe Projects::Settings::CiCdController, '(JavaScript fixtures)', type: :controller do + it 'projects/ci_cd_settings.html.raw' do |example| + get :show, + namespace_id: project.namespace.to_param, + project_id: project + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end + + it 'projects/ci_cd_settings_with_variables.html.raw' do |example| + get :show, + namespace_id: project_variable_populated.namespace.to_param, + project_id: project_variable_populated + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end end end diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..ea4e8386eff58aff0540ed75f26ffdbaed1b69c7 --- /dev/null +++ b/spec/support/features/variable_list_shared_examples.rb @@ -0,0 +1,269 @@ +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) + end + end + + it 'adds new secret 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') + 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(1)') 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') + end + end + + it 'adds empty 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('') + 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(1)') do + expect(find('.js-ci-variable-input-key').value).to eq('key') + expect(find('.js-ci-variable-input-value', visible: false).value).to eq('') + end + end + + it 'adds new unprotected 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') + find('.ci-variable-protected-item .js-project-feature-toggle').click + + expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false') + 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(1)') 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('false') + end + 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('*' * 20) + + 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('*' * 20) + + click_button('Hide value') + + 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('*' * 20) + 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) + end + end + + it 'edits variable' do + page.within('.js-ci-variable-list-section') do + click_button('Reveal value') + + page.within('.js-row:nth-child(1)') do + find('.js-ci-variable-input-key').set('new_key') + find('.js-ci-variable-input-value').set('new_value') + end + + click_button('Save variables') + wait_for_requests + + visit page_path + + page.within('.js-row:nth-child(1)') 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') + end + end + end + + it 'edits variable with empty value' do + page.within('.js-ci-variable-list-section') do + click_button('Reveal value') + + page.within('.js-row:nth-child(1)') do + find('.js-ci-variable-input-key').set('new_key') + find('.js-ci-variable-input-value').set('') + end + + click_button('Save variables') + wait_for_requests + + visit page_path + + page.within('.js-row:nth-child(1)') 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('') + end + end + 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 + + expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false') + 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 + find('.ci-variable-protected-item .js-project-feature-toggle').click + + expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true') + 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('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') + 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') + + expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('true') + end + + click_button('Save variables') + wait_for_requests + + visit page_path + + page.within('.js-ci-variable-list-section .js-row:nth-child(1)') do + find('.ci-variable-protected-item .js-project-feature-toggle').click + + expect(find('.js-ci-variable-input-protected', visible: false).value).to eq('false') + end + + click_button('Save variables') + wait_for_requests + + visit page_path + + page.within('.js-ci-variable-list-section .js-row:nth-child(1)') 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 + 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') + end + + click_button('Save variables') + wait_for_requests + + expect(page).to have_selector('.js-row', count: 4) + + # Remove the `akey` variable + page.within('.js-row:nth-child(2)') do + first('.js-row-remove-button').click + 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') + end + + click_button('Save variables') + wait_for_requests + + 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('value1') + 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('value2') + end + + click_button('Save variables') + wait_for_requests + + # 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('Variables key has already been taken') + end + end +end