From 75ed475cc79bde30b98d71eeac15c0ff33b22c07 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Mon, 6 Mar 2023 13:04:28 +1300 Subject: [PATCH 01/15] Refactor compliance frameworks create/edit to modals Add feature flag manage_compliance_frameworks_modals_refactor for this refactor, and when enabled, stop linking to separate pages for create and edit of compliance frameworks. Show these forms inside a modal. Changelog: changed EE: true MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113637 --- .../components/create_form.vue | 28 +++++- .../components/edit_form.vue | 25 ++++- .../components/form_modal.vue | 91 +++++++++++++++++++ .../components/shared_form.vue | 18 +++- .../components/table.vue | 68 +++++++++++++- .../components/table_actions.vue | 15 +++ .../components/table_empty_state.vue | 27 +++++- .../compliance_frameworks/init_table.js | 13 ++- .../settings/compliance_frameworks/utils.js | 15 +++ ee/app/controllers/ee/groups_controller.rb | 1 + .../group_settings_helper.rb | 3 + ..._compliance_frameworks_modals_refactor.yml | 8 ++ .../components/create_form_spec.js | 19 +++- .../components/table_empty_state_spec.js | 11 ++- .../components/table_spec.js | 3 + .../group_settings_helper_spec.rb | 6 +- locale/gitlab.pot | 9 ++ 17 files changed, 337 insertions(+), 23 deletions(-) create mode 100644 ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue create mode 100644 ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue index 5772d45669da9e06..1eb27cd532b0d453 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue @@ -1,10 +1,13 @@ <script> import * as Sentry from '@sentry/browser'; + import { visitUrl } from '~/lib/utils/url_utility'; import { s__ } from '~/locale'; + import { SAVE_ERROR } from '../constants'; import createComplianceFrameworkMutation from '../graphql/queries/create_compliance_framework.mutation.graphql'; -import { getSubmissionParams, initialiseFormData } from '../utils'; +import { getSubmissionParams, initialiseFormData, isModalsRefactorEnabled } from '../utils'; +import getComplianceFrameworkQuery from '../graphql/queries/get_compliance_framework.query.graphql'; import FormStatus from './form_status.vue'; import SharedForm from './shared_form.vue'; @@ -46,15 +49,18 @@ export default { this.errorMessage = userFriendlyText; Sentry.captureException(error); }, + onCancel() { + this.$emit('cancel'); + }, async onSubmit() { this.saving = true; this.errorMessage = ''; - try { const params = getSubmissionParams( this.formData, this.pipelineConfigurationFullPathEnabled, ); + const { data } = await this.$apollo.mutate({ mutation: createComplianceFrameworkMutation, variables: { @@ -63,6 +69,15 @@ export default { params, }, }, + awaitRefetchQueries: true, + refetchQueries: [ + { + query: getComplianceFrameworkQuery, + variables: { + fullPath: this.groupPath, + }, + }, + ], }); const [error] = data?.createComplianceFramework?.errors || []; @@ -70,7 +85,12 @@ export default { if (error) { this.setError(new Error(error), error); } else { - visitUrl(this.groupEditPath); + if (!isModalsRefactorEnabled()) { + visitUrl(this.groupEditPath); + return; + } + + this.$emit('success', this.$options.i18n.successMessageText); } } catch (e) { this.setError(e, SAVE_ERROR); @@ -79,6 +99,7 @@ export default { }, i18n: { submitButtonText: s__('ComplianceFrameworks|Add framework'), + successMessageText: s__('ComplianceFrameworks|Compliance framework created'), }, }; </script> @@ -92,6 +113,7 @@ export default { :pipeline-configuration-full-path.sync="formData.pipelineConfigurationFullPath" :color.sync="formData.color" :submit-button-text="$options.i18n.submitButtonText" + @cancel="onCancel" @submit="onSubmit" /> </form-status> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue index 48d044d735b1bf04..e85c9014cc9c6e99 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue @@ -2,12 +2,12 @@ import * as Sentry from '@sentry/browser'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import { visitUrl } from '~/lib/utils/url_utility'; -import { __ } from '~/locale'; +import { __, s__ } from '~/locale'; import getComplianceFrameworkQuery from 'ee/graphql_shared/queries/get_compliance_framework.query.graphql'; import { FETCH_ERROR, SAVE_ERROR } from '../constants'; import updateComplianceFrameworkMutation from '../graphql/queries/update_compliance_framework.mutation.graphql'; -import { getSubmissionParams, initialiseFormData } from '../utils'; +import { getSubmissionParams, initialiseFormData, isModalsRefactorEnabled } from '../utils'; import FormStatus from './form_status.vue'; import SharedForm from './shared_form.vue'; @@ -112,6 +112,9 @@ export default { this.saveErrorMessage = userFriendlyText; Sentry.captureException(error); }, + onCancel() { + this.$emit('cancel'); + }, async onSubmit() { this.saving = true; this.saveErrorMessage = ''; @@ -129,6 +132,15 @@ export default { params, }, }, + awaitRefetchQueries: true, + refetchQueries: [ + { + query: getComplianceFrameworkQuery, + variables: { + fullPath: this.groupPath, + }, + }, + ], }); const [error] = data?.updateComplianceFramework?.errors || []; @@ -136,7 +148,12 @@ export default { if (error) { this.setSavingError(new Error(error), error); } else { - visitUrl(this.groupEditPath); + if (!isModalsRefactorEnabled()) { + visitUrl(this.groupEditPath); + return; + } + + this.$emit('success', this.$options.i18n.successMessageText); } } catch (e) { this.setSavingError(e, SAVE_ERROR); @@ -145,6 +162,7 @@ export default { }, i18n: { submitButtonText: __('Save changes'), + successMessageText: s__('ComplianceFrameworks|Changes to compliance framework saved'), }, }; </script> @@ -159,6 +177,7 @@ export default { :pipeline-configuration-full-path.sync="formData.pipelineConfigurationFullPath" :color.sync="formData.color" :submit-button-text="$options.i18n.submitButtonText" + @cancel="onCancel" @submit="onSubmit" /> </form-status> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue new file mode 100644 index 0000000000000000..d350322fa6e9c0a6 --- /dev/null +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue @@ -0,0 +1,91 @@ +<script> +import { GlModal } from '@gitlab/ui'; +import { __, s__, sprintf } from '~/locale'; + +import CreateForm from './create_form.vue'; +import EditForm from './edit_form.vue'; + +export default { + components: { + GlModal, + CreateForm, + EditForm, + }, + props: { + framework: { + type: Object, + required: false, + default: null, + }, + graphqlFieldName: { + type: String, + required: true, + }, + groupPath: { + type: String, + required: true, + }, + groupEditPath: { + type: String, + required: true, + }, + pipelineConfigurationFullPathEnabled: { + type: Boolean, + required: true, + }, + }, + computed: { + isEdit() { + return Boolean(this.framework?.id); + }, + title() { + return sprintf(this.$options.i18n.title, { + action: this.isEdit ? this.$options.i18n.edit : this.$options.i18n.new, + }); + }, + }, + methods: { + show() { + this.errorMessage = ''; + this.$refs.modal.show(); + }, + hide() { + this.$refs.modal.hide(); + }, + onSuccess(successMessage) { + this.$emit('change', successMessage); + }, + }, + i18n: { + title: s__('ComplianceFrameworks|%{action} compliance framework'), + addFramework: s__('ComplianceFrameworks|Add framework'), + cancel: __('Cancel'), + edit: __('Edit'), + new: __('New'), + }, +}; +</script> +<template> + <gl-modal ref="modal" :title="title" modal-id="framework-form-modal" hide-footer> + <edit-form + v-if="isEdit" + :id="framework.id" + ref="formComponent" + :graphql-field-name="graphqlFieldName" + :group-path="groupPath" + :group-edit-path="groupEditPath" + :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" + @cancel="hide" + @success="onSuccess" + /> + <create-form + v-else + ref="formComponent" + :group-path="groupPath" + :group-edit-path="groupEditPath" + :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" + @cancel="hide" + @success="onSuccess" + /> + </gl-modal> +</template> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue index 941098ccc018ea06..e7f6a9c46a8039e8 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue @@ -6,8 +6,13 @@ import { helpPagePath } from '~/helpers/help_page_helper'; import { validateHexColor } from '~/lib/utils/color_utils'; import { s__ } from '~/locale'; import ColorPicker from '~/vue_shared/components/color_picker/color_picker.vue'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { DEBOUNCE_DELAY } from '../constants'; -import { fetchPipelineConfigurationFileExists, validatePipelineConfirmationFormat } from '../utils'; +import { + fetchPipelineConfigurationFileExists, + isModalsRefactorEnabled, + validatePipelineConfirmationFormat, +} from '../utils'; export default { components: { @@ -19,6 +24,7 @@ export default { GlLink, GlSprintf, }, + mixins: [glFeatureFlagMixin()], props: { color: { type: String, @@ -127,6 +133,14 @@ export default { validatePipelineInput: debounce(function debounceValidation(path) { this.validatePipelineConfigurationPath(path); }, DEBOUNCE_DELAY), + onCancel(event) { + if (!isModalsRefactorEnabled()) { + return; + } + + event.preventDefault(); + this.$emit('cancel'); + }, }, i18n: { titleInputLabel: s__('ComplianceFrameworks|Name'), @@ -223,7 +237,7 @@ export default { :disabled="disableSubmitBtn" >{{ submitButtonText }}</gl-button > - <gl-button :href="groupEditPath" data-testid="cancel-btn">{{ + <gl-button :href="groupEditPath" data-testid="cancel-btn" @click="onCancel">{{ $options.i18n.cancelBtnText }}</gl-button> </div> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue index fbee0e79d4123ea3..10171369ed48037d 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue @@ -1,10 +1,20 @@ <script> -import { GlAlert, GlBadge, GlButton, GlLoadingIcon, GlTableLite, GlLabel } from '@gitlab/ui'; +import Vue from 'vue'; +import { + GlAlert, + GlBadge, + GlButton, + GlLoadingIcon, + GlTableLite, + GlLabel, + GlToast, +} from '@gitlab/ui'; import * as Sentry from '@sentry/browser'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import getComplianceFrameworkQuery from 'ee/graphql_shared/queries/get_compliance_framework.query.graphql'; import { s__ } from '~/locale'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { DANGER, INFO, EDIT_BUTTON_LABEL } from '../constants'; import updateComplianceFrameworkMutation from '../graphql/queries/update_compliance_framework.mutation.graphql'; @@ -12,11 +22,15 @@ import { injectIdIntoEditPath } from '../utils'; import DeleteModal from './delete_modal.vue'; import EmptyState from './table_empty_state.vue'; import TableActions from './table_actions.vue'; +import FormModal from './form_modal.vue'; + +Vue.use(GlToast); export default { components: { DeleteModal, EmptyState, + FormModal, GlAlert, GlBadge, GlButton, @@ -25,6 +39,7 @@ export default { GlLabel, TableActions, }, + mixins: [glFeatureFlagMixin()], props: { addFrameworkPath: { type: String, @@ -40,13 +55,26 @@ export default { type: String, required: true, }, + graphqlFieldName: { + type: String, + required: true, + }, groupPath: { type: String, required: true, }, + groupEditPath: { + type: String, + required: true, + }, + pipelineConfigurationFullPathEnabled: { + type: Boolean, + required: true, + }, }, data() { return { + markedForEdit: {}, markedForDeletion: {}, deletingFrameworksIds: [], complianceFrameworks: [], @@ -131,6 +159,14 @@ export default { }, }, methods: { + onClickAdd(event) { + if (!this.glFeatures.manageComplianceFrameworksModalsRefactor) { + return; + } + + event?.preventDefault(); + this.markForAdd(); + }, setError(error, userFriendlyText) { this.error = userFriendlyText; Sentry.captureException(error); @@ -138,13 +174,26 @@ export default { dismissAlertMessage() { this.message = null; }, + markForAdd() { + this.markedForEdit = null; + this.$refs.formModal.show(); + }, + markForEdit(framework) { + this.markedForEdit = framework; + this.$refs.formModal.show(); + }, markForDeletion(framework) { this.markedForDeletion = framework; - this.$refs.modal.show(); + this.$refs.deleteModal.show(); }, onError() { this.error = this.$options.i18n.deleteError; }, + onChange(userFriendlyText) { + this.$refs.formModal.hide(); + this.$toast.show(userFriendlyText); + this.markedForEdit = null; + }, onDelete(id) { this.message = this.$options.i18n.deleteMessage; const idx = this.deletingFrameworksIds.indexOf(id); @@ -227,6 +276,7 @@ export default { v-if="isEmpty" :image-path="emptyStateSvgPath" :add-framework-path="addFrameworkPath" + @addFramework="onClickAdd" /> <gl-table-lite v-if="hasFrameworks" :items="complianceFrameworks" :fields="tableFields"> @@ -257,6 +307,7 @@ export default { :framework="framework" :loading="isDeleting(framework.id)" @delete="markForDeletion" + @edit="markForEdit" @setDefault="setDefaultFramework" @removeDefault="setDefaultFramework" /> @@ -270,13 +321,24 @@ export default { variant="confirm" size="small" :href="addFrameworkPath" + @click="onClickAdd" > {{ $options.i18n.addBtn }} </gl-button> + + <form-modal + ref="formModal" + :framework="markedForEdit" + :graphql-field-name="graphqlFieldName" + :group-path="groupPath" + :group-edit-path="groupEditPath" + :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" + @change="onChange" + /> <delete-modal v-if="hasFrameworks" :id="markedForDeletion.id" - ref="modal" + ref="deleteModal" :name="markedForDeletion.name" :group-path="groupPath" @deleting="onDeleting" diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue index c982ba0dad9d7dd3..fab7d8d3c2c1fc14 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue @@ -1,5 +1,8 @@ <script> import { GlButton, GlDropdown, GlDropdownItem, GlTooltipDirective } from '@gitlab/ui'; + +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + import { OPTIONS_BUTTON_LABEL, DELETE_BUTTON_LABEL, @@ -17,6 +20,7 @@ export default { GlDropdown, GlDropdownItem, }, + mixins: [glFeatureFlagMixin()], props: { framework: { type: Object, @@ -39,6 +43,16 @@ export default { return Boolean(this.framework.default); }, }, + methods: { + onEdit(event) { + if (!this.glFeatures.manageComplianceFrameworksModalsRefactor) { + return; + } + + event.preventDefault(); + this.$emit('edit', this.framework); + }, + }, }; </script> <template> @@ -52,6 +66,7 @@ export default { data-testid="compliance-framework-edit-button" icon="pencil" category="tertiary" + @click="onEdit" /> <gl-dropdown v-gl-tooltip.hover.focus="$options.i18n.optionsFramework" diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue index 9479b685daaa4c45..f333de7250b8e76e 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue @@ -1,11 +1,14 @@ <script> -import { GlEmptyState } from '@gitlab/ui'; +import { GlButton, GlEmptyState } from '@gitlab/ui'; import { s__ } from '~/locale'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; export default { components: { + GlButton, GlEmptyState, }, + mixins: [glFeatureFlagMixin()], props: { imagePath: { type: String, @@ -18,6 +21,16 @@ export default { default: null, }, }, + methods: { + onAddFramework(event) { + if (!this.glFeatures.manageComplianceFrameworksModalsRefactor) { + return; + } + + event.preventDefault(); + this.$emit('addFramework', event); + }, + }, i18n: { heading: s__('ComplianceFrameworks|No compliance frameworks are set up yet'), description: s__('ComplianceFrameworks|Frameworks that have been added will appear here.'), @@ -30,13 +43,21 @@ export default { <gl-empty-state :description="$options.i18n.description" :svg-path="imagePath" - :primary-button-link="addFrameworkPath" - :primary-button-text="$options.i18n.addButton" compact :svg-height="100" > <template #title> <h5 class="gl-mt-0">{{ $options.i18n.heading }}</h5> </template> + <template #actions> + <gl-button + :href="addFrameworkPath" + category="primary" + variant="confirm" + class="gl-mb-3" + @click="onAddFramework" + >{{ $options.i18n.addButton }} + </gl-button> + </template> </gl-empty-state> </template> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js index 36db71234112ed12..987a8f94240b2f2e 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js @@ -15,7 +15,15 @@ const createComplianceFrameworksTableApp = (el) => { return false; } - const { addFrameworkPath, editFrameworkPath, emptyStateSvgPath, groupPath } = el.dataset; + const { + addFrameworkPath, + editFrameworkPath, + emptyStateSvgPath, + graphqlFieldName = null, + groupPath, + groupEditPath, + pipelineConfigurationFullPathEnabled, + } = el.dataset; return new Vue({ el, @@ -26,7 +34,10 @@ const createComplianceFrameworksTableApp = (el) => { addFrameworkPath, editFrameworkPath, emptyStateSvgPath, + graphqlFieldName, + groupEditPath, groupPath, + pipelineConfigurationFullPathEnabled: Boolean(pipelineConfigurationFullPathEnabled), }, }); }, diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js index 745cc1a7728d0026..52418a9f48be2bf8 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js @@ -52,3 +52,18 @@ export const fetchPipelineConfigurationFileExists = async (path) => { return false; } }; + +export function isModalsRefactorEnabled() { + /* + * Temporary util function to determine if we are in the new refactored modal flow, or the existing full-page form + * flow. We need to check both feature flag and the current page to handle the situation where the FF is enabled + * but someone has navigated directly to the old page by URL. + */ + if (!gon.features?.manageComplianceFrameworksModalsRefactor) { + return false; + } + + return !['groups:compliance_frameworks:edit', 'groups:compliance_frameworks:new'].includes( + document.body.dataset.page, + ); +} diff --git a/ee/app/controllers/ee/groups_controller.rb b/ee/app/controllers/ee/groups_controller.rb index ddd714d4bf0febc7..66ededc7a3a8cfac 100644 --- a/ee/app/controllers/ee/groups_controller.rb +++ b/ee/app/controllers/ee/groups_controller.rb @@ -19,6 +19,7 @@ module GroupsController before_action do push_frontend_feature_flag(:saas_user_caps_auto_approve_pending_users_on_cap_increase, @group) + push_frontend_feature_flag(:manage_compliance_frameworks_modals_refactor, @group) end before_action only: :show do diff --git a/ee/app/helpers/compliance_management/compliance_framework/group_settings_helper.rb b/ee/app/helpers/compliance_management/compliance_framework/group_settings_helper.rb index 44b8af41e35b1554..071f8bdcfe24c17c 100644 --- a/ee/app/helpers/compliance_management/compliance_framework/group_settings_helper.rb +++ b/ee/app/helpers/compliance_management/compliance_framework/group_settings_helper.rb @@ -10,9 +10,12 @@ def show_compliance_frameworks?(group) def compliance_frameworks_list_data(group) {}.tap do |data| data[:empty_state_svg_path] = image_path('illustrations/welcome/ee_trial.svg') + data[:group_edit_path] = edit_group_path(group, anchor: 'js-compliance-frameworks-settings') data[:group_path] = group.root_ancestor.full_path data[:add_framework_path] = new_group_compliance_framework_path(group) unless group.subgroup? data[:edit_framework_path] = edit_group_compliance_framework_path(group, :id) unless group.subgroup? + data[:pipeline_configuration_full_path_enabled] = pipeline_configuration_full_path_enabled?(group).to_s + data[:graphql_field_name] = ComplianceManagement::Framework.name end end diff --git a/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml b/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml new file mode 100644 index 0000000000000000..fa31309cc48707c2 --- /dev/null +++ b/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml @@ -0,0 +1,8 @@ +--- +name: manage_compliance_frameworks_modals_refactor +introduced_by_url: +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/389653 +milestone: '15.10' +type: development +group: group::compliance +default_enabled: false diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js index f455af283bd08a3c..7996b0fa0bea922f 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js @@ -8,11 +8,12 @@ import FormStatus from 'ee/groups/settings/compliance_frameworks/components/form import SharedForm from 'ee/groups/settings/compliance_frameworks/components/shared_form.vue'; import { SAVE_ERROR } from 'ee/groups/settings/compliance_frameworks/constants'; import createComplianceFrameworkMutation from 'ee/groups/settings/compliance_frameworks/graphql/queries/create_compliance_framework.mutation.graphql'; +import getComplianceFrameworkQuery from 'ee/groups/settings/compliance_frameworks/graphql/queries/get_compliance_framework.query.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { visitUrl } from '~/lib/utils/url_utility'; -import { validCreateResponse, errorCreateResponse } from '../mock_data'; +import { errorCreateResponse, validCreateResponse, validFetchResponse } from '../mock_data'; Vue.use(VueApollo); @@ -33,6 +34,7 @@ describe('CreateForm', () => { const create = jest.fn().mockResolvedValue(validCreateResponse); const createWithNetworkErrors = jest.fn().mockRejectedValue(sentryError); const createWithErrors = jest.fn().mockResolvedValue(errorCreateResponse); + const refetchFrameworks = jest.fn().mockResolvedValue(validFetchResponse); const findForm = () => wrapper.findComponent(SharedForm); const findFormStatus = () => wrapper.findComponent(FormStatus); @@ -101,7 +103,10 @@ describe('CreateForm', () => { it('passes the error to the form status when saving causes an exception and does not redirect', async () => { const captureExceptionSpy = jest.spyOn(Sentry, 'captureException'); - wrapper = createComponent([[createComplianceFrameworkMutation, createWithNetworkErrors]]); + wrapper = createComponent([ + [createComplianceFrameworkMutation, createWithNetworkErrors], + [getComplianceFrameworkQuery, refetchFrameworks], + ]); await submitForm(name, description, pipelineConfigurationFullPath, color); @@ -114,7 +119,10 @@ describe('CreateForm', () => { it('passes the errors to the form status when saving fails and does not redirect', async () => { const captureExceptionSpy = jest.spyOn(Sentry, 'captureException'); - wrapper = createComponent([[createComplianceFrameworkMutation, createWithErrors]]); + wrapper = createComponent([ + [createComplianceFrameworkMutation, createWithErrors], + [getComplianceFrameworkQuery, refetchFrameworks], + ]); await submitForm(name, description, pipelineConfigurationFullPath, color); @@ -126,7 +134,10 @@ describe('CreateForm', () => { }); it('saves inputted values, redirects and continues to show loading while redirecting', async () => { - wrapper = createComponent([[createComplianceFrameworkMutation, create]]); + wrapper = createComponent([ + [createComplianceFrameworkMutation, create], + [getComplianceFrameworkQuery, refetchFrameworks], + ]); await submitForm(name, description, pipelineConfigurationFullPath, color); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js index 23b562d6f8a9c7bd..c69cde67b05ab364 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js @@ -1,4 +1,4 @@ -import { GlEmptyState } from '@gitlab/ui'; +import { GlButton, GlEmptyState } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import TableEmptyState from 'ee/groups/settings/compliance_frameworks/components/table_empty_state.vue'; @@ -23,8 +23,6 @@ describe('TableEmptyState', () => { expect(findEmptyState().props()).toMatchObject({ description: 'Frameworks that have been added will appear here.', svgPath: 'dir/image.svg', - primaryButtonLink: 'group/framework/new', - primaryButtonText: 'Add framework', svgHeight: 100, compact: true, }); @@ -35,4 +33,11 @@ describe('TableEmptyState', () => { expect(findEmptyState().find('h5').text()).toBe('No compliance frameworks are set up yet'); }); + + it('has an add framework action', () => { + createComponent(); + const button = wrapper.findComponent(GlButton); + + expect(button.text()).toBe('Add framework'); + }); }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js index 14056c970f3638d4..558f4ae317249063 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js @@ -68,6 +68,9 @@ describe('Table', () => { editFrameworkPath: 'group/framework/id/edit', emptyStateSvgPath: 'dir/image.svg', groupPath: 'group-1', + groupEditPath: '/groups/group-1/-/edit#js-compliance-frameworks-settings', + graphqlFieldName: 'ComplianceManagement::Framework', + pipelineConfigurationFullPathEnabled: true, ...props, }, stubs: { diff --git a/ee/spec/helpers/compliance_management/compliance_framework/group_settings_helper_spec.rb b/ee/spec/helpers/compliance_management/compliance_framework/group_settings_helper_spec.rb index 0b00f95c6a5a413b..17b02fc899a0994d 100644 --- a/ee/spec/helpers/compliance_management/compliance_framework/group_settings_helper_spec.rb +++ b/ee/spec/helpers/compliance_management/compliance_framework/group_settings_helper_spec.rb @@ -37,14 +37,18 @@ before do allow(helper).to receive(:can?).with(current_user, :admin_compliance_framework, group).and_return(true) + allow(helper).to receive(:can?).with(current_user, :admin_compliance_pipeline_configuration, group).and_return(true) end it 'returns the correct data' do expect(helper.compliance_frameworks_list_data(group)).to contain_exactly( [:empty_state_svg_path, ActionController::Base.helpers.image_path('illustrations/welcome/ee_trial.svg')], + [:group_edit_path, edit_group_path(group, anchor: 'js-compliance-frameworks-settings')], [:group_path, group.full_path], [:add_framework_path, new_group_compliance_framework_path(group)], - [:edit_framework_path, edit_group_compliance_framework_path(group, :id)] + [:edit_framework_path, edit_group_compliance_framework_path(group, :id)], + [:pipeline_configuration_full_path_enabled, 'true'], + [:graphql_field_name, ComplianceManagement::Framework.name] ) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fd265e61876e08d6..cb3e89053c646e74 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10781,6 +10781,9 @@ msgstr "" msgid "Compliance violations and compliance frameworks for the group." msgstr "" +msgid "ComplianceFrameworks|%{action} compliance framework" +msgstr "" + msgid "ComplianceFrameworks|Add framework" msgstr "" @@ -10790,6 +10793,12 @@ msgstr "" msgid "ComplianceFrameworks|Cancel" msgstr "" +msgid "ComplianceFrameworks|Changes to compliance framework saved" +msgstr "" + +msgid "ComplianceFrameworks|Compliance framework created" +msgstr "" + msgid "ComplianceFrameworks|Compliance framework deleted successfully" msgstr "" -- GitLab From 81b08113166546feac2d721bf44e67a2241a2182 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Mon, 6 Mar 2023 15:41:48 +1300 Subject: [PATCH 02/15] Tests for form_modal --- .../components/form_modal_spec.js | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js new file mode 100644 index 0000000000000000..ff4e8e10121661c0 --- /dev/null +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js @@ -0,0 +1,75 @@ +import { shallowMount } from '@vue/test-utils'; + +import { GlModal } from '@gitlab/ui'; + +import FormModal from 'ee/groups/settings/compliance_frameworks/components/form_modal.vue'; +import { frameworkFoundResponse } from '../mock_data'; + +jest.mock('~/lib/utils/url_utility'); + +describe('FormModal', () => { + let wrapper; + + const propsData = { + framework: {}, + groupPath: 'group-1', + groupEditPath: 'group-1/edit', + pipelineConfigurationFullPathEnabled: true, + graphqlFieldName: 'ComplianceManagement::Framework', + }; + + const findModal = () => wrapper.findComponent(GlModal); + + function createComponent(props = {}, mountFn = shallowMount) { + return mountFn(FormModal, { + propsData: { + ...propsData, + ...props, + }, + }); + } + + describe('initialized', () => { + it('sets the modal title when adding', () => { + wrapper = createComponent(); + + expect(findModal().props('title')).toBe('New compliance framework'); + }); + + it('sets the modal title when editing', () => { + wrapper = createComponent({ + framework: frameworkFoundResponse, + }); + + expect(findModal().props('title')).toBe('Edit compliance framework'); + }); + }); + + describe('show', () => { + beforeEach(async () => { + wrapper = createComponent(); + }); + + it('shows the modal', () => { + wrapper.vm.$refs.modal.show = jest.fn(); + + wrapper.vm.show(); + + expect(wrapper.vm.$refs.modal.show).toHaveBeenCalled(); + }); + }); + + describe('hide', () => { + beforeEach(() => { + wrapper = createComponent(); + }); + + it('hides the modal', () => { + wrapper.vm.$refs.modal.hide = jest.fn(); + + wrapper.vm.hide(); + + expect(wrapper.vm.$refs.modal.hide).toHaveBeenCalled(); + }); + }); +}); -- GitLab From ea485926499827e13a987b0206584a2d8854458c Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Mon, 6 Mar 2023 16:06:24 +1300 Subject: [PATCH 03/15] Add MR link to comment --- .../javascripts/groups/settings/compliance_frameworks/utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js index 52418a9f48be2bf8..e1d11c168c62207a 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js @@ -58,6 +58,7 @@ export function isModalsRefactorEnabled() { * Temporary util function to determine if we are in the new refactored modal flow, or the existing full-page form * flow. We need to check both feature flag and the current page to handle the situation where the FF is enabled * but someone has navigated directly to the old page by URL. + * Clean up in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113641 */ if (!gon.features?.manageComplianceFrameworksModalsRefactor) { return false; -- GitLab From 21c0411a0dcbd358b2bb370afa90e7a4fd2b6a94 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Fri, 17 Mar 2023 16:21:48 +1300 Subject: [PATCH 04/15] Apply reviewer suggestions Use provide/inject instead of prop drilling. Wrap more non-functional changes behind feature flag. Remove low quality tests. --- .../components/create_form.vue | 62 +++++++++-------- .../components/edit_form.vue | 66 +++++++++---------- .../components/form_modal.vue | 18 ----- .../components/shared_form.vue | 12 +--- .../components/table.vue | 17 +---- .../compliance_frameworks/init_table.js | 8 ++- ..._compliance_frameworks_modals_refactor.yml | 2 +- .../components/form_modal_spec.js | 28 -------- .../components/table_spec.js | 16 +++-- 9 files changed, 84 insertions(+), 145 deletions(-) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue index 1eb27cd532b0d453..5363072bec81c3d9 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue @@ -16,20 +16,12 @@ export default { FormStatus, SharedForm, }, + inject: ['groupEditPath', 'pipelineConfigurationFullPathEnabled'], props: { - groupEditPath: { - type: String, - required: true, - }, groupPath: { type: String, required: true, }, - pipelineConfigurationFullPathEnabled: { - type: Boolean, - required: false, - default: false, - }, }, data() { return { @@ -50,7 +42,9 @@ export default { Sentry.captureException(error); }, onCancel() { - this.$emit('cancel'); + if (isModalsRefactorEnabled()) { + this.$emit('cancel'); + } }, async onSubmit() { this.saving = true; @@ -61,24 +55,36 @@ export default { this.pipelineConfigurationFullPathEnabled, ); - const { data } = await this.$apollo.mutate({ - mutation: createComplianceFrameworkMutation, - variables: { - input: { - namespacePath: this.groupPath, - params, - }, - }, - awaitRefetchQueries: true, - refetchQueries: [ - { - query: getComplianceFrameworkQuery, - variables: { - fullPath: this.groupPath, + const { data } = await this.$apollo.mutate( + isModalsRefactorEnabled() + ? { + mutation: createComplianceFrameworkMutation, + variables: { + input: { + namespacePath: this.groupPath, + params, + }, + }, + awaitRefetchQueries: true, + refetchQueries: [ + { + query: getComplianceFrameworkQuery, + variables: { + fullPath: this.groupPath, + }, + }, + ], + } + : { + mutation: createComplianceFrameworkMutation, + variables: { + input: { + namespacePath: this.groupPath, + params, + }, + }, }, - }, - ], - }); + ); const [error] = data?.createComplianceFramework?.errors || []; @@ -106,8 +112,6 @@ export default { <template> <form-status :loading="isLoading" :error="errorMessage"> <shared-form - :group-edit-path="groupEditPath" - :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" :name.sync="formData.name" :description.sync="formData.description" :pipeline-configuration-full-path.sync="formData.pipelineConfigurationFullPath" diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue index e85c9014cc9c6e99..1a22855637191eed 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue @@ -16,15 +16,8 @@ export default { FormStatus, SharedForm, }, + inject: ['graphqlFieldName', 'groupEditPath', 'pipelineConfigurationFullPathEnabled'], props: { - graphqlFieldName: { - type: String, - required: true, - }, - groupEditPath: { - type: String, - required: true, - }, groupPath: { type: String, required: true, @@ -34,11 +27,6 @@ export default { required: false, default: null, }, - pipelineConfigurationFullPathEnabled: { - type: Boolean, - required: false, - default: false, - }, }, data() { return { @@ -113,7 +101,9 @@ export default { Sentry.captureException(error); }, onCancel() { - this.$emit('cancel'); + if (isModalsRefactorEnabled()) { + this.$emit('cancel'); + } }, async onSubmit() { this.saving = true; @@ -124,24 +114,36 @@ export default { this.formData, this.pipelineConfigurationFullPathEnabled, ); - const { data } = await this.$apollo.mutate({ - mutation: updateComplianceFrameworkMutation, - variables: { - input: { - id: this.graphqlId, - params, - }, - }, - awaitRefetchQueries: true, - refetchQueries: [ - { - query: getComplianceFrameworkQuery, - variables: { - fullPath: this.groupPath, + const { data } = await this.$apollo.mutate( + isModalsRefactorEnabled() + ? { + mutation: updateComplianceFrameworkMutation, + variables: { + input: { + id: this.graphqlId, + params, + }, + }, + awaitRefetchQueries: true, + refetchQueries: [ + { + query: getComplianceFrameworkQuery, + variables: { + fullPath: this.groupPath, + }, + }, + ], + } + : { + mutation: updateComplianceFrameworkMutation, + variables: { + input: { + id: this.graphqlId, + params, + }, + }, }, - }, - ], - }); + ); const [error] = data?.updateComplianceFramework?.errors || []; @@ -170,8 +172,6 @@ export default { <form-status :loading="isLoading" :error="errorMessage"> <shared-form v-if="showForm" - :group-edit-path="groupEditPath" - :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" :name.sync="formData.name" :description.sync="formData.description" :pipeline-configuration-full-path.sync="formData.pipelineConfigurationFullPath" diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue index d350322fa6e9c0a6..9ab75e5e1bb84f23 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue @@ -17,22 +17,10 @@ export default { required: false, default: null, }, - graphqlFieldName: { - type: String, - required: true, - }, groupPath: { type: String, required: true, }, - groupEditPath: { - type: String, - required: true, - }, - pipelineConfigurationFullPathEnabled: { - type: Boolean, - required: true, - }, }, computed: { isEdit() { @@ -46,7 +34,6 @@ export default { }, methods: { show() { - this.errorMessage = ''; this.$refs.modal.show(); }, hide() { @@ -71,10 +58,7 @@ export default { v-if="isEdit" :id="framework.id" ref="formComponent" - :graphql-field-name="graphqlFieldName" :group-path="groupPath" - :group-edit-path="groupEditPath" - :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" @cancel="hide" @success="onSuccess" /> @@ -82,8 +66,6 @@ export default { v-else ref="formComponent" :group-path="groupPath" - :group-edit-path="groupEditPath" - :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" @cancel="hide" @success="onSuccess" /> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue index e7f6a9c46a8039e8..6a74df554b09c82c 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/shared_form.vue @@ -6,7 +6,6 @@ import { helpPagePath } from '~/helpers/help_page_helper'; import { validateHexColor } from '~/lib/utils/color_utils'; import { s__ } from '~/locale'; import ColorPicker from '~/vue_shared/components/color_picker/color_picker.vue'; -import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { DEBOUNCE_DELAY } from '../constants'; import { fetchPipelineConfigurationFileExists, @@ -24,7 +23,7 @@ export default { GlLink, GlSprintf, }, - mixins: [glFeatureFlagMixin()], + inject: ['groupEditPath', 'pipelineConfigurationFullPathEnabled'], props: { color: { type: String, @@ -36,20 +35,11 @@ export default { required: false, default: null, }, - groupEditPath: { - type: String, - required: true, - }, name: { type: String, required: false, default: null, }, - pipelineConfigurationFullPathEnabled: { - type: Boolean, - required: false, - default: false, - }, pipelineConfigurationFullPath: { type: String, required: false, diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue index 10171369ed48037d..494901735a788f85 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue @@ -55,22 +55,10 @@ export default { type: String, required: true, }, - graphqlFieldName: { - type: String, - required: true, - }, groupPath: { type: String, required: true, }, - groupEditPath: { - type: String, - required: true, - }, - pipelineConfigurationFullPathEnabled: { - type: Boolean, - required: true, - }, }, data() { return { @@ -320,6 +308,7 @@ export default { category="secondary" variant="confirm" size="small" + data-testid="add-framework-btn" :href="addFrameworkPath" @click="onClickAdd" > @@ -327,12 +316,10 @@ export default { </gl-button> <form-modal + v-if="glFeatures.manageComplianceFrameworksModalsRefactor" ref="formModal" :framework="markedForEdit" - :graphql-field-name="graphqlFieldName" :group-path="groupPath" - :group-edit-path="groupEditPath" - :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" @change="onChange" /> <delete-modal diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js index 987a8f94240b2f2e..f59968b9eff22007 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js @@ -28,16 +28,18 @@ const createComplianceFrameworksTableApp = (el) => { return new Vue({ el, apolloProvider, + provide: { + graphqlFieldName, + groupEditPath, + pipelineConfigurationFullPathEnabled: Boolean(pipelineConfigurationFullPathEnabled), + }, render(createElement) { return createElement(Table, { props: { addFrameworkPath, editFrameworkPath, emptyStateSvgPath, - graphqlFieldName, - groupEditPath, groupPath, - pipelineConfigurationFullPathEnabled: Boolean(pipelineConfigurationFullPathEnabled), }, }); }, diff --git a/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml b/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml index fa31309cc48707c2..36384686ed1b43d1 100644 --- a/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml +++ b/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml @@ -1,6 +1,6 @@ --- name: manage_compliance_frameworks_modals_refactor -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113637 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/389653 milestone: '15.10' type: development diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js index ff4e8e10121661c0..3081b1a9d1bbac7b 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js @@ -44,32 +44,4 @@ describe('FormModal', () => { expect(findModal().props('title')).toBe('Edit compliance framework'); }); }); - - describe('show', () => { - beforeEach(async () => { - wrapper = createComponent(); - }); - - it('shows the modal', () => { - wrapper.vm.$refs.modal.show = jest.fn(); - - wrapper.vm.show(); - - expect(wrapper.vm.$refs.modal.show).toHaveBeenCalled(); - }); - }); - - describe('hide', () => { - beforeEach(() => { - wrapper = createComponent(); - }); - - it('hides the modal', () => { - wrapper.vm.$refs.modal.hide = jest.fn(); - - wrapper.vm.hide(); - - expect(wrapper.vm.$refs.modal.hide).toHaveBeenCalled(); - }); - }); }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js index 558f4ae317249063..51e770fcdd09ddcd 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js @@ -1,4 +1,4 @@ -import { GlAlert, GlButton, GlLoadingIcon, GlLabel, GlTableLite } from '@gitlab/ui'; +import { GlAlert, GlLabel, GlLoadingIcon, GlTableLite } from '@gitlab/ui'; import * as Sentry from '@sentry/browser'; import { mount, shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; @@ -16,10 +16,10 @@ import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { - validFetchResponse, emptyFetchResponse, - validSetDefaultFrameworkResponse, errorSetDefaultFrameworkResponse, + validFetchResponse, + validSetDefaultFrameworkResponse, } from '../mock_data'; Vue.use(VueApollo); @@ -42,7 +42,7 @@ describe('Table', () => { const findDeleteModal = () => wrapper.findComponent(DeleteModal); const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findEmptyState = () => wrapper.findComponent(EmptyState); - const findAddBtn = () => wrapper.findComponent(GlButton); + const findAddBtn = () => wrapper.findByTestId('add-framework-btn'); const findAllTableActions = () => wrapper.findAllComponents(TableActions); function createMockApolloProvider(fetchMockResolver, updateMockResolver = updateDefault) { @@ -68,14 +68,16 @@ describe('Table', () => { editFrameworkPath: 'group/framework/id/edit', emptyStateSvgPath: 'dir/image.svg', groupPath: 'group-1', - groupEditPath: '/groups/group-1/-/edit#js-compliance-frameworks-settings', - graphqlFieldName: 'ComplianceManagement::Framework', - pipelineConfigurationFullPathEnabled: true, ...props, }, stubs: { GlLoadingIcon, }, + provide: { + groupEditPath: '/groups/group-1/-/edit#js-compliance-frameworks-settings', + graphqlFieldName: 'ComplianceManagement::Framework', + pipelineConfigurationFullPathEnabled: true, + }, }), ); } -- GitLab From 16a7e38150b4c12ee94a0a99b3f2fd0aaad69c16 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Wed, 22 Mar 2023 16:57:03 +1300 Subject: [PATCH 05/15] Provide groupPath instead of passing through props --- .../components/create_form.vue | 8 +------- .../components/delete_modal.vue | 5 +---- .../components/edit_form.vue | 11 ++++++----- .../components/form_modal.vue | 13 +------------ .../compliance_frameworks/components/table.vue | 7 +------ .../settings/compliance_frameworks/init_form.js | 15 ++++++++------- .../settings/compliance_frameworks/init_table.js | 2 +- .../components/create_form_spec.js | 6 +++--- .../components/delete_modal_spec.js | 2 ++ .../components/edit_form_spec.js | 9 +++++---- .../components/form_modal_spec.js | 4 ---- .../components/shared_form_spec.js | 12 +++++++++--- .../components/table_spec.js | 2 +- .../compliance_frameworks/init_form_spec.js | 11 +---------- 14 files changed, 40 insertions(+), 67 deletions(-) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue index 5363072bec81c3d9..41069c8498f0fac9 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue @@ -16,13 +16,7 @@ export default { FormStatus, SharedForm, }, - inject: ['groupEditPath', 'pipelineConfigurationFullPathEnabled'], - props: { - groupPath: { - type: String, - required: true, - }, - }, + inject: ['groupEditPath', 'groupPath', 'pipelineConfigurationFullPathEnabled'], data() { return { errorMessage: '', diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/delete_modal.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/delete_modal.vue index fb27947d9c8cccd4..d60c5e510895baee 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/delete_modal.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/delete_modal.vue @@ -11,6 +11,7 @@ export default { GlModal, GlSprintf, }, + inject: ['groupPath'], props: { name: { type: String, @@ -22,10 +23,6 @@ export default { required: false, default: null, }, - groupPath: { - type: String, - required: true, - }, }, computed: { title() { diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue index 1a22855637191eed..178a557555d47af6 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue @@ -16,12 +16,13 @@ export default { FormStatus, SharedForm, }, - inject: ['graphqlFieldName', 'groupEditPath', 'pipelineConfigurationFullPathEnabled'], + inject: [ + 'graphqlFieldName', + 'groupEditPath', + 'groupPath', + 'pipelineConfigurationFullPathEnabled', + ], props: { - groupPath: { - type: String, - required: true, - }, id: { type: String, required: false, diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue index 9ab75e5e1bb84f23..8f84c8c71a48e9e2 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue @@ -17,10 +17,6 @@ export default { required: false, default: null, }, - groupPath: { - type: String, - required: true, - }, }, computed: { isEdit() { @@ -58,16 +54,9 @@ export default { v-if="isEdit" :id="framework.id" ref="formComponent" - :group-path="groupPath" - @cancel="hide" - @success="onSuccess" - /> - <create-form - v-else - ref="formComponent" - :group-path="groupPath" @cancel="hide" @success="onSuccess" /> + <create-form v-else ref="formComponent" @cancel="hide" @success="onSuccess" /> </gl-modal> </template> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue index 494901735a788f85..ef8aeb86d1836b61 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue @@ -40,6 +40,7 @@ export default { TableActions, }, mixins: [glFeatureFlagMixin()], + inject: ['groupPath'], props: { addFrameworkPath: { type: String, @@ -55,10 +56,6 @@ export default { type: String, required: true, }, - groupPath: { - type: String, - required: true, - }, }, data() { return { @@ -319,7 +316,6 @@ export default { v-if="glFeatures.manageComplianceFrameworksModalsRefactor" ref="formModal" :framework="markedForEdit" - :group-path="groupPath" @change="onChange" /> <delete-modal @@ -327,7 +323,6 @@ export default { :id="markedForDeletion.id" ref="deleteModal" :name="markedForDeletion.name" - :group-path="groupPath" @deleting="onDeleting" @delete="onDelete" @error="onError" diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_form.js b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_form.js index ad30efc815bd03d5..9c4e1ad5e6cc6ac3 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_form.js +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_form.js @@ -2,7 +2,6 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; -import { parseBoolean } from '~/lib/utils/common_utils'; import CreateForm from './components/create_form.vue'; import EditForm from './components/edit_form.vue'; @@ -28,17 +27,19 @@ const createComplianceFrameworksFormApp = (el) => { return new Vue({ el, apolloProvider, + provide: { + graphqlFieldName, + groupEditPath, + groupPath, + pipelineConfigurationFullPathEnabled: Boolean(pipelineConfigurationFullPathEnabled), + }, render(createElement) { let element = CreateForm; - let props = { - groupEditPath, - groupPath, - pipelineConfigurationFullPathEnabled: parseBoolean(pipelineConfigurationFullPathEnabled), - }; + let props = {}; if (id) { element = EditForm; - props = { ...props, graphqlFieldName, id }; + props = { id }; } return createElement(element, { diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js index f59968b9eff22007..2c48e1471c52e0c2 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/init_table.js @@ -31,6 +31,7 @@ const createComplianceFrameworksTableApp = (el) => { provide: { graphqlFieldName, groupEditPath, + groupPath, pipelineConfigurationFullPathEnabled: Boolean(pipelineConfigurationFullPathEnabled), }, render(createElement) { @@ -39,7 +40,6 @@ const createComplianceFrameworksTableApp = (el) => { addFrameworkPath, editFrameworkPath, emptyStateSvgPath, - groupPath, }, }); }, diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js index 7996b0fa0bea922f..1418334c23e4a77d 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js @@ -22,7 +22,7 @@ jest.mock('~/lib/utils/url_utility'); describe('CreateForm', () => { let wrapper; - const propsData = { + const provideData = { groupPath: 'group-1', groupEditPath: 'group-1/edit', pipelineConfigurationFullPathEnabled: true, @@ -48,7 +48,7 @@ describe('CreateForm', () => { function createComponent(requestHandlers = []) { return shallowMount(CreateForm, { apolloProvider: createMockApolloProvider(requestHandlers), - propsData, + provide: provideData, }); } @@ -143,7 +143,7 @@ describe('CreateForm', () => { expect(create).toHaveBeenCalledWith(creationProps); expect(findFormStatus().props('loading')).toBe(true); - expect(visitUrl).toHaveBeenCalledWith(propsData.groupEditPath); + expect(visitUrl).toHaveBeenCalledWith(provideData.groupEditPath); }); }); }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/delete_modal_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/delete_modal_spec.js index a4e5040f7dcb9a04..29cf4321678385ad 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/delete_modal_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/delete_modal_spec.js @@ -47,6 +47,8 @@ describe('DeleteModal', () => { propsData: { name: frameworkFoundResponse.name, id: frameworkFoundResponse.id, + }, + provide: { groupPath: 'group-1', }, stubs: { diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/edit_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/edit_form_spec.js index cf4c4ffde20d12ea..8b439e0c37d8e810 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/edit_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/edit_form_spec.js @@ -28,10 +28,12 @@ jest.mock('~/lib/utils/url_utility'); describe('EditForm', () => { let wrapper; const propsData = { + id: '1', + }; + const provideData = { graphqlFieldName: 'ComplianceManagement::Framework', groupEditPath: 'group-1/edit', groupPath: 'group-1', - id: '1', pipelineConfigurationFullPathEnabled: true, }; @@ -59,6 +61,7 @@ describe('EditForm', () => { function createComponent(requestHandlers = []) { return shallowMount(EditForm, { apolloProvider: createMockApolloProvider(requestHandlers), + provide: provideData, propsData, }); } @@ -95,10 +98,8 @@ describe('EditForm', () => { expect(findForm().props()).toStrictEqual({ color: frameworkFoundResponse.color, description: frameworkFoundResponse.description, - groupEditPath: propsData.groupEditPath, name: frameworkFoundResponse.name, pipelineConfigurationFullPath: frameworkFoundResponse.pipelineConfigurationFullPath, - pipelineConfigurationFullPathEnabled: true, submitButtonText: 'Save changes', }); expect(findForm().exists()).toBe(true); @@ -189,7 +190,7 @@ describe('EditForm', () => { expect(update).toHaveBeenCalledWith(updateProps); expect(findFormStatus().props('loading')).toBe(true); - expect(visitUrl).toHaveBeenCalledWith(propsData.groupEditPath); + expect(visitUrl).toHaveBeenCalledWith(provideData.groupEditPath); }); }); }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js index 3081b1a9d1bbac7b..51a45d527b1d9353 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js @@ -12,10 +12,6 @@ describe('FormModal', () => { const propsData = { framework: {}, - groupPath: 'group-1', - groupEditPath: 'group-1/edit', - pipelineConfigurationFullPathEnabled: true, - graphqlFieldName: 'ComplianceManagement::Framework', }; const findModal = () => wrapper.findComponent(GlModal); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js index 720ada8635d935b0..75bad877e73cccac 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js @@ -11,9 +11,11 @@ import { GlFormGroup, GlFormInput } from '../stubs'; describe('SharedForm', () => { let wrapper; const defaultPropsData = { + submitButtonText: 'Save changes', + }; + const defaultProvideData = { groupEditPath: 'group-1', pipelineConfigurationFullPathEnabled: true, - submitButtonText: 'Save changes', }; const findForm = () => wrapper.findComponent(GlForm); @@ -29,12 +31,16 @@ describe('SharedForm', () => { const findSubmitBtn = () => wrapper.find('[data-testid="submit-btn"]'); const findCancelBtn = () => wrapper.find('[data-testid="cancel-btn"]'); - function createComponent(props = {}) { + function createComponent(props = {}, provide = {}) { return shallowMount(SharedForm, { propsData: { ...defaultPropsData, ...props, }, + provide: { + ...defaultProvideData, + ...provide, + }, stubs: { GlFormGroup, GlFormInput, @@ -68,7 +74,7 @@ describe('SharedForm', () => { it.each([true, false])( 'renders the pipeline configuration correctly when enabled is %s', (enabled) => { - wrapper = createComponent({ pipelineConfigurationFullPathEnabled: enabled }); + wrapper = createComponent({}, { pipelineConfigurationFullPathEnabled: enabled }); expect(findPipelineConfigurationGroup().exists()).toBe(enabled); }, diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js index 51e770fcdd09ddcd..3ffb0a1f2d73ae34 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js @@ -67,7 +67,6 @@ describe('Table', () => { addFrameworkPath: 'group/framework/new', editFrameworkPath: 'group/framework/id/edit', emptyStateSvgPath: 'dir/image.svg', - groupPath: 'group-1', ...props, }, stubs: { @@ -75,6 +74,7 @@ describe('Table', () => { }, provide: { groupEditPath: '/groups/group-1/-/edit#js-compliance-frameworks-settings', + groupPath: 'group-1', graphqlFieldName: 'ComplianceManagement::Framework', pipelineConfigurationFullPathEnabled: true, }, diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/init_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/init_form_spec.js index 833d2e7400702750..5ff635939da09909 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/init_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/init_form_spec.js @@ -11,7 +11,6 @@ describe('createComplianceFrameworksFormApp', () => { const groupEditPath = 'group-1/edit'; const groupPath = 'group-1'; - const pipelineConfigurationFullPathEnabled = true; const graphqlFieldName = 'field'; const testId = '1'; @@ -48,11 +47,7 @@ describe('createComplianceFrameworksFormApp', () => { }); it('parses and passes props', () => { - expect(findFormApp(CreateForm).props()).toStrictEqual({ - groupEditPath, - groupPath, - pipelineConfigurationFullPathEnabled, - }); + expect(findFormApp(CreateForm).props()).toStrictEqual({}); }); }); @@ -63,11 +58,7 @@ describe('createComplianceFrameworksFormApp', () => { it('parses and passes props', () => { expect(findFormApp(EditForm).props()).toStrictEqual({ - graphqlFieldName, - groupEditPath, - groupPath, id: testId, - pipelineConfigurationFullPathEnabled, }); }); }); -- GitLab From 96ad758082fde8e85574a16aaa434e87bb6c764a Mon Sep 17 00:00:00 2001 From: Laura Meckley <lmeckley@gitlab.com> Date: Wed, 22 Mar 2023 03:57:58 +0000 Subject: [PATCH 06/15] Change target milestone for feature flag --- .../manage_compliance_frameworks_modals_refactor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml b/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml index 36384686ed1b43d1..45687fc1576cd18a 100644 --- a/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml +++ b/ee/config/feature_flags/development/manage_compliance_frameworks_modals_refactor.yml @@ -2,7 +2,7 @@ name: manage_compliance_frameworks_modals_refactor introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113637 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/389653 -milestone: '15.10' +milestone: '15.11' type: development group: group::compliance default_enabled: false -- GitLab From a8aad120ce2ff2f629adfb045cf5355b44f55e3c Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Wed, 29 Mar 2023 14:40:31 +1300 Subject: [PATCH 07/15] Revert refactor from edit form --- .../components/edit_form.vue | 59 +++++-------------- .../components/form_modal.vue | 38 ++---------- .../components/table.vue | 7 --- .../components/table_actions.vue | 15 ----- .../settings/compliance_frameworks/utils.js | 4 +- .../components/form_modal_spec.js | 9 --- locale/gitlab.pot | 9 +-- 7 files changed, 22 insertions(+), 119 deletions(-) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue index 178a557555d47af6..6042920f19ebf092 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/edit_form.vue @@ -2,12 +2,12 @@ import * as Sentry from '@sentry/browser'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import { visitUrl } from '~/lib/utils/url_utility'; -import { __, s__ } from '~/locale'; +import { __ } from '~/locale'; import getComplianceFrameworkQuery from 'ee/graphql_shared/queries/get_compliance_framework.query.graphql'; import { FETCH_ERROR, SAVE_ERROR } from '../constants'; import updateComplianceFrameworkMutation from '../graphql/queries/update_compliance_framework.mutation.graphql'; -import { getSubmissionParams, initialiseFormData, isModalsRefactorEnabled } from '../utils'; +import { getSubmissionParams, initialiseFormData } from '../utils'; import FormStatus from './form_status.vue'; import SharedForm from './shared_form.vue'; @@ -101,11 +101,6 @@ export default { this.saveErrorMessage = userFriendlyText; Sentry.captureException(error); }, - onCancel() { - if (isModalsRefactorEnabled()) { - this.$emit('cancel'); - } - }, async onSubmit() { this.saving = true; this.saveErrorMessage = ''; @@ -115,48 +110,22 @@ export default { this.formData, this.pipelineConfigurationFullPathEnabled, ); - const { data } = await this.$apollo.mutate( - isModalsRefactorEnabled() - ? { - mutation: updateComplianceFrameworkMutation, - variables: { - input: { - id: this.graphqlId, - params, - }, - }, - awaitRefetchQueries: true, - refetchQueries: [ - { - query: getComplianceFrameworkQuery, - variables: { - fullPath: this.groupPath, - }, - }, - ], - } - : { - mutation: updateComplianceFrameworkMutation, - variables: { - input: { - id: this.graphqlId, - params, - }, - }, - }, - ); + const { data } = await this.$apollo.mutate({ + mutation: updateComplianceFrameworkMutation, + variables: { + input: { + id: this.graphqlId, + params, + }, + }, + }); const [error] = data?.updateComplianceFramework?.errors || []; if (error) { this.setSavingError(new Error(error), error); } else { - if (!isModalsRefactorEnabled()) { - visitUrl(this.groupEditPath); - return; - } - - this.$emit('success', this.$options.i18n.successMessageText); + visitUrl(this.groupEditPath); } } catch (e) { this.setSavingError(e, SAVE_ERROR); @@ -165,7 +134,6 @@ export default { }, i18n: { submitButtonText: __('Save changes'), - successMessageText: s__('ComplianceFrameworks|Changes to compliance framework saved'), }, }; </script> @@ -173,12 +141,13 @@ export default { <form-status :loading="isLoading" :error="errorMessage"> <shared-form v-if="showForm" + :group-edit-path="groupEditPath" + :pipeline-configuration-full-path-enabled="pipelineConfigurationFullPathEnabled" :name.sync="formData.name" :description.sync="formData.description" :pipeline-configuration-full-path.sync="formData.pipelineConfigurationFullPath" :color.sync="formData.color" :submit-button-text="$options.i18n.submitButtonText" - @cancel="onCancel" @submit="onSubmit" /> </form-status> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue index 8f84c8c71a48e9e2..91eed0ac1e94a90d 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/form_modal.vue @@ -1,32 +1,13 @@ <script> import { GlModal } from '@gitlab/ui'; -import { __, s__, sprintf } from '~/locale'; +import { s__ } from '~/locale'; import CreateForm from './create_form.vue'; -import EditForm from './edit_form.vue'; export default { components: { GlModal, CreateForm, - EditForm, - }, - props: { - framework: { - type: Object, - required: false, - default: null, - }, - }, - computed: { - isEdit() { - return Boolean(this.framework?.id); - }, - title() { - return sprintf(this.$options.i18n.title, { - action: this.isEdit ? this.$options.i18n.edit : this.$options.i18n.new, - }); - }, }, methods: { show() { @@ -40,23 +21,12 @@ export default { }, }, i18n: { - title: s__('ComplianceFrameworks|%{action} compliance framework'), - addFramework: s__('ComplianceFrameworks|Add framework'), - cancel: __('Cancel'), - edit: __('Edit'), - new: __('New'), + title: s__('ComplianceFrameworks|New compliance framework'), }, }; </script> <template> - <gl-modal ref="modal" :title="title" modal-id="framework-form-modal" hide-footer> - <edit-form - v-if="isEdit" - :id="framework.id" - ref="formComponent" - @cancel="hide" - @success="onSuccess" - /> - <create-form v-else ref="formComponent" @cancel="hide" @success="onSuccess" /> + <gl-modal ref="modal" :title="$options.i18n.title" modal-id="framework-form-modal" hide-footer> + <create-form @cancel="hide" @success="onSuccess" /> </gl-modal> </template> diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue index ef8aeb86d1836b61..5a16a3760099a915 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue @@ -163,10 +163,6 @@ export default { this.markedForEdit = null; this.$refs.formModal.show(); }, - markForEdit(framework) { - this.markedForEdit = framework; - this.$refs.formModal.show(); - }, markForDeletion(framework) { this.markedForDeletion = framework; this.$refs.deleteModal.show(); @@ -177,7 +173,6 @@ export default { onChange(userFriendlyText) { this.$refs.formModal.hide(); this.$toast.show(userFriendlyText); - this.markedForEdit = null; }, onDelete(id) { this.message = this.$options.i18n.deleteMessage; @@ -292,7 +287,6 @@ export default { :framework="framework" :loading="isDeleting(framework.id)" @delete="markForDeletion" - @edit="markForEdit" @setDefault="setDefaultFramework" @removeDefault="setDefaultFramework" /> @@ -315,7 +309,6 @@ export default { <form-modal v-if="glFeatures.manageComplianceFrameworksModalsRefactor" ref="formModal" - :framework="markedForEdit" @change="onChange" /> <delete-modal diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue index fab7d8d3c2c1fc14..c982ba0dad9d7dd3 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_actions.vue @@ -1,8 +1,5 @@ <script> import { GlButton, GlDropdown, GlDropdownItem, GlTooltipDirective } from '@gitlab/ui'; - -import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; - import { OPTIONS_BUTTON_LABEL, DELETE_BUTTON_LABEL, @@ -20,7 +17,6 @@ export default { GlDropdown, GlDropdownItem, }, - mixins: [glFeatureFlagMixin()], props: { framework: { type: Object, @@ -43,16 +39,6 @@ export default { return Boolean(this.framework.default); }, }, - methods: { - onEdit(event) { - if (!this.glFeatures.manageComplianceFrameworksModalsRefactor) { - return; - } - - event.preventDefault(); - this.$emit('edit', this.framework); - }, - }, }; </script> <template> @@ -66,7 +52,6 @@ export default { data-testid="compliance-framework-edit-button" icon="pencil" category="tertiary" - @click="onEdit" /> <gl-dropdown v-gl-tooltip.hover.focus="$options.i18n.optionsFramework" diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js index e1d11c168c62207a..cda12422d7a6f091 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/utils.js @@ -64,7 +64,5 @@ export function isModalsRefactorEnabled() { return false; } - return !['groups:compliance_frameworks:edit', 'groups:compliance_frameworks:new'].includes( - document.body.dataset.page, - ); + return document.body.dataset.page !== 'groups:compliance_frameworks:new'; } diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js index 51a45d527b1d9353..9f2ea95024387e85 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js @@ -3,7 +3,6 @@ import { shallowMount } from '@vue/test-utils'; import { GlModal } from '@gitlab/ui'; import FormModal from 'ee/groups/settings/compliance_frameworks/components/form_modal.vue'; -import { frameworkFoundResponse } from '../mock_data'; jest.mock('~/lib/utils/url_utility'); @@ -31,13 +30,5 @@ describe('FormModal', () => { expect(findModal().props('title')).toBe('New compliance framework'); }); - - it('sets the modal title when editing', () => { - wrapper = createComponent({ - framework: frameworkFoundResponse, - }); - - expect(findModal().props('title')).toBe('Edit compliance framework'); - }); }); }); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index cb3e89053c646e74..daca768c5cc87cbe 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10781,9 +10781,6 @@ msgstr "" msgid "Compliance violations and compliance frameworks for the group." msgstr "" -msgid "ComplianceFrameworks|%{action} compliance framework" -msgstr "" - msgid "ComplianceFrameworks|Add framework" msgstr "" @@ -10793,9 +10790,6 @@ msgstr "" msgid "ComplianceFrameworks|Cancel" msgstr "" -msgid "ComplianceFrameworks|Changes to compliance framework saved" -msgstr "" - msgid "ComplianceFrameworks|Compliance framework created" msgstr "" @@ -10850,6 +10844,9 @@ msgstr "" msgid "ComplianceFrameworks|Name is required" msgstr "" +msgid "ComplianceFrameworks|New compliance framework" +msgstr "" + msgid "ComplianceFrameworks|No compliance frameworks are set up yet" msgstr "" -- GitLab From 3a0fb5e29adfa05c8cdc5e2e21906421b63025d5 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Thu, 30 Mar 2023 12:02:51 +1300 Subject: [PATCH 08/15] Add additional tests --- .../components/create_form_spec.js | 26 ++++++++- .../components/form_modal_spec.js | 6 -- .../components/table_empty_state_spec.js | 29 +++++++++- .../components/table_spec.js | 55 ++++++++++++++++++- 4 files changed, 105 insertions(+), 11 deletions(-) diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js index 1418334c23e4a77d..45a80501e57bf68d 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js @@ -12,12 +12,16 @@ import getComplianceFrameworkQuery from 'ee/groups/settings/compliance_framework import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { visitUrl } from '~/lib/utils/url_utility'; - +import { isModalsRefactorEnabled } from 'ee/groups/settings/compliance_frameworks/utils'; import { errorCreateResponse, validCreateResponse, validFetchResponse } from '../mock_data'; Vue.use(VueApollo); jest.mock('~/lib/utils/url_utility'); +jest.mock('ee/groups/settings/compliance_frameworks/utils', () => ({ + ...jest.requireActual('ee/groups/settings/compliance_frameworks/utils'), + isModalsRefactorEnabled: jest.fn(), +})); describe('CreateForm', () => { let wrapper; @@ -146,4 +150,24 @@ describe('CreateForm', () => { expect(visitUrl).toHaveBeenCalledWith(provideData.groupEditPath); }); }); + + describe('onCancel', () => { + beforeEach(() => { + wrapper = createComponent(); + }); + + it('should emit a cancel event when the "manageComplianceFrameworksModalsRefactor" feature flag is enabled', () => { + isModalsRefactorEnabled.mockReturnValue(true); + findForm().vm.$emit('cancel'); + + expect(wrapper.emitted('cancel')).toHaveLength(1); + }); + + it('should not emit a cancel event when the "manageComplianceFrameworksModalsRefactor" feature flag is disabled', () => { + isModalsRefactorEnabled.mockReturnValue(false); + findForm().vm.$emit('cancel'); + + expect(wrapper.emitted('cancel')).toBeUndefined(); + }); + }); }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js index 9f2ea95024387e85..9e4690fc15962444 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js @@ -1,5 +1,4 @@ import { shallowMount } from '@vue/test-utils'; - import { GlModal } from '@gitlab/ui'; import FormModal from 'ee/groups/settings/compliance_frameworks/components/form_modal.vue'; @@ -9,16 +8,11 @@ jest.mock('~/lib/utils/url_utility'); describe('FormModal', () => { let wrapper; - const propsData = { - framework: {}, - }; - const findModal = () => wrapper.findComponent(GlModal); function createComponent(props = {}, mountFn = shallowMount) { return mountFn(FormModal, { propsData: { - ...propsData, ...props, }, }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js index c69cde67b05ab364..ebaeba89c7331913 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js @@ -7,13 +7,18 @@ describe('TableEmptyState', () => { let wrapper; const findEmptyState = () => wrapper.findComponent(GlEmptyState); - const createComponent = (props = {}) => { + const findAddFrameworkButton = () => wrapper.findComponent(GlButton); + + const createComponent = (props = {}, glFeatures = {}) => { wrapper = shallowMount(TableEmptyState, { propsData: { imagePath: 'dir/image.svg', addFrameworkPath: 'group/framework/new', ...props, }, + provide: { + glFeatures, + }, }); }; @@ -36,8 +41,28 @@ describe('TableEmptyState', () => { it('has an add framework action', () => { createComponent(); - const button = wrapper.findComponent(GlButton); + const button = findAddFrameworkButton(); expect(button.text()).toBe('Add framework'); }); + + describe('"manageComplianceFrameworksModalsRefactor" feature flag', () => { + it('emits the expected event when flag is enabled and add framework button is clicked', () => { + createComponent({}, { manageComplianceFrameworksModalsRefactor: true }); + + const clickEvent = new Event('click'); + findAddFrameworkButton().vm.$emit('click', clickEvent); + + expect(wrapper.emitted('addFramework')).toHaveLength(1); + expect(wrapper.emitted('addFramework')[0][0]).toBe(clickEvent); + }); + + it('does not emit events when flag is disabled and add framework button is clicked', () => { + createComponent({}, { manageComplianceFrameworksModalsRefactor: false }); + + findAddFrameworkButton().vm.$emit('click', new Event('click')); + + expect(wrapper.emitted('addFramework')).toBeUndefined(); + }); + }); }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js index 3ffb0a1f2d73ae34..59292bb9e77adf90 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js @@ -1,19 +1,21 @@ -import { GlAlert, GlLabel, GlLoadingIcon, GlTableLite } from '@gitlab/ui'; +import { GlAlert, GlLabel, GlLoadingIcon, GlTableLite, GlModal } from '@gitlab/ui'; import * as Sentry from '@sentry/browser'; import { mount, shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; - import VueApollo from 'vue-apollo'; import Table from 'ee/groups/settings/compliance_frameworks/components/table.vue'; import EmptyState from 'ee/groups/settings/compliance_frameworks/components/table_empty_state.vue'; import TableActions from 'ee/groups/settings/compliance_frameworks/components/table_actions.vue'; import DeleteModal from 'ee/groups/settings/compliance_frameworks/components/delete_modal.vue'; +import CreateForm from 'ee/groups/settings/compliance_frameworks/components/create_form.vue'; +import FormModal from 'ee/groups/settings/compliance_frameworks/components/form_modal.vue'; import { PIPELINE_CONFIGURATION_PATH_FORMAT } from 'ee/groups/settings/compliance_frameworks/constants'; import getComplianceFrameworkQuery from 'ee/graphql_shared/queries/get_compliance_framework.query.graphql'; import updateComplianceFrameworkMutation from 'ee/groups/settings/compliance_frameworks/graphql//queries/update_compliance_framework.mutation.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import { stubComponent } from 'helpers/stub_component'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import { emptyFetchResponse, @@ -28,6 +30,9 @@ describe('Table', () => { let wrapper; const sentryError = new Error('Network error'); + const mockModalShow = jest.fn(); + const mockModalHide = jest.fn(); + const mockToastShow = jest.fn(); const fetch = jest.fn().mockResolvedValue(validFetchResponse); const fetchEmpty = jest.fn().mockResolvedValue(emptyFetchResponse); const fetchLoading = jest.fn().mockResolvedValue(new Promise(() => {})); @@ -44,6 +49,7 @@ describe('Table', () => { const findEmptyState = () => wrapper.findComponent(EmptyState); const findAddBtn = () => wrapper.findByTestId('add-framework-btn'); const findAllTableActions = () => wrapper.findAllComponents(TableActions); + const findFormModal = () => wrapper.findComponent(FormModal); function createMockApolloProvider(fetchMockResolver, updateMockResolver = updateDefault) { Vue.use(VueApollo); @@ -59,6 +65,7 @@ describe('Table', () => { updateMockResolver, props = {}, mountFn = shallowMount, + glFeatures = {}, ) { return extendedWrapper( mountFn(Table, { @@ -70,13 +77,26 @@ describe('Table', () => { ...props, }, stubs: { + CreateForm: stubComponent(CreateForm), GlLoadingIcon, + GlModal: stubComponent(GlModal, { + methods: { + show: mockModalShow, + hide: mockModalHide, + }, + }), + }, + mocks: { + $toast: { + show: mockToastShow, + }, }, provide: { groupEditPath: '/groups/group-1/-/edit#js-compliance-frameworks-settings', groupPath: 'group-1', graphqlFieldName: 'ComplianceManagement::Framework', pipelineConfigurationFullPathEnabled: true, + glFeatures, }, }), ); @@ -347,4 +367,35 @@ describe('Table', () => { }); }); }); + + describe('create framework', () => { + describe('with "manageComplianceFrameworksModalsRefactor" feature flag enabled', () => { + beforeEach(async () => { + wrapper = createComponentWithApollo(fetch, updateDefault, {}, mount, { + manageComplianceFrameworksModalsRefactor: true, + }); + await waitForPromises(); + }); + + it('shows modal when clicking add framework button', () => { + findAddBtn().trigger('click'); + + expect(mockModalShow).toHaveBeenCalled(); + }); + + it('hides modal when successful', () => { + const successMessage = 'woo!'; + findFormModal().vm.$emit('change', successMessage); + + expect(mockModalHide).toHaveBeenCalled(); + }); + + it('shows a toast when successful', () => { + const successMessage = 'woo!'; + findFormModal().vm.$emit('change', successMessage); + + expect(mockToastShow).toHaveBeenCalledWith(successMessage); + }); + }); + }); }); -- GitLab From ebb066404d0124d846876d3f16bbbc29f3a6d622 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Thu, 30 Mar 2023 12:14:12 +1300 Subject: [PATCH 09/15] Fix import path issues after rebase --- .../settings/compliance_frameworks/components/create_form.vue | 2 +- .../compliance_frameworks/components/create_form_spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue index 41069c8498f0fac9..853751e06167ed49 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue @@ -4,10 +4,10 @@ import * as Sentry from '@sentry/browser'; import { visitUrl } from '~/lib/utils/url_utility'; import { s__ } from '~/locale'; +import getComplianceFrameworkQuery from 'ee/graphql_shared/queries/get_compliance_framework.query.graphql'; import { SAVE_ERROR } from '../constants'; import createComplianceFrameworkMutation from '../graphql/queries/create_compliance_framework.mutation.graphql'; import { getSubmissionParams, initialiseFormData, isModalsRefactorEnabled } from '../utils'; -import getComplianceFrameworkQuery from '../graphql/queries/get_compliance_framework.query.graphql'; import FormStatus from './form_status.vue'; import SharedForm from './shared_form.vue'; diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js index 45a80501e57bf68d..0c23bb4349f4bc72 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/create_form_spec.js @@ -8,7 +8,7 @@ import FormStatus from 'ee/groups/settings/compliance_frameworks/components/form import SharedForm from 'ee/groups/settings/compliance_frameworks/components/shared_form.vue'; import { SAVE_ERROR } from 'ee/groups/settings/compliance_frameworks/constants'; import createComplianceFrameworkMutation from 'ee/groups/settings/compliance_frameworks/graphql/queries/create_compliance_framework.mutation.graphql'; -import getComplianceFrameworkQuery from 'ee/groups/settings/compliance_frameworks/graphql/queries/get_compliance_framework.query.graphql'; +import getComplianceFrameworkQuery from 'ee/graphql_shared/queries/get_compliance_framework.query.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { visitUrl } from '~/lib/utils/url_utility'; -- GitLab From 198efcab89d338acc765c17bbda6edc86ac9e8eb Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Tue, 11 Apr 2023 15:10:22 +1200 Subject: [PATCH 10/15] Conditionally render href This ensures we have the correct element in each situation --- .../components/table.vue | 7 +++++- .../components/table_empty_state.vue | 9 ++++++- .../components/table_empty_state_spec.js | 25 ++++++++++++++++--- .../components/table_spec.js | 7 ++++++ 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue index 5a16a3760099a915..39af07496d24cfb4 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue @@ -142,6 +142,11 @@ export default { showAddButton() { return this.hasLoaded && this.addFrameworkPath && !this.isEmpty; }, + addFrameworkHref() { + return this.glFeatures.manageComplianceFrameworksModalsRefactor + ? undefined + : this.addFrameworkPath; + }, }, methods: { onClickAdd(event) { @@ -300,7 +305,7 @@ export default { variant="confirm" size="small" data-testid="add-framework-btn" - :href="addFrameworkPath" + :href="addFrameworkHref" @click="onClickAdd" > {{ $options.i18n.addBtn }} diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue index f333de7250b8e76e..6a6e25bf96535e6f 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table_empty_state.vue @@ -21,6 +21,13 @@ export default { default: null, }, }, + computed: { + addFrameworkHref() { + return this.glFeatures.manageComplianceFrameworksModalsRefactor + ? undefined + : this.addFrameworkPath; + }, + }, methods: { onAddFramework(event) { if (!this.glFeatures.manageComplianceFrameworksModalsRefactor) { @@ -51,7 +58,7 @@ export default { </template> <template #actions> <gl-button - :href="addFrameworkPath" + :href="addFrameworkHref" category="primary" variant="confirm" class="gl-mb-3" diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js index ebaeba89c7331913..c78ae7cd7e059cd7 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_empty_state_spec.js @@ -1,19 +1,20 @@ import { GlButton, GlEmptyState } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; +import { shallowMount, mount } from '@vue/test-utils'; import TableEmptyState from 'ee/groups/settings/compliance_frameworks/components/table_empty_state.vue'; describe('TableEmptyState', () => { let wrapper; + const addFrameworkPath = 'group/framework/new'; const findEmptyState = () => wrapper.findComponent(GlEmptyState); const findAddFrameworkButton = () => wrapper.findComponent(GlButton); - const createComponent = (props = {}, glFeatures = {}) => { - wrapper = shallowMount(TableEmptyState, { + const createComponent = (props = {}, glFeatures = {}, mountFn = shallowMount) => { + wrapper = mountFn(TableEmptyState, { propsData: { imagePath: 'dir/image.svg', - addFrameworkPath: 'group/framework/new', + addFrameworkPath, ...props, }, provide: { @@ -64,5 +65,21 @@ describe('TableEmptyState', () => { expect(wrapper.emitted('addFramework')).toBeUndefined(); }); + + it('renders a button when the flag is enabled', () => { + createComponent({}, { manageComplianceFrameworksModalsRefactor: true }, mount); + const btn = findAddFrameworkButton(); + + expect(btn.find('button').exists()).toBe(true); + expect(btn.find('a').exists()).toBe(false); + }); + + it('renders a link when the flag is disabled', () => { + createComponent({}, { manageComplianceFrameworksModalsRefactor: false }, mount); + const btn = findAddFrameworkButton(); + + expect(btn.find('a').exists()).toBe(true); + expect(btn.find('button').exists()).toBe(false); + }); }); }); diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js index 59292bb9e77adf90..146d82124abf2d45 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js @@ -377,6 +377,13 @@ describe('Table', () => { await waitForPromises(); }); + it('renders a button instead of a link', () => { + const btn = findAddBtn(); + + expect(btn.find('button').exists()).toBe(true); + expect(btn.find('a').exists()).toBe(false); + }); + it('shows modal when clicking add framework button', () => { findAddBtn().trigger('click'); -- GitLab From af2cb8cd76990a43d20c6cf5e2f5c1d54682bda2 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Tue, 11 Apr 2023 15:10:49 +1200 Subject: [PATCH 11/15] Remove unused state Missed when removing changes to edit form --- .../groups/settings/compliance_frameworks/components/table.vue | 2 -- 1 file changed, 2 deletions(-) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue index 39af07496d24cfb4..732c4248a4272638 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/table.vue @@ -59,7 +59,6 @@ export default { }, data() { return { - markedForEdit: {}, markedForDeletion: {}, deletingFrameworksIds: [], complianceFrameworks: [], @@ -165,7 +164,6 @@ export default { this.message = null; }, markForAdd() { - this.markedForEdit = null; this.$refs.formModal.show(); }, markForDeletion(framework) { -- GitLab From ad2399ce2bbc700823e97600d2ac9bba2a477bdb Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Tue, 11 Apr 2023 15:13:16 +1200 Subject: [PATCH 12/15] Change apollo mutation Merge in additional properties when FF is enabled instead of duplicating entire object --- .../components/create_form.vue | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue index 853751e06167ed49..c00001d95ac7f4d5 100644 --- a/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue +++ b/ee/app/assets/javascripts/groups/settings/compliance_frameworks/components/create_form.vue @@ -49,16 +49,16 @@ export default { this.pipelineConfigurationFullPathEnabled, ); - const { data } = await this.$apollo.mutate( - isModalsRefactorEnabled() + const { data } = await this.$apollo.mutate({ + mutation: createComplianceFrameworkMutation, + variables: { + input: { + namespacePath: this.groupPath, + params, + }, + }, + ...(isModalsRefactorEnabled() ? { - mutation: createComplianceFrameworkMutation, - variables: { - input: { - namespacePath: this.groupPath, - params, - }, - }, awaitRefetchQueries: true, refetchQueries: [ { @@ -69,16 +69,8 @@ export default { }, ], } - : { - mutation: createComplianceFrameworkMutation, - variables: { - input: { - namespacePath: this.groupPath, - params, - }, - }, - }, - ); + : {}), + }); const [error] = data?.createComplianceFramework?.errors || []; -- GitLab From 659326311b1f6a53ebc3bd81a04cd05ba4166de0 Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Tue, 11 Apr 2023 15:14:26 +1200 Subject: [PATCH 13/15] Add test for success event --- .../components/form_modal_spec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js index 9e4690fc15962444..a962c1d0af017212 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/form_modal_spec.js @@ -2,6 +2,7 @@ import { shallowMount } from '@vue/test-utils'; import { GlModal } from '@gitlab/ui'; import FormModal from 'ee/groups/settings/compliance_frameworks/components/form_modal.vue'; +import CreateForm from 'ee/groups/settings/compliance_frameworks/components/create_form.vue'; jest.mock('~/lib/utils/url_utility'); @@ -9,6 +10,7 @@ describe('FormModal', () => { let wrapper; const findModal = () => wrapper.findComponent(GlModal); + const findCreateForm = () => wrapper.findComponent(CreateForm); function createComponent(props = {}, mountFn = shallowMount) { return mountFn(FormModal, { @@ -24,5 +26,15 @@ describe('FormModal', () => { expect(findModal().props('title')).toBe('New compliance framework'); }); + + it('emits the change event on success', async () => { + wrapper = createComponent(); + + expect(wrapper.emitted('change')).toBe(undefined); + + await findCreateForm().vm.$emit('success'); + + expect(wrapper.emitted('change')).toHaveLength(1); + }); }); }); -- GitLab From 9307fe18949e88bc9246be9e078c0d9536f6861d Mon Sep 17 00:00:00 2001 From: Elwyn Benson <ebenson@gitlab.com> Date: Tue, 11 Apr 2023 15:28:20 +1200 Subject: [PATCH 14/15] Test cancel event --- .../components/shared_form_spec.js | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js index 75bad877e73cccac..40c33e1ea6036290 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/shared_form_spec.js @@ -240,4 +240,24 @@ describe('SharedForm', () => { expect(Utils.fetchPipelineConfigurationFileExists).toHaveBeenCalledWith('foo.yaml@bar/baz'); }); }); + + describe('on cancelling', () => { + it('should emit a cancel event when the "manageComplianceFrameworksModalsRefactor" feature flag is enabled', () => { + jest.spyOn(Utils, 'isModalsRefactorEnabled').mockReturnValue(true); + wrapper = createComponent(); + + findCancelBtn().vm.$emit('click', new MouseEvent('click')); + + expect(wrapper.emitted('cancel')).toHaveLength(1); + }); + + it('should not emit a cancel event when the "manageComplianceFrameworksModalsRefactor" feature flag is disabled', () => { + jest.spyOn(Utils, 'isModalsRefactorEnabled').mockReturnValue(false); + wrapper = createComponent(); + + findCancelBtn().vm.$emit('click', new MouseEvent('click')); + + expect(wrapper.emitted('cancel')).toBeUndefined(); + }); + }); }); -- GitLab From d71a5141e0a1f8c25cad4a15516e4102c8eae832 Mon Sep 17 00:00:00 2001 From: Natalia Tepluhina <ntepluhina@gitlab.com> Date: Tue, 11 Apr 2023 03:29:38 +0000 Subject: [PATCH 15/15] Prefer use of $emit --- .../settings/compliance_frameworks/components/table_spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js index 146d82124abf2d45..c053d3b840253e88 100644 --- a/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js +++ b/ee/spec/frontend/groups/settings/compliance_frameworks/components/table_spec.js @@ -385,7 +385,7 @@ describe('Table', () => { }); it('shows modal when clicking add framework button', () => { - findAddBtn().trigger('click'); + findAddBtn().vm.$emit('click', new MouseEvent('click')); expect(mockModalShow).toHaveBeenCalled(); }); -- GitLab