Skip to content

G
gitlab-ce
Commit d60c6f46 authored by Tom Quirk's avatar Tom Quirk
Browse files
Options

Remove vue_integration_form feature flag 

Changelog: changed
parent 6cfa36b0
Hide whitespace changes
Inline Side-by-side
Showing with 180 additions and 349 deletions
app/assets/javascripts/integrations/constants.js
View file @ d60c6f46
......@@ -24,5 +24,3 @@ export const I18N_SUCCESSFUL_CONNECTION_MESSAGE = s__('Integrations|Connection s
export const settingsTabTitle = __('Settings');
export const overridesTabTitle = s__('Integrations|Projects using custom settings');
export const INTEGRATION_FORM_SELECTOR = '.js-integration-settings-form';
app/assets/javascripts/integrations/edit/components/integration_form.vue
View file @ d60c6f46
......@@ -8,7 +8,6 @@ import {
I18N_FETCH_TEST_SETTINGS_DEFAULT_ERROR_MESSAGE,
I18N_DEFAULT_ERROR_MESSAGE,
I18N_SUCCESSFUL_CONNECTION_MESSAGE,
INTEGRATION_FORM_SELECTOR,
integrationLevels,
} from '~/integrations/constants';
import { refreshCurrentPage } from '~/lib/utils/url_utility';
......@@ -82,28 +81,9 @@ export default {
disableButtons() {
return Boolean(this.isSaving || this.isResetting || this.isTesting);
},
useVueForm() {
return this.glFeatures?.vueIntegrationForm;
form() {
return this.$refs.integrationForm.$el;
},
formContainerProps() {
return this.useVueForm
? {
ref: 'integrationForm',
method: 'post',
class: 'gl-mb-3 gl-show-field-errors integration-settings-form',
action: this.propsSource.formPath,
novalidate: !this.integrationActive,
}
: {};
},
formContainer() {
return this.useVueForm ? GlForm : 'div';
},
},
mounted() {
this.form = this.useVueForm
? this.$refs.integrationForm.$el
: document.querySelector(INTEGRATION_FORM_SELECTOR);
},
methods: {
...mapActions(['setOverride', 'requestJiraIssueTypes']),
......@@ -122,13 +102,13 @@ export default {
this.form.submit();
},
onTestClick() {
this.isTesting = true;
if (!this.form.checkValidity()) {
this.setIsValidated();
return;
}
this.isTesting = true;
testIntegrationSettings(this.propsSource.testPath, this.getFormData())
.then(({ data: { error, message = I18N_FETCH_TEST_SETTINGS_DEFAULT_ERROR_MESSAGE } }) => {
if (error) {
......@@ -171,16 +151,6 @@ export default {
},
onToggleIntegrationState(integrationActive) {
this.integrationActive = integrationActive;
if (!this.form || this.useVueForm) {
return;
}
// If integration will be active, enable form validation.
if (integrationActive) {
this.form.removeAttribute('novalidate');
} else {
this.form.setAttribute('novalidate', true);
}
},
},
helpHtmlConfig: {
......@@ -193,17 +163,21 @@ export default {
</script>
<template>
<component :is="formContainer" v-bind="formContainerProps">
<template v-if="useVueForm">
<input type="hidden" name="_method" value="put" />
<input type="hidden" name="authenticity_token" :value="$options.csrf.token" />
<input
type="hidden"
name="redirect_to"
:value="propsSource.redirectTo"
data-testid="redirect-to-field"
/>
</template>
<gl-form
ref="integrationForm"
method="post"
class="gl-mb-3 gl-show-field-errors integration-settings-form"
:action="propsSource.formPath"
:novalidate="!integrationActive"
>
<input type="hidden" name="_method" value="put" />
<input type="hidden" name="authenticity_token" :value="$options.csrf.token" />
<input
type="hidden"
name="redirect_to"
:value="propsSource.redirectTo"
data-testid="redirect-to-field"
/>
<override-dropdown
v-if="defaultState !== null"
......@@ -316,5 +290,5 @@ export default {
</div>
</div>
</div>
</component>
</gl-form>
</template>
app/controllers/concerns/integrations/actions.rb
View file @ d60c6f46
......@@ -8,9 +8,6 @@ module Integrations::Actions
include IntegrationsHelper
before_action :integration, only: [:edit, :update, :overrides, :test]
before_action do
push_frontend_feature_flag(:vue_integration_form, current_user, default_enabled: :yaml)
end
urgency :low, [:test]
end
......
app/controllers/projects/services_controller.rb
View file @ d60c6f46
......@@ -12,9 +12,6 @@ class Projects::ServicesController < Projects::ApplicationController
before_action :web_hook_logs, only: [:edit, :update]
before_action :set_deprecation_notice_for_prometheus_integration, only: [:edit, :update]
before_action :redirect_deprecated_prometheus_integration, only: [:update]
before_action do
push_frontend_feature_flag(:vue_integration_form, current_user, default_enabled: :yaml)
end
respond_to :html
......
app/helpers/integrations_helper.rb
View file @ d60c6f46
......@@ -228,10 +228,6 @@ module IntegrationsHelper
name: integration.to_param
}
end
def vue_integration_form_enabled?
Feature.enabled?(:vue_integration_form, current_user, default_enabled: :yaml)
end
end
IntegrationsHelper.prepend_mod_with('IntegrationsHelper')
......
app/views/projects/services/_form.html.haml
View file @ d60c6f46
......@@ -6,13 +6,7 @@
- if integration.operating?
= sprite_icon('check', css_class: 'gl-text-green-500')
- if vue_integration_form_enabled?
= render 'shared/integration_settings', integration: integration
- else
= form_for(integration, as: :service, url: scoped_integration_path(integration, project: @project, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => test_project_integration_path(@project, integration), testid: 'integration-form' } }) do |form|
= render 'shared/integration_settings', form: form, integration: integration
%input{ id: 'services_redirect_to', type: 'hidden', name: 'redirect_to', value: request.referer }
= render 'shared/integration_settings', integration: integration
- if lookup_context.template_exists?('show', "projects/services/#{integration.to_param}", true)
%hr
= render "projects/services/#{integration.to_param}/show", integration: integration
app/views/shared/integrations/_form.html.haml deleted 100644 → 0
View file @ 6cfa36b0
- integration = local_assigns.fetch(:integration)
= form_for integration, as: :service, url: scoped_integration_path(integration, group: @group), method: :put, html: { class: 'gl-show-field-errors integration-settings-form js-integration-settings-form', data: { 'test-url' => scoped_test_integration_path(integration, group: @group), testid: 'integration-form' } } do |form|
= render 'shared/integration_settings', form: form, integration: integration
app/views/shared/integrations/edit.html.haml
View file @ d60c6f46
......@@ -7,7 +7,4 @@
= @integration.title
= render 'shared/integrations/tabs', integration: @integration, active_tab: 'edit' do
- if vue_integration_form_enabled?
= render 'shared/integration_settings', integration: @integration
- else
= render 'shared/integrations/form', integration: @integration
= render 'shared/integration_settings', integration: @integration
config/feature_flags/development/vue_integration_form.yml deleted 100644 → 0
View file @ 6cfa36b0
---
name: vue_integration_form
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77934
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/350444
milestone: '14.7'
type: development
group: group::integrations
default_enabled: false
spec/features/admin/integrations/user_activates_mattermost_slash_command_spec.rb
View file @ d60c6f46
......@@ -19,19 +19,4 @@ RSpec.describe 'User activates the instance-level Mattermost Slash Command integ
expect(page).to have_link('Settings', href: edit_path)
expect(page).to have_link('Projects using custom settings', href: overrides_path)
end
it 'does not render integration form element' do
expect(page).not_to have_selector('[data-testid="integration-form"]')
end
context 'when `vue_integration_form` feature flag is disabled' do
before do
stub_feature_flags(vue_integration_form: false)
visit_instance_integration('Mattermost slash commands')
end
it 'renders integration form element' do
expect(page).to have_selector('[data-testid="integration-form"]')
end
end
end
spec/frontend/integrations/edit/components/integration_form_spec.js
View file @ d60c6f46
......@@ -33,13 +33,6 @@ describe('IntegrationForm', () => {
let wrapper;
let dispatch;
let mockAxios;
let mockForm;
let vueIntegrationFormFeatureFlag;
const createForm = () => {
mockForm = document.createElement('form');
jest.spyOn(document, 'querySelector').mockReturnValue(mockForm);
};
const createComponent = ({
customStateProps = {},
......@@ -53,10 +46,6 @@ describe('IntegrationForm', () => {
});
dispatch = jest.spyOn(store, 'dispatch').mockImplementation();
if (!vueIntegrationFormFeatureFlag) {
createForm();
}
wrapper = mountFn(IntegrationForm, {
propsData: { ...props },
store,
......@@ -72,11 +61,6 @@ describe('IntegrationForm', () => {
show: mockToastShow,
},
},
provide: {
glFeatures: {
vueIntegrationForm: vueIntegrationFormFeatureFlag,
},
},
});
};
......@@ -93,14 +77,8 @@ describe('IntegrationForm', () => {
const findTriggerFields = () => wrapper.findComponent(TriggerFields);
const findGlForm = () => wrapper.findComponent(GlForm);
const findRedirectToField = () => wrapper.findByTestId('redirect-to-field');
const findFormElement = () => (vueIntegrationFormFeatureFlag ? findGlForm().element : mockForm);
const findDynamicField = () => wrapper.findComponent(DynamicField);
const mockFormFunctions = ({ checkValidityReturn }) => {
jest.spyOn(findFormElement(), 'checkValidity').mockReturnValue(checkValidityReturn);
jest.spyOn(findFormElement(), 'submit');
};
beforeEach(() => {
mockAxios = new MockAdapter(axios);
});
......@@ -355,17 +333,14 @@ describe('IntegrationForm', () => {
});
});
describe('when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled', () => {
it('renders hidden fields', () => {
vueIntegrationFormFeatureFlag = true;
createComponent({
customStateProps: {
redirectTo: '/services',
},
});
expect(findRedirectToField().attributes('value')).toBe('/services');
it('renders hidden fields', () => {
createComponent({
customStateProps: {
redirectTo: '/services',
},
});
expect(findRedirectToField().attributes('value')).toBe('/services');
});
});
......@@ -387,218 +362,200 @@ describe('IntegrationForm', () => {
});
describe.each`
formActive | vueIntegrationFormEnabled | novalidate
${true} | ${true} | ${null}
${false} | ${true} | ${'novalidate'}
${true} | ${false} | ${null}
${false} | ${false} | ${'true'}
formActive | novalidate
${true} | ${undefined}
${false} | ${'true'}
`(
'when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled and `toggle-integration-active` is emitted with $formActive',
({ formActive, vueIntegrationFormEnabled, novalidate }) => {
'when `toggle-integration-active` is emitted with $formActive',
({ formActive, novalidate }) => {
beforeEach(async () => {
vueIntegrationFormFeatureFlag = vueIntegrationFormEnabled;
createComponent({
customStateProps: {
showActive: true,
initialActivated: false,
},
mountFn: mountExtended,
});
mockFormFunctions({ checkValidityReturn: false });
await findActiveCheckbox().vm.$emit('toggle-integration-active', formActive);
});
it(`sets noValidate to ${novalidate}`, () => {
expect(findFormElement().getAttribute('novalidate')).toBe(novalidate);
expect(findGlForm().attributes('novalidate')).toBe(novalidate);
});
},
);
});
describe.each`
vueIntegrationFormEnabled
${true}
${false}
`(
'when `vueIntegrationForm` feature flag is $vueIntegrationFormEnabled',
({ vueIntegrationFormEnabled }) => {
beforeEach(() => {
vueIntegrationFormFeatureFlag = vueIntegrationFormEnabled;
});
describe('when `save` button is clicked', () => {
describe('buttons', () => {
beforeEach(async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
initialActivated: true,
},
mountFn: mountExtended,
});
await findProjectSaveButton().vm.$emit('click', new Event('click'));
});
describe('when `save` button is clicked', () => {
describe('buttons', () => {
beforeEach(async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
initialActivated: true,
},
mountFn: mountExtended,
});
it('sets save button `loading` prop to `true`', () => {
expect(findProjectSaveButton().props('loading')).toBe(true);
});
await findProjectSaveButton().vm.$emit('click', new Event('click'));
});
it('sets test button `disabled` prop to `true`', () => {
expect(findTestButton().props('disabled')).toBe(true);
it('sets save button `loading` prop to `true`', () => {
expect(findProjectSaveButton().props('loading')).toBe(true);
});
it('sets test button `disabled` prop to `true`', () => {
expect(findTestButton().props('disabled')).toBe(true);
});
});
describe.each`
checkValidityReturn | integrationActive
${true} | ${false}
${true} | ${true}
${false} | ${false}
`(
'when form is valid (checkValidity returns $checkValidityReturn and integrationActive is $integrationActive)',
({ integrationActive, checkValidityReturn }) => {
beforeEach(async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
initialActivated: integrationActive,
},
mountFn: mountExtended,
});
jest.spyOn(findGlForm().element, 'submit');
jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(checkValidityReturn);
await findProjectSaveButton().vm.$emit('click', new Event('click'));
});
it('submit form', () => {
expect(findGlForm().element.submit).toHaveBeenCalledTimes(1);
});
},
);
describe.each`
checkValidityReturn | integrationActive
${true} | ${false}
${true} | ${true}
${false} | ${false}
`(
'when form is valid (checkValidity returns $checkValidityReturn and integrationActive is $integrationActive)',
({ integrationActive, checkValidityReturn }) => {
beforeEach(async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
initialActivated: integrationActive,
},
mountFn: mountExtended,
});
mockFormFunctions({ checkValidityReturn });
await findProjectSaveButton().vm.$emit('click', new Event('click'));
});
it('submits form', () => {
expect(findFormElement().submit).toHaveBeenCalledTimes(1);
});
describe('when form is invalid (checkValidity returns false and integrationActive is true)', () => {
beforeEach(async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
initialActivated: true,
fields: [mockField],
},
);
describe('when form is invalid (checkValidity returns false and integrationActive is true)', () => {
beforeEach(async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
initialActivated: true,
fields: [mockField],
},
mountFn: mountExtended,
});
mockFormFunctions({ checkValidityReturn: false });
await findProjectSaveButton().vm.$emit('click', new Event('click'));
});
mountFn: mountExtended,
});
jest.spyOn(findGlForm().element, 'submit');
jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(false);
it('does not submit form', () => {
expect(findFormElement().submit).not.toHaveBeenCalled();
});
await findProjectSaveButton().vm.$emit('click', new Event('click'));
});
it('sets save button `loading` prop to `false`', () => {
expect(findProjectSaveButton().props('loading')).toBe(false);
});
it('does not submit form', () => {
expect(findGlForm().element.submit).not.toHaveBeenCalled();
});
it('sets test button `disabled` prop to `false`', () => {
expect(findTestButton().props('disabled')).toBe(false);
});
it('sets save button `loading` prop to `false`', () => {
expect(findProjectSaveButton().props('loading')).toBe(false);
});
it('sets `isValidated` props on form fields', () => {
expect(findDynamicField().props('isValidated')).toBe(true);
});
it('sets test button `disabled` prop to `false`', () => {
expect(findTestButton().props('disabled')).toBe(false);
});
it('sets `isValidated` props on form fields', () => {
expect(findDynamicField().props('isValidated')).toBe(true);
});
});
});
describe('when `test` button is clicked', () => {
describe('when form is invalid', () => {
it('sets `isValidated` props on form fields', async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
fields: [mockField],
},
mountFn: mountExtended,
});
jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(false);
await findTestButton().vm.$emit('click', new Event('click'));
expect(findDynamicField().props('isValidated')).toBe(true);
});
});
describe('when `test` button is clicked', () => {
describe('when form is invalid', () => {
it('sets `isValidated` props on form fields', async () => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
fields: [mockField],
},
mountFn: mountExtended,
});
mockFormFunctions({ checkValidityReturn: false });
describe('when form is valid', () => {
const mockTestPath = '/test';
await findTestButton().vm.$emit('click', new Event('click'));
beforeEach(() => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
testPath: mockTestPath,
},
mountFn: mountExtended,
});
jest.spyOn(findGlForm().element, 'checkValidity').mockReturnValue(true);
});
expect(findDynamicField().props('isValidated')).toBe(true);
});
describe('buttons', () => {
beforeEach(async () => {
await findTestButton().vm.$emit('click', new Event('click'));
});
describe('when form is valid', () => {
const mockTestPath = '/test';
it('sets test button `loading` prop to `true`', () => {
expect(findTestButton().props('loading')).toBe(true);
});
beforeEach(() => {
createComponent({
customStateProps: {
showActive: true,
canTest: true,
testPath: mockTestPath,
},
mountFn: mountExtended,
});
mockFormFunctions({ checkValidityReturn: true });
it('sets save button `disabled` prop to `true`', () => {
expect(findProjectSaveButton().props('disabled')).toBe(true);
});
});
describe.each`
scenario | replyStatus | errorMessage | expectToast | expectSentry
${'when "test settings" request fails'} | ${httpStatus.INTERNAL_SERVER_ERROR} | ${undefined} | ${I18N_DEFAULT_ERROR_MESSAGE} | ${true}
${'when "test settings" returns an error'} | ${httpStatus.OK} | ${'an error'} | ${'an error'} | ${false}
${'when "test settings" succeeds'} | ${httpStatus.OK} | ${undefined} | ${I18N_SUCCESSFUL_CONNECTION_MESSAGE} | ${false}
`('$scenario', ({ replyStatus, errorMessage, expectToast, expectSentry }) => {
beforeEach(async () => {
mockAxios.onPut(mockTestPath).replyOnce(replyStatus, {
error: Boolean(errorMessage),
message: errorMessage,
});
describe('buttons', () => {
beforeEach(async () => {
await findTestButton().vm.$emit('click', new Event('click'));
});
await findTestButton().vm.$emit('click', new Event('click'));
await waitForPromises();
});
it('sets test button `loading` prop to `true`', () => {
expect(findTestButton().props('loading')).toBe(true);
});
it(`calls toast with '${expectToast}'`, () => {
expect(mockToastShow).toHaveBeenCalledWith(expectToast);
});
it('sets save button `disabled` prop to `true`', () => {
expect(findProjectSaveButton().props('disabled')).toBe(true);
});
});
it('sets `loading` prop of test button to `false`', () => {
expect(findTestButton().props('loading')).toBe(false);
});
describe.each`
scenario | replyStatus | errorMessage | expectToast | expectSentry
${'when "test settings" request fails'} | ${httpStatus.INTERNAL_SERVER_ERROR} | ${undefined} | ${I18N_DEFAULT_ERROR_MESSAGE} | ${true}
${'when "test settings" returns an error'} | ${httpStatus.OK} | ${'an error'} | ${'an error'} | ${false}
${'when "test settings" succeeds'} | ${httpStatus.OK} | ${undefined} | ${I18N_SUCCESSFUL_CONNECTION_MESSAGE} | ${false}
`('$scenario', ({ replyStatus, errorMessage, expectToast, expectSentry }) => {
beforeEach(async () => {
mockAxios.onPut(mockTestPath).replyOnce(replyStatus, {
error: Boolean(errorMessage),
message: errorMessage,
});
await findTestButton().vm.$emit('click', new Event('click'));
await waitForPromises();
});
it(`calls toast with '${expectToast}'`, () => {
expect(mockToastShow).toHaveBeenCalledWith(expectToast);
});
it('sets `loading` prop of test button to `false`', () => {
expect(findTestButton().props('loading')).toBe(false);
});
it('sets save button `disabled` prop to `false`', () => {
expect(findProjectSaveButton().props('disabled')).toBe(false);
});
it(`${expectSentry ? 'does' : 'does not'} capture exception in Sentry`, () => {
expect(Sentry.captureException).toHaveBeenCalledTimes(expectSentry ? 1 : 0);
});
});
it('sets save button `disabled` prop to `false`', () => {
expect(findProjectSaveButton().props('disabled')).toBe(false);
});
it(`${expectSentry ? 'does' : 'does not'} capture exception in Sentry`, () => {
expect(Sentry.captureException).toHaveBeenCalledTimes(expectSentry ? 1 : 0);
});
});
},
);
});
});
describe('when `reset-confirmation-modal` emits `reset` event', () => {
const mockResetPath = '/reset';
......
spec/views/projects/services/_form.haml_spec.rb deleted 100644 → 0
View file @ 6cfa36b0
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'projects/services/_form' do
let(:project) { create(:redmine_project) }
let(:user) { create(:admin) }
before do
assign(:project, project)
allow(controller).to receive(:current_user).and_return(user)
allow(view).to receive_messages(
current_user: user,
can?: true,
current_application_settings: Gitlab::CurrentSettings.current_application_settings,
integration: project.redmine_integration,
request: double(referer: '/services')
)
end
context 'integrations form' do
it 'does not render form element' do
render
expect(rendered).not_to have_selector('[data-testid="integration-form"]')
end
context 'when vue_integration_form feature flag is disabled' do
before do
stub_feature_flags(vue_integration_form: false)
end
it 'renders form element' do
render
expect(rendered).to have_selector('[data-testid="integration-form"]')
end
context 'commit_events and merge_request_events' do
it 'display merge_request_events and commit_events descriptions' do
allow(Integrations::Redmine).to receive(:supported_events).and_return(%w(commit merge_request))
render
expect(rendered).to have_css("input[name='redirect_to'][value='/services']", count: 1, visible: false)
end
end
end
end
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7