diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue new file mode 100644 index 00000000000..3e58fc40755 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue @@ -0,0 +1,168 @@ + + + diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_settings.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_settings.vue new file mode 100644 index 00000000000..ed1240c247f --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_settings.vue @@ -0,0 +1,32 @@ + + + diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue new file mode 100644 index 00000000000..e240323d2c5 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_table.vue @@ -0,0 +1,130 @@ + + + diff --git a/app/assets/javascripts/ci_variable_list/constants.js b/app/assets/javascripts/ci_variable_list/constants.js new file mode 100644 index 00000000000..bfc9cbbd840 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/constants.js @@ -0,0 +1,2 @@ +// eslint-disable-next-line import/prefer-default-export +export const ADD_CI_VARIABLE_MODAL_ID = 'add-ci-variable'; diff --git a/app/assets/javascripts/ci_variable_list/index.js b/app/assets/javascripts/ci_variable_list/index.js new file mode 100644 index 00000000000..58501b216c1 --- /dev/null +++ b/app/assets/javascripts/ci_variable_list/index.js @@ -0,0 +1,25 @@ +import Vue from 'vue'; +import CiVariableSettings from './components/ci_variable_settings.vue'; +import createStore from './store'; +import { parseBoolean } from '~/lib/utils/common_utils'; + +export default () => { + const el = document.getElementById('js-ci-project-variables'); + const { endpoint, projectId, group, maskableRegex } = el.dataset; + const isGroup = parseBoolean(group); + + const store = createStore({ + endpoint, + projectId, + isGroup, + maskableRegex, + }); + + return new Vue({ + el, + store, + render(createElement) { + return createElement(CiVariableSettings); + }, + }); +}; diff --git a/app/assets/javascripts/ci_variable_list/store/utils.js b/app/assets/javascripts/ci_variable_list/store/utils.js index 44807e03dad..0b9932d9bb5 100644 --- a/app/assets/javascripts/ci_variable_list/store/utils.js +++ b/app/assets/javascripts/ci_variable_list/store/utils.js @@ -1,4 +1,5 @@ import { __ } from '~/locale'; +import { cloneDeep } from 'lodash'; const variableType = 'env_var'; const fileType = 'file'; @@ -24,9 +25,9 @@ export const prepareDataForDisplay = variables => { }; export const prepareDataForApi = (variable, destroy = false) => { - const variableCopy = variable; - variableCopy.protected.toString(); - variableCopy.masked.toString(); + const variableCopy = cloneDeep(variable); + variableCopy.protected = variableCopy.protected.toString(); + variableCopy.masked = variableCopy.masked.toString(); variableCopy.variable_type = variableTypeHandler(variableCopy.variable_type); if (variableCopy.environment_scope === __('All environments')) { 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 8a5300c9266..479c82265f2 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,17 +1,22 @@ import initSettingsPanels from '~/settings_panels'; import AjaxVariableList from '~/ci_variable_list/ajax_variable_list'; +import initVariableList from '~/ci_variable_list'; document.addEventListener('DOMContentLoaded', () => { // Initialize expandable settings panels initSettingsPanels(); - 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, - }); + 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, + }); + } }); 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 b4aac8eea2b..e08d0407245 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 @@ -2,6 +2,7 @@ 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'; document.addEventListener('DOMContentLoaded', () => { // Initialize expandable settings panels @@ -15,15 +16,19 @@ document.addEventListener('DOMContentLoaded', () => { runnerTokenSecretValue.init(); } - 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, - }); + 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'); diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss index 416537ef763..7294816aa26 100644 --- a/app/assets/stylesheets/pages/settings.scss +++ b/app/assets/stylesheets/pages/settings.scss @@ -130,6 +130,10 @@ border-radius: $border-radius-base; } +.empty-variables { + padding: 20px 0; +} + .warning-title { color: $orange-500; } @@ -370,3 +374,10 @@ .push-pull-table { margin-top: 1em; } + +.ci-variable-table { + table tr th { + background-color: transparent; + border: 0; + } +} diff --git a/doc/development/fe_guide/principles.md b/doc/development/fe_guide/principles.md index 6fb3456222f..2bef48fddcf 100644 --- a/doc/development/fe_guide/principles.md +++ b/doc/development/fe_guide/principles.md @@ -8,8 +8,8 @@ Discuss your architecture design in an issue before writing code. This helps dec ## Be consistent -There are multiple ways of writing code to accomplish the same results. We should be as consistent as possible in how we write code across our codebases. This will make it more easier us to maintain our code across GitLab. +There are multiple ways of writing code to accomplish the same results. We should be as consistent as possible in how we write code across our codebases. This will make it easier for us to maintain our code across GitLab. ## Improve code [iteratively](https://about.gitlab.com/handbook/values/#iteration) -Whenever you see with existing code that does not follow our current style guide, update it proactively. You don't need to fix everything, but each merge request should iteratively improve our codebase, and reduce technical debt where possible. +Whenever you see existing code that does not follow our current style guide, update it proactively. You don’t need to fix everything, but each merge request should iteratively improve our codebase, and reduce technical debt where possible. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 8fd4fc8ddd0..d0cb12da37c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1057,6 +1057,9 @@ msgstr "" msgid "Add README" msgstr "" +msgid "Add Variable" +msgstr "" + msgid "Add Zoom meeting" msgstr "" @@ -1189,6 +1192,9 @@ msgstr "" msgid "Add users to group" msgstr "" +msgid "Add variable" +msgstr "" + msgid "Add webhook" msgstr "" @@ -1638,6 +1644,9 @@ msgstr "" msgid "Allow users to request access (if visibility is public or internal)" msgstr "" +msgid "Allow variables to run on protected branches and tags." +msgstr "" + msgid "Allowed email domain restriction only permitted for top-level groups" msgstr "" @@ -3702,6 +3711,9 @@ msgstr "" msgid "CiVariables|Cannot use Masked Variable with current value" msgstr "" +msgid "CiVariables|Environment Scope" +msgstr "" + msgid "CiVariables|Input variable key" msgstr "" @@ -3714,6 +3726,9 @@ msgstr "" msgid "CiVariables|Masked" msgstr "" +msgid "CiVariables|Protected" +msgstr "" + msgid "CiVariables|Remove variable row" msgstr "" @@ -11822,6 +11837,9 @@ msgstr "" msgid "Marks this issue as related to %{issue_ref}." msgstr "" +msgid "Mask variable" +msgstr "" + msgid "Match not found; try refining your search query." msgstr "" @@ -15491,6 +15509,9 @@ msgstr "" msgid "Prompt users to upload SSH keys" msgstr "" +msgid "Protect variable" +msgstr "" + msgid "Protected" msgstr "" @@ -19372,6 +19393,9 @@ msgstr "" msgid "The value lying at the midpoint of a series of observed values. E.g., between 3, 5, 9, the median is 5. Between 3, 5, 7, 8, the median is (5+7)/2 = 6." msgstr "" +msgid "There are currently no variables, add a variable with the Add Variable button below." +msgstr "" + msgid "There are no GPG keys associated with this account." msgstr "" @@ -20831,6 +20855,9 @@ msgstr "" msgid "Update" msgstr "" +msgid "Update Variable" +msgstr "" + msgid "Update all" msgstr "" @@ -21443,6 +21470,9 @@ msgstr "" msgid "Variables" msgstr "" +msgid "Variables will be masked in job logs. Requires values to meet regular expression requirements." +msgstr "" + msgid "Various container registry settings." msgstr "" diff --git a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js new file mode 100644 index 00000000000..be2c017cc7e --- /dev/null +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -0,0 +1,93 @@ +import Vuex from 'vuex'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { GlModal } from '@gitlab/ui'; +import CiVariableModal from '~/ci_variable_list/components/ci_variable_modal.vue'; +import createStore from '~/ci_variable_list/store'; +import mockData from '../services/mock_data'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('Ci variable modal', () => { + let wrapper; + let store; + + const createComponent = () => { + store = createStore(); + wrapper = shallowMount(CiVariableModal, { + localVue, + store, + }); + }; + + const findModal = () => wrapper.find(GlModal); + + beforeEach(() => { + createComponent(); + jest.spyOn(store, 'dispatch').mockImplementation(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('button is disabled when no key/value pair are present', () => { + expect(findModal().props('actionPrimary').attributes.disabled).toBeTruthy(); + }); + + it('masked checkbox is disabled when value does not meet regex requirements', () => { + expect(wrapper.find({ ref: 'masked-ci-variable' }).attributes('disabled')).toBeTruthy(); + }); + + describe('Adding a new variable', () => { + beforeEach(() => { + const [variable] = mockData.mockVariables; + store.state.variable = variable; + }); + + it('button is enabled when key/value pair are present', () => { + expect(findModal().props('actionPrimary').attributes.disabled).toBeFalsy(); + }); + + it('masked checkbox is enabled when value meets regex requirements', () => { + store.state.maskableRegex = '^[a-zA-Z0-9_+=/@:-]{8,}$'; + return wrapper.vm.$nextTick(() => { + expect(wrapper.find({ ref: 'masked-ci-variable' }).attributes('disabled')).toBeFalsy(); + }); + }); + + it('Add variable button dispatches addVariable action', () => { + findModal().vm.$emit('ok'); + expect(store.dispatch).toHaveBeenCalledWith('addVariable'); + }); + + it('Clears the modal state once modal is hidden', () => { + findModal().vm.$emit('hidden'); + expect(store.dispatch).toHaveBeenCalledWith('clearModal'); + }); + }); + + describe('Editing a variable', () => { + beforeEach(() => { + const [variable] = mockData.mockVariables; + store.state.variableBeingEdited = variable; + }); + + it('button text is Update variable when updating', () => { + expect(wrapper.vm.modalActionText).toBe('Update Variable'); + }); + + it('Update variable button dispatches updateVariable with correct variable', () => { + findModal().vm.$emit('ok'); + expect(store.dispatch).toHaveBeenCalledWith( + 'updateVariable', + store.state.variableBeingEdited, + ); + }); + + it('Resets the editing state once modal is hidden', () => { + findModal().vm.$emit('hidden'); + expect(store.dispatch).toHaveBeenCalledWith('resetEditing'); + }); + }); +}); diff --git a/spec/frontend/ci_variable_list/components/ci_variable_settings_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_settings_spec.js new file mode 100644 index 00000000000..7dcd82eac5e --- /dev/null +++ b/spec/frontend/ci_variable_list/components/ci_variable_settings_spec.js @@ -0,0 +1,39 @@ +import Vuex from 'vuex'; +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import CiVariableSettings from '~/ci_variable_list/components/ci_variable_settings.vue'; +import createStore from '~/ci_variable_list/store'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('Ci variable table', () => { + let wrapper; + let store; + let isGroup; + + const createComponent = groupState => { + store = createStore(); + store.state.isGroup = groupState; + jest.spyOn(store, 'dispatch').mockImplementation(); + wrapper = shallowMount(CiVariableSettings, { + localVue, + store, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + it('dispatches fetchEnvironments when mounted', () => { + isGroup = false; + createComponent(isGroup); + expect(store.dispatch).toHaveBeenCalledWith('fetchEnvironments'); + }); + + it('does not dispatch fetchenvironments when in group context', () => { + isGroup = true; + createComponent(isGroup); + expect(store.dispatch).not.toHaveBeenCalled(); + }); +}); diff --git a/spec/frontend/ci_variable_list/components/ci_variable_table_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_table_spec.js new file mode 100644 index 00000000000..973a9c51f16 --- /dev/null +++ b/spec/frontend/ci_variable_list/components/ci_variable_table_spec.js @@ -0,0 +1,89 @@ +import Vuex from 'vuex'; +import { createLocalVue, mount } from '@vue/test-utils'; +import { GlTable } from '@gitlab/ui'; +import CiVariableTable from '~/ci_variable_list/components/ci_variable_table.vue'; +import createStore from '~/ci_variable_list/store'; +import mockData from '../services/mock_data'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('Ci variable table', () => { + let wrapper; + let store; + + const createComponent = () => { + store = createStore(); + store.state.isGroup = true; + jest.spyOn(store, 'dispatch').mockImplementation(); + wrapper = mount(CiVariableTable, { + localVue, + store, + }); + }; + + const findDeleteButton = () => wrapper.find({ ref: 'delete-ci-variable' }); + const findRevealButton = () => wrapper.find({ ref: 'secret-value-reveal-button' }); + const findEditButton = () => wrapper.find({ ref: 'edit-ci-variable' }); + const findEmptyVariablesPlaceholder = () => wrapper.find({ ref: 'empty-variables' }); + const findTable = () => wrapper.find(GlTable); + + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('dispatches fetchVariables when mounted', () => { + expect(store.dispatch).toHaveBeenCalledWith('fetchVariables'); + }); + + it('fields prop does not contain environment_scope if group', () => { + expect(findTable().props('fields')).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ + key: 'environment_scope', + label: 'Environment Scope', + }), + ]), + ); + }); + + describe('Renders correct data', () => { + it('displays empty message when variables are not present', () => { + expect(findEmptyVariablesPlaceholder().exists()).toBe(true); + }); + + it('displays correct amount of variables present and no empty message', () => { + store.state.variables = mockData.mockVariables; + + return wrapper.vm.$nextTick(() => { + expect(wrapper.findAll('.js-ci-variable-row').length).toBe(1); + expect(findEmptyVariablesPlaceholder().exists()).toBe(false); + }); + }); + }); + + describe('Table click actions', () => { + beforeEach(() => { + store.state.variables = mockData.mockVariables; + }); + + it('dispatches deleteVariable with correct variable to delete', () => { + findDeleteButton().trigger('click'); + expect(store.dispatch).toHaveBeenCalledWith('deleteVariable', mockData.mockVariables[0]); + }); + + it('reveals secret values when button is clicked', () => { + findRevealButton().trigger('click'); + expect(store.dispatch).toHaveBeenCalledWith('toggleValues', false); + }); + + it('dispatches editVariable with correct variable to edit', () => { + findEditButton().trigger('click'); + expect(store.dispatch).toHaveBeenCalledWith('editVariable', mockData.mockVariables[0]); + }); + }); +}); diff --git a/spec/frontend/ci_variable_list/services/mock_data.js b/spec/frontend/ci_variable_list/services/mock_data.js index 89473b57af9..b04cd223d42 100644 --- a/spec/frontend/ci_variable_list/services/mock_data.js +++ b/spec/frontend/ci_variable_list/services/mock_data.js @@ -9,24 +9,6 @@ export default { value: 'test_val', variable_type: 'Variable', }, - { - environment_scope: 'All environments', - id: 114, - key: 'test_var_2', - masked: false, - protected: false, - value: 'test_val_2', - variable_type: 'Variable', - }, - { - environment_scope: 'All environments', - id: 115, - key: 'test_var_3', - masked: false, - protected: false, - value: 'test_val_3', - variable_type: 'Variable', - }, ], mockVariablesApi: [ diff --git a/spec/frontend/ci_variable_list/store/utils_spec.js b/spec/frontend/ci_variable_list/store/utils_spec.js index 9d5dd6b4f29..070bc996d75 100644 --- a/spec/frontend/ci_variable_list/store/utils_spec.js +++ b/spec/frontend/ci_variable_list/store/utils_spec.js @@ -17,8 +17,8 @@ describe('CI variables store utils', () => { environment_scope: '*', id: 113, key: 'test_var', - masked: false, - protected: false, + masked: 'false', + protected: 'false', value: 'test_val', variable_type: 'env_var', }); @@ -27,8 +27,8 @@ describe('CI variables store utils', () => { environment_scope: '*', id: 114, key: 'test_var_2', - masked: false, - protected: false, + masked: 'false', + protected: 'false', value: 'test_val_2', variable_type: 'file', }); diff --git a/spec/frontend/self_monitor/components/__snapshots__/self_monitor_spec.js.snap b/spec/frontend/self_monitor/components/__snapshots__/self_monitor_form_spec.js.snap similarity index 100% rename from spec/frontend/self_monitor/components/__snapshots__/self_monitor_spec.js.snap rename to spec/frontend/self_monitor/components/__snapshots__/self_monitor_form_spec.js.snap diff --git a/spec/frontend/self_monitor/components/self_monitor_spec.js b/spec/frontend/self_monitor/components/self_monitor_form_spec.js similarity index 100% rename from spec/frontend/self_monitor/components/self_monitor_spec.js rename to spec/frontend/self_monitor/components/self_monitor_form_spec.js