From 0c1ba9730f05c36d6a0107986874b6fc9a418b48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Thu, 4 Aug 2022 10:34:47 -0400 Subject: [PATCH 1/4] Add new components for graphQL CI variables Ci envs dropdown and ci modal are getting tweaked to accomodate the new graphql structure. These components now are agnostic to any global state manager and instead expect props, which allow us to pass this data down from topmost components that are connected to graphQL. --- .../components/ci_environments_dropdown.vue | 47 ++- .../components/ci_variable_modal.vue | 203 +++++----- .../components/legacy_ci_variable_modal.vue | 6 +- .../javascripts/ci_variable_list/constants.js | 52 ++- .../javascripts/ci_variable_list/utils.js | 45 +++ .../ci_environments_dropdown_spec.js | 139 +++++++ .../components/ci_variable_modal_spec.js | 368 ++++++++++++++++++ spec/frontend/ci_variable_list/mocks.js | 116 ++++++ spec/frontend/ci_variable_list/utils_spec.js | 68 ++++ 9 files changed, 929 insertions(+), 115 deletions(-) create mode 100644 app/assets/javascripts/ci_variable_list/utils.js create mode 100644 spec/frontend/ci_variable_list/components/ci_environments_dropdown_spec.js create mode 100644 spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js create mode 100644 spec/frontend/ci_variable_list/mocks.js create mode 100644 spec/frontend/ci_variable_list/utils_spec.js diff --git a/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue b/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue index ecb39f214ec56f..69feceb63bac49 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue @@ -1,7 +1,7 @@ - + {{ __('Protect variable') }} @@ -322,7 +341,7 @@ export default { {{ __('Mask variable') }} @@ -403,7 +422,7 @@ export default { - { + const scopesFromVariables = variables.map((variable) => variable.environmentScope); + return uniq(environments.concat(scopesFromVariables)).sort(); +}; + +/** + * This function job is to convert the * wildcard to text when applicable + * in the UI. It uses a constants to compare the incoming value to that + * of the * and then apply the corresponding label if applicable. If there + * is no scope, then we return the default value as well. + * @param {String} scope + * @returns {String} - Converted value if applicable + */ + +export const convertEnvironmentScope = (environmentScope = '') => { + if (environmentScope === allEnvironments.type || !environmentScope) { + return allEnvironments.text; + } + + return environmentScope; +}; + +/** + * Gives us an array of all the environments by name + * @param {Array} nodes + * @return {Array} - Array of environments strings + */ +export const mapEnvironmentNames = (nodes = []) => { + return nodes.map((env) => env.name); +}; diff --git a/spec/frontend/ci_variable_list/components/ci_environments_dropdown_spec.js b/spec/frontend/ci_variable_list/components/ci_environments_dropdown_spec.js new file mode 100644 index 00000000000000..e9966576cabd69 --- /dev/null +++ b/spec/frontend/ci_variable_list/components/ci_environments_dropdown_spec.js @@ -0,0 +1,139 @@ +import { GlDropdown, GlDropdownItem, GlIcon, GlSearchBoxByType } from '@gitlab/ui'; +import { mount } from '@vue/test-utils'; +import { nextTick } from 'vue'; +import { allEnvironments } from '~/ci_variable_list/constants'; +import CiEnvironmentsDropdown from '~/ci_variable_list/components/ci_environments_dropdown.vue'; + +describe('Ci environments dropdown', () => { + let wrapper; + + const envs = ['dev', 'prod', 'staging']; + const defaultProps = { environments: envs, selectedEnvironmentScope: '' }; + + const findDropdownText = () => wrapper.findComponent(GlDropdown).text(); + const findAllDropdownItems = () => wrapper.findAllComponents(GlDropdownItem); + const findDropdownItemByIndex = (index) => wrapper.findAllComponents(GlDropdownItem).at(index); + const findActiveIconByIndex = (index) => findDropdownItemByIndex(index).findComponent(GlIcon); + const findSearchBox = () => wrapper.findComponent(GlSearchBoxByType); + + const createComponent = ({ props = {}, searchTerm = '' } = {}) => { + wrapper = mount(CiEnvironmentsDropdown, { + propsData: { + ...defaultProps, + ...props, + }, + }); + + findSearchBox().vm.$emit('input', searchTerm); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('No environments found', () => { + beforeEach(() => { + createComponent({ searchTerm: 'stable' }); + }); + + it('renders create button with search term if environments do not contain search term', () => { + expect(findAllDropdownItems()).toHaveLength(2); + expect(findDropdownItemByIndex(1).text()).toBe('Create wildcard: stable'); + }); + + it('renders empty results message', () => { + expect(findDropdownItemByIndex(0).text()).toBe('No matching results'); + }); + }); + + describe('Search term is empty', () => { + beforeEach(() => { + createComponent({ props: { environments: envs } }); + }); + + it('renders all environments when search term is empty', () => { + expect(findAllDropdownItems()).toHaveLength(3); + expect(findDropdownItemByIndex(0).text()).toBe(envs[0]); + expect(findDropdownItemByIndex(1).text()).toBe(envs[1]); + expect(findDropdownItemByIndex(2).text()).toBe(envs[2]); + }); + + it('should not display active checkmark on the inactive stage', () => { + expect(findActiveIconByIndex(0).classes('gl-visibility-hidden')).toBe(true); + }); + }); + + describe('when `*` is the value of selectedEnvironmentScope props', () => { + const wildcardScope = '*'; + + beforeEach(() => { + createComponent({ props: { selectedEnvironmentScope: wildcardScope } }); + }); + + it('shows the `All environments` text and not the wildcard', () => { + expect(findDropdownText()).toContain(allEnvironments.text); + expect(findDropdownText()).not.toContain(wildcardScope); + }); + }); + + describe('Environments found', () => { + const currentEnv = envs[2]; + + beforeEach(async () => { + createComponent({ searchTerm: currentEnv }); + await nextTick(); + }); + + it('renders only the environment searched for', () => { + expect(findAllDropdownItems()).toHaveLength(1); + expect(findDropdownItemByIndex(0).text()).toBe(currentEnv); + }); + + it('should not display create button', () => { + const environments = findAllDropdownItems().filter((env) => env.text().startsWith('Create')); + expect(environments).toHaveLength(0); + expect(findAllDropdownItems()).toHaveLength(1); + }); + + it('should not display empty results message', () => { + expect(wrapper.findComponent({ ref: 'noMatchingResults' }).exists()).toBe(false); + }); + + it('should clear the search term when showing the dropdown', () => { + wrapper.findComponent(GlDropdown).trigger('click'); + + expect(findSearchBox().text()).toBe(''); + }); + + describe('Custom events', () => { + describe('when clicking on an environment', () => { + const itemIndex = 0; + + beforeEach(() => { + createComponent(); + }); + + it('should emit `select-environment` if an environment is clicked', async () => { + await nextTick(); + + await findDropdownItemByIndex(itemIndex).vm.$emit('click'); + + expect(wrapper.emitted('select-environment')).toEqual([[envs[itemIndex]]]); + }); + }); + + describe('when creating a new environment from a search term', () => { + const search = 'new-env'; + beforeEach(() => { + createComponent({ searchTerm: search }); + }); + + it('should emit createClicked if an environment is clicked', async () => { + await nextTick(); + findDropdownItemByIndex(1).vm.$emit('click'); + expect(wrapper.emitted('create-environment-scope')).toEqual([[search]]); + }); + }); + }); + }); +}); 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 00000000000000..4e29d6388b4af9 --- /dev/null +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -0,0 +1,368 @@ +import { GlButton, GlFormInput } from '@gitlab/ui'; +import { mockTracking } from 'helpers/tracking_helper'; +import { shallowMountExtended, mountExtended } from 'helpers/vue_test_utils_helper'; +import CiEnvironmentsDropdown from '~/ci_variable_list/components/ci_environments_dropdown.vue'; +import CiVariableModal from '~/ci_variable_list/components/ci_variable_modal.vue'; +import { + ADD_VARIABLE_ACTION, + AWS_ACCESS_KEY_ID, + EDIT_VARIABLE_ACTION, + EVENT_LABEL, + EVENT_ACTION, + ENVIRONMENT_SCOPE_LINK_TITLE, +} from '~/ci_variable_list/constants'; +import { mockVariablesWithScopes } from '../mocks'; +import ModalStub from '../stubs'; + +describe('Ci variable modal', () => { + let wrapper; + let trackingSpy; + + const maskableRegex = '^[a-zA-Z0-9_+=/@:.~-]{8,}$'; + + const defaultProvide = { + awsLogoSvgPath: '/logo', + awsTipCommandsLink: '/tips', + awsTipDeployLink: '/deploy', + awsTipLearnLink: '/learn-link', + containsVariableReferenceLink: '/reference', + environmentScopeLink: '/help/environments', + isProtectedByDefault: false, + maskedEnvironmentVariablesLink: '/variables-link', + maskableRegex, + protectedEnvironmentVariablesLink: '/protected-link', + }; + + const defaultProps = { + areScopedVariablesAvailable: true, + environments: [], + mode: ADD_VARIABLE_ACTION, + selectedVariable: {}, + }; + + const createComponent = ({ mountFn = shallowMountExtended, props = {}, provide = {} } = {}) => { + wrapper = mountFn(CiVariableModal, { + attachTo: document.body, + provide: { ...defaultProvide, ...provide }, + propsData: { + ...defaultProps, + ...props, + }, + stubs: { + GlModal: ModalStub, + }, + }); + }; + + const findCiEnvironmentsDropdown = () => wrapper.find(CiEnvironmentsDropdown); + const findReferenceWarning = () => wrapper.findByTestId('contains-variable-reference'); + const findModal = () => wrapper.find(ModalStub); + const findAWSTip = () => wrapper.findByTestId('aws-guidance-tip'); + const findAddorUpdateButton = () => findModal().find('[data-testid="ciUpdateOrAddVariableBtn"]'); + const deleteVariableButton = () => + findModal() + .findAll(GlButton) + .wrappers.find((button) => button.props('variant') === 'danger'); + const findProtectedVariableCheckbox = () => + wrapper.findByTestId('ci-variable-protected-checkbox'); + const findMaskedVariableCheckbox = () => wrapper.findByTestId('ci-variable-masked-checkbox'); + const findValueField = () => wrapper.find('#ci-variable-value'); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('Adding a variable', () => { + describe('when no key/value pair are present', () => { + beforeEach(() => { + createComponent(); + }); + + it('shows the submit button as disabled ', () => { + expect(findAddorUpdateButton().attributes('disabled')).toBeTruthy(); + }); + }); + + describe('when a key/value pair is present', () => { + beforeEach(() => { + createComponent({ props: { selectedVariable: mockVariablesWithScopes[0] } }); + }); + + it('shows the submit button as enabled ', () => { + expect(findAddorUpdateButton().attributes('disabled')).toBeFalsy(); + }); + }); + + describe('events', () => { + const [currentVariable] = mockVariablesWithScopes; + + beforeEach(() => { + createComponent({ props: { selectedVariable: currentVariable } }); + jest.spyOn(wrapper.vm, '$emit'); + }); + + it('Dispatches `add-variable` action on submit', () => { + findAddorUpdateButton().vm.$emit('click'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('add-variable', currentVariable); + }); + + it('Dispatches the `hideModal` event when dismissing', () => { + findModal().vm.$emit('hidden'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('hideModal'); + }); + }); + }); + + describe('when protected by default', () => { + describe('when adding a new variable', () => { + beforeEach(() => { + createComponent({ provide: { isProtectedByDefault: true } }); + findModal().vm.$emit('shown'); + }); + + it('updates the protected value to true', () => { + expect( + findProtectedVariableCheckbox().attributes('data-is-protected-checked'), + ).toBeTruthy(); + }); + }); + + describe('when editing a variable', () => { + beforeEach(() => { + createComponent({ + provide: { isProtectedByDefault: false }, + props: { + selectedVariable: {}, + mode: EDIT_VARIABLE_ACTION, + }, + }); + findModal().vm.$emit('shown'); + }); + + it('keeps the value as false', async () => { + expect( + findProtectedVariableCheckbox().attributes('data-is-protected-checked'), + ).toBeUndefined(); + }); + }); + }); + + describe('Adding a new non-AWS variable', () => { + beforeEach(() => { + const [variable] = mockVariablesWithScopes; + createComponent({ mountFn: mountExtended, props: { selectedVariable: variable } }); + }); + + it('does not show AWS guidance tip', () => { + const tip = findAWSTip(); + expect(tip.exists()).toBe(true); + expect(tip.isVisible()).toBe(false); + }); + }); + + describe('Adding a new AWS variable', () => { + beforeEach(() => { + const [variable] = mockVariablesWithScopes; + const AWSKeyVariable = { + ...variable, + key: AWS_ACCESS_KEY_ID, + value: 'AKIAIOSFODNN7EXAMPLEjdhy', + }; + createComponent({ mountFn: mountExtended, props: { selectedVariable: AWSKeyVariable } }); + }); + + it('shows AWS guidance tip', () => { + const tip = findAWSTip(); + expect(tip.exists()).toBe(true); + expect(tip.isVisible()).toBe(true); + }); + }); + + describe.each` + value | secret | isShown + ${'value'} | ${'secret_value'} | ${false} + ${'dollar$ign'} | ${'dollar$ign'} | ${true} + `('Adding a new variable', ({ value, secret, isShown }) => { + beforeEach(() => { + const [variable] = mockVariablesWithScopes; + const invalidKeyVariable = { + ...variable, + key: 'key', + value, + secret_value: secret, + }; + createComponent({ mountFn: mountExtended, props: { selectedVariable: invalidKeyVariable } }); + }); + + it(`${isShown ? 'renders' : 'does not render'} the variable reference warning`, () => { + expect(findReferenceWarning().exists()).toBe(isShown); + }); + }); + + describe('Editing a variable', () => { + const [variable] = mockVariablesWithScopes; + + beforeEach(() => { + createComponent({ props: { selectedVariable: variable, mode: EDIT_VARIABLE_ACTION } }); + jest.spyOn(wrapper.vm, '$emit'); + }); + + it('button text is Update variable when updating', () => { + expect(findAddorUpdateButton().text()).toBe('Update variable'); + }); + + it('Update variable button dispatches updateVariable with correct variable', () => { + findAddorUpdateButton().vm.$emit('click'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('update-variable', variable); + }); + + it('Propagates the `hideModal` event', () => { + findModal().vm.$emit('hidden'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('hideModal'); + }); + + it('dispatches `delete-variable` with correct variable to delete', () => { + deleteVariableButton().vm.$emit('click'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('delete-variable', variable); + }); + }); + + describe('Environment scope', () => { + describe('when feature is available', () => { + it('renders the environment dropdown', () => { + createComponent({ + mountFn: mountExtended, + props: { + areScopedVariablesAvailable: true, + }, + }); + + expect(findCiEnvironmentsDropdown().exists()).toBe(true); + expect(findCiEnvironmentsDropdown().isVisible()).toBe(true); + }); + + it('renders a link to documentation on scopes', () => { + createComponent({ mountFn: mountExtended }); + + const link = wrapper.find('[data-testid="environment-scope-link"]'); + + expect(link.attributes('title')).toBe(ENVIRONMENT_SCOPE_LINK_TITLE); + expect(link.attributes('href')).toBe(defaultProvide.environmentScopeLink); + }); + }); + + describe('when feature is not available', () => { + it('disables the dropdown', () => { + createComponent({ + mountFn: mountExtended, + props: { + areScopedVariablesAvailable: false, + }, + }); + + const environmentScopeInput = wrapper + .find('[data-testid="environment-scope"]') + .find(GlFormInput); + expect(findCiEnvironmentsDropdown().exists()).toBe(false); + expect(environmentScopeInput.attributes('readonly')).toBe('readonly'); + }); + }); + }); + + describe('Validations', () => { + const maskError = 'This variable can not be masked.'; + + describe('when the mask state is invalid', () => { + beforeEach(async () => { + const [variable] = mockVariablesWithScopes; + const invalidMaskVariable = { + ...variable, + value: 'd:;', + masked: false, + }; + createComponent({ + mountFn: mountExtended, + props: { selectedVariable: invalidMaskVariable }, + }); + trackingSpy = mockTracking(undefined, wrapper.element, jest.spyOn); + await findMaskedVariableCheckbox().trigger('click'); + }); + + it('disables the submit button', () => { + expect(findAddorUpdateButton().attributes('disabled')).toBeTruthy(); + }); + + it('shows the correct error text', () => { + expect(findModal().text()).toContain(maskError); + }); + + it('sends the correct tracking event', () => { + expect(trackingSpy).toHaveBeenCalledWith(undefined, EVENT_ACTION, { + label: EVENT_LABEL, + property: ';', + }); + }); + }); + + describe.each` + value | masked | eventSent | trackingErrorProperty + ${'secretValue'} | ${false} | ${0} | ${null} + ${'short'} | ${true} | ${0} | ${null} + ${'dollar$ign'} | ${false} | ${1} | ${'$'} + ${'dollar$ign'} | ${true} | ${1} | ${'$'} + ${'unsupported|char'} | ${true} | ${1} | ${'|'} + ${'unsupported|char'} | ${false} | ${0} | ${null} + `('Adding a new variable', ({ value, masked, eventSent, trackingErrorProperty }) => { + beforeEach(async () => { + const [variable] = mockVariablesWithScopes; + const invalidKeyVariable = { + ...variable, + value: '', + masked: false, + }; + createComponent({ + mountFn: mountExtended, + props: { selectedVariable: invalidKeyVariable }, + }); + trackingSpy = mockTracking(undefined, wrapper.element, jest.spyOn); + await findValueField().vm.$emit('input', value); + if (masked) { + await findMaskedVariableCheckbox().trigger('click'); + } + }); + + it(`${ + eventSent > 0 ? 'sends the correct' : 'does not send the' + } variable validation tracking event with ${value}`, () => { + expect(trackingSpy).toHaveBeenCalledTimes(eventSent); + + if (eventSent > 0) { + expect(trackingSpy).toHaveBeenCalledWith(undefined, EVENT_ACTION, { + label: EVENT_LABEL, + property: trackingErrorProperty, + }); + } + }); + }); + + describe('when masked variable has acceptable value', () => { + beforeEach(() => { + const [variable] = mockVariablesWithScopes; + const validMaskandKeyVariable = { + ...variable, + key: AWS_ACCESS_KEY_ID, + value: '12345678', + masked: true, + }; + createComponent({ + mountFn: mountExtended, + props: { selectedVariable: validMaskandKeyVariable }, + }); + }); + + it('does not disable the submit button', () => { + expect(findAddorUpdateButton().attributes('disabled')).toBeFalsy(); + }); + }); + }); +}); diff --git a/spec/frontend/ci_variable_list/mocks.js b/spec/frontend/ci_variable_list/mocks.js new file mode 100644 index 00000000000000..1ba50c74152f1c --- /dev/null +++ b/spec/frontend/ci_variable_list/mocks.js @@ -0,0 +1,116 @@ +import { variableTypes } from '~/ci_variable_list/constants'; + +export const devName = 'dev'; +export const prodName = 'prod'; + +export const mockVariables = [ + { + __typename: 'CiVariable', + id: 1, + key: 'my-var', + masked: false, + protected: true, + value: 'env_val', + variableType: variableTypes.variableType, + }, + { + __typename: 'CiVariable', + id: 2, + key: 'secret', + masked: true, + protected: false, + value: 'the_secret_value', + variableType: variableTypes.fileType, + }, +]; + +export const mockVariablesWithScopes = mockVariables.map((variable) => { + return { ...variable, environmentScope: '*' }; +}); + +const createDefaultVars = ({ withScope = true } = {}) => { + let base = mockVariables; + + if (withScope) { + base = mockVariablesWithScopes; + } + + return { + __typename: 'CiVariableConnection', + nodes: base, + }; +}; + +const defaultEnvs = { + __typename: 'EnvironmentConnection', + nodes: [ + { + __typename: 'Environment', + id: 1, + name: prodName, + }, + { + __typename: 'Environment', + id: 2, + name: devName, + }, + ], +}; + +export const mockEnvs = defaultEnvs.nodes; + +export const mockProjectEnvironments = { + data: { + project: { + __typename: 'Project', + id: 1, + environments: defaultEnvs, + }, + }, +}; + +export const mockProjectVariables = { + data: { + project: { + __typename: 'Project', + id: 1, + ciVariables: createDefaultVars(), + }, + }, +}; + +export const mockGroupEnvironments = { + data: { + group: { + __typename: 'Group', + id: 1, + environments: defaultEnvs, + }, + }, +}; + +export const mockGroupVariables = { + data: { + group: { + __typename: 'Group', + id: 1, + ciVariables: createDefaultVars(), + }, + }, +}; + +export const mockAdminVariables = { + data: { + ciVariables: createDefaultVars({ withScope: false }), + }, +}; + +export const newVariable = { + id: 3, + environmentScope: 'new', + key: 'AWS_RANDOM_THING', + masked: true, + protected: false, + value: 'devops', + variableType: variableTypes.variableType, +}; diff --git a/spec/frontend/ci_variable_list/utils_spec.js b/spec/frontend/ci_variable_list/utils_spec.js new file mode 100644 index 00000000000000..1676e786515dcc --- /dev/null +++ b/spec/frontend/ci_variable_list/utils_spec.js @@ -0,0 +1,68 @@ +import { + createJoinedEnvironments, + convertEnvironmentScope, + mapEnvironmentNames, +} from '~/ci_variable_list/utils'; +import { allEnvironments } from '~/ci_variable_list/constants'; + +describe('utils', () => { + const environments = ['dev', 'prod']; + + describe('createJoinedEnvironments', () => { + it('returns only `environments` if `variables` argument is undefined', () => { + const variables = undefined; + + expect(createJoinedEnvironments(variables, environments)).toEqual(environments); + }); + + it('returns a list of environments and environment scopes taken from variables in alphabetical order', () => { + const envScope1 = 'new1'; + const envScope2 = 'new2'; + + const variables = [{ environmentScope: envScope1 }, { environmentScope: envScope2 }]; + + expect(createJoinedEnvironments(variables, environments)).toEqual([ + environments[0], + envScope1, + envScope2, + environments[1], + ]); + }); + + it('removes duplicate environments', () => { + const envScope1 = environments[0]; + const envScope2 = 'new2'; + + const variables = [{ environmentScope: envScope1 }, { environmentScope: envScope2 }]; + + expect(createJoinedEnvironments(variables, environments)).toEqual([ + environments[0], + envScope2, + environments[1], + ]); + }); + }); + + describe('convertEnvironmentScope', () => { + it('converts the * to the `All environments` text', () => { + expect(convertEnvironmentScope('*')).toBe(allEnvironments.text); + }); + + it('returns the environment as is if not the *', () => { + expect(convertEnvironmentScope('prod')).toBe('prod'); + }); + }); + + describe('mapEnvironmentNames', () => { + const envName = 'dev'; + const envName2 = 'prod'; + + const nodes = [ + { name: envName, otherProp: {} }, + { name: envName2, otherProp: {} }, + ]; + it('flatten a nodes array with only their names', () => { + expect(mapEnvironmentNames(nodes)).toEqual([envName, envName2]); + }); + }); +}); -- GitLab From bb7865e8538035fd40c233e75668a3b73daa13b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Thu, 4 Aug 2022 14:32:48 -0400 Subject: [PATCH 2/4] Update VueX to use new constants structure --- app/assets/javascripts/ci_variable_list/store/utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/ci_variable_list/store/utils.js b/app/assets/javascripts/ci_variable_list/store/utils.js index d9ca460a8e11ae..f46a671ae7be93 100644 --- a/app/assets/javascripts/ci_variable_list/store/utils.js +++ b/app/assets/javascripts/ci_variable_list/store/utils.js @@ -1,5 +1,5 @@ import { cloneDeep } from 'lodash'; -import { displayText, types } from '../constants'; +import { displayText, types, allEnvironments } from '../constants'; const variableTypeHandler = (type) => type === displayText.variableText ? types.variableType : types.fileType; @@ -15,7 +15,7 @@ export const prepareDataForDisplay = (variables) => { } variableCopy.secret_value = variableCopy.value; - if (variableCopy.environment_scope === types.allEnvironmentsType) { + if (variableCopy.environment_scope === allEnvironments.type) { variableCopy.environment_scope = displayText.allEnvironmentsText; } variableCopy.protected_variable = variableCopy.protected; @@ -31,7 +31,7 @@ export const prepareDataForApi = (variable, destroy = false) => { variableCopy.masked = variableCopy.masked.toString(); variableCopy.variable_type = variableTypeHandler(variableCopy.variable_type); if (variableCopy.environment_scope === displayText.allEnvironmentsText) { - variableCopy.environment_scope = types.allEnvironmentsType; + variableCopy.environment_scope = allEnvironments.type; } if (destroy) { -- GitLab From fac435e862156f05f90fc2902618b77f1e643fc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Fri, 5 Aug 2022 09:58:42 -0400 Subject: [PATCH 3/4] Address reviewer feedback in specs --- .../components/ci_variable_modal_spec.js | 56 +++++++++++-------- 1 file changed, 34 insertions(+), 22 deletions(-) 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 index 4e29d6388b4af9..9c0b35ae8ce1b1 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -58,7 +58,7 @@ describe('Ci variable modal', () => { const findReferenceWarning = () => wrapper.findByTestId('contains-variable-reference'); const findModal = () => wrapper.find(ModalStub); const findAWSTip = () => wrapper.findByTestId('aws-guidance-tip'); - const findAddorUpdateButton = () => findModal().find('[data-testid="ciUpdateOrAddVariableBtn"]'); + const findAddorUpdateButton = () => wrapper.findByTestId('ciUpdateOrAddVariableBtn'); const deleteVariableButton = () => findModal() .findAll(GlButton) @@ -67,6 +67,8 @@ describe('Ci variable modal', () => { wrapper.findByTestId('ci-variable-protected-checkbox'); const findMaskedVariableCheckbox = () => wrapper.findByTestId('ci-variable-masked-checkbox'); const findValueField = () => wrapper.find('#ci-variable-value'); + const findEnvScopeLink = () => wrapper.findByTestId('environment-scope-link'); + const findEnvScopeInput = () => wrapper.findByTestId('environment-scope').find(GlFormInput); afterEach(() => { wrapper.destroy(); @@ -178,24 +180,37 @@ describe('Ci variable modal', () => { }); }); - describe.each` - value | secret | isShown - ${'value'} | ${'secret_value'} | ${false} - ${'dollar$ign'} | ${'dollar$ign'} | ${true} - `('Adding a new variable', ({ value, secret, isShown }) => { - beforeEach(() => { - const [variable] = mockVariablesWithScopes; - const invalidKeyVariable = { - ...variable, - key: 'key', - value, - secret_value: secret, - }; - createComponent({ mountFn: mountExtended, props: { selectedVariable: invalidKeyVariable } }); + describe('Reference warning when adding a variable', () => { + describe('with a $ character', () => { + beforeEach(() => { + const [variable] = mockVariablesWithScopes; + const variableWithDollarSign = { + ...variable, + value: 'valueWith$', + }; + createComponent({ + mountFn: mountExtended, + props: { selectedVariable: variableWithDollarSign }, + }); + }); + + it(`renders the variable reference warning`, () => { + expect(findReferenceWarning().exists()).toBe(true); + }); }); - it(`${isShown ? 'renders' : 'does not render'} the variable reference warning`, () => { - expect(findReferenceWarning().exists()).toBe(isShown); + describe('without a $ character', () => { + beforeEach(() => { + const [variable] = mockVariablesWithScopes; + createComponent({ + mountFn: mountExtended, + props: { selectedVariable: variable }, + }); + }); + + it(`does not render the variable reference warning`, () => { + expect(findReferenceWarning().exists()).toBe(false); + }); }); }); @@ -244,7 +259,7 @@ describe('Ci variable modal', () => { it('renders a link to documentation on scopes', () => { createComponent({ mountFn: mountExtended }); - const link = wrapper.find('[data-testid="environment-scope-link"]'); + const link = findEnvScopeLink(); expect(link.attributes('title')).toBe(ENVIRONMENT_SCOPE_LINK_TITLE); expect(link.attributes('href')).toBe(defaultProvide.environmentScopeLink); @@ -260,11 +275,8 @@ describe('Ci variable modal', () => { }, }); - const environmentScopeInput = wrapper - .find('[data-testid="environment-scope"]') - .find(GlFormInput); expect(findCiEnvironmentsDropdown().exists()).toBe(false); - expect(environmentScopeInput.attributes('readonly')).toBe('readonly'); + expect(findEnvScopeInput().attributes('readonly')).toBe('readonly'); }); }); }); -- GitLab From 209066969f05c8607ae2c74430f64762ed66519b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= Date: Mon, 8 Aug 2022 11:19:10 -0400 Subject: [PATCH 4/4] Address maintainer feedback --- .../components/ci_environments_dropdown.vue | 4 ++-- .../components/ci_variable_modal_spec.js | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue b/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue index 69feceb63bac49..8ee7132bb252f6 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_environments_dropdown.vue @@ -52,8 +52,8 @@ export default { convertEnvironmentScopeValue(scope) { return convertEnvironmentScope(scope); }, - async createEnvironmentScope() { - await this.$emit('create-environment-scope', this.searchTerm); + createEnvironmentScope() { + this.$emit('create-environment-scope', this.searchTerm); this.selectEnvironment(this.searchTerm); }, isSelected(env) { 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 index 9c0b35ae8ce1b1..51b902d97dca8d 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -105,12 +105,12 @@ describe('Ci variable modal', () => { it('Dispatches `add-variable` action on submit', () => { findAddorUpdateButton().vm.$emit('click'); - expect(wrapper.vm.$emit).toHaveBeenCalledWith('add-variable', currentVariable); + expect(wrapper.emitted('add-variable')).toEqual([[currentVariable]]); }); it('Dispatches the `hideModal` event when dismissing', () => { findModal().vm.$emit('hidden'); - expect(wrapper.vm.$emit).toHaveBeenCalledWith('hideModal'); + expect(wrapper.emitted('hideModal')).toEqual([[]]); }); }); }); @@ -228,17 +228,17 @@ describe('Ci variable modal', () => { it('Update variable button dispatches updateVariable with correct variable', () => { findAddorUpdateButton().vm.$emit('click'); - expect(wrapper.vm.$emit).toHaveBeenCalledWith('update-variable', variable); + expect(wrapper.emitted('update-variable')).toEqual([[variable]]); }); it('Propagates the `hideModal` event', () => { findModal().vm.$emit('hidden'); - expect(wrapper.vm.$emit).toHaveBeenCalledWith('hideModal'); + expect(wrapper.emitted('hideModal')).toEqual([[]]); }); it('dispatches `delete-variable` with correct variable to delete', () => { deleteVariableButton().vm.$emit('click'); - expect(wrapper.vm.$emit).toHaveBeenCalledWith('delete-variable', variable); + expect(wrapper.emitted('delete-variable')).toEqual([[variable]]); }); }); -- GitLab