Commit 83865fed authored by Vitaly Slobodin's avatar Vitaly Slobodin

Merge branch '235114-remove-approvals-feature-flag' into 'master'

Remove approvalSuggestions feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54271
parents c4d97e34 70783954
...@@ -36,7 +36,6 @@ class ProjectsController < Projects::ApplicationController ...@@ -36,7 +36,6 @@ class ProjectsController < Projects::ApplicationController
end end
before_action only: [:edit] do before_action only: [:edit] do
push_frontend_feature_flag(:approval_suggestions, @project, default_enabled: true)
push_frontend_feature_flag(:allow_editing_commit_messages, @project) push_frontend_feature_flag(:allow_editing_commit_messages, @project)
end end
......
---
name: approval_suggestions
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38992
rollout_issue_url:
milestone: '13.3'
type: development
group: group::composition analysis
default_enabled: true
...@@ -147,7 +147,7 @@ export default { ...@@ -147,7 +147,7 @@ export default {
</template> </template>
</template> </template>
</rules> </rules>
<!-- TODO: Remove feature flag in https://gitlab.com/gitlab-org/gitlab/-/issues/235114 -->
<unconfigured-security-rules v-if="glFeatures.approvalSuggestions" /> <unconfigured-security-rules />
</div> </div>
</template> </template>
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
import { groupBy, isNumber } from 'lodash'; import { groupBy, isNumber } from 'lodash';
import { mapState, mapActions } from 'vuex'; import { mapState, mapActions } from 'vuex';
import { sprintf, __ } from '~/locale'; import { sprintf, __ } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { TYPE_USER, TYPE_GROUP, TYPE_HIDDEN_GROUPS } from '../constants'; import { TYPE_USER, TYPE_GROUP, TYPE_HIDDEN_GROUPS } from '../constants';
import ApproversList from './approvers_list.vue'; import ApproversList from './approvers_list.vue';
import ApproversSelect from './approvers_select.vue'; import ApproversSelect from './approvers_select.vue';
...@@ -23,8 +22,6 @@ export default { ...@@ -23,8 +22,6 @@ export default {
ApproversSelect, ApproversSelect,
BranchesSelect, BranchesSelect,
}, },
// TODO: Remove feature flag in https://gitlab.com/gitlab-org/gitlab/-/issues/235114
mixins: [glFeatureFlagsMixin()],
props: { props: {
initRule: { initRule: {
type: Object, type: Object,
...@@ -44,7 +41,7 @@ export default { ...@@ -44,7 +41,7 @@ export default {
}, },
data() { data() {
const defaults = { const defaults = {
name: '', name: this.defaultRuleName,
approvalsRequired: 1, approvalsRequired: 1,
minApprovalsRequired: 0, minApprovalsRequired: 0,
approvers: [], approvers: [],
...@@ -57,10 +54,6 @@ export default { ...@@ -57,10 +54,6 @@ export default {
serverValidationErrors: [], serverValidationErrors: [],
...this.getInitialData(), ...this.getInitialData(),
}; };
// TODO: Remove feature flag in https://gitlab.com/gitlab-org/gitlab/-/issues/235114
if (this.glFeatures.approvalSuggestions) {
return { ...defaults, name: this.defaultRuleName || defaults.name };
}
return defaults; return defaults;
}, },
...@@ -162,13 +155,9 @@ export default { ...@@ -162,13 +155,9 @@ export default {
return !this.settings.lockedApprovalsRuleName; return !this.settings.lockedApprovalsRuleName;
}, },
isNameDisabled() { isNameDisabled() {
// TODO: Remove feature flag in https://gitlab.com/gitlab-org/gitlab/-/issues/235114 return (
if (this.glFeatures.approvalSuggestions) { Boolean(this.isPersisted || this.defaultRuleName) && READONLY_NAMES.includes(this.name)
return ( );
Boolean(this.isPersisted || this.defaultRuleName) && READONLY_NAMES.includes(this.name)
);
}
return this.isPersisted && READONLY_NAMES.includes(this.name);
}, },
removeHiddenGroups() { removeHiddenGroups() {
return this.containsHiddenGroups && !this.approversByType[TYPE_HIDDEN_GROUPS]; return this.containsHiddenGroups && !this.approversByType[TYPE_HIDDEN_GROUPS];
......
...@@ -100,15 +100,11 @@ describe('Approvals ModalRuleCreate', () => { ...@@ -100,15 +100,11 @@ describe('Approvals ModalRuleCreate', () => {
}); });
}); });
describe('with approvalSuggestions feature flag', () => { describe('with approval suggestions', () => {
beforeEach(() => { beforeEach(() => {
createModalState.data = { ...TEST_RULE, defaultRuleName: 'Vulnerability-Check' }; createModalState.data = { ...TEST_RULE, defaultRuleName: 'Vulnerability-Check' };
factory({ factory();
provide: {
glFeatures: { approvalSuggestions: true },
},
});
modal = findModal(); modal = findModal();
form = findForm(); form = findForm();
}); });
......
...@@ -3,6 +3,7 @@ import Vuex from 'vuex'; ...@@ -3,6 +3,7 @@ import Vuex from 'vuex';
import RuleInput from 'ee/approvals/components/mr_edit/rule_input.vue'; import RuleInput from 'ee/approvals/components/mr_edit/rule_input.vue';
import ProjectRules from 'ee/approvals/components/project_settings/project_rules.vue'; import ProjectRules from 'ee/approvals/components/project_settings/project_rules.vue';
import RuleName from 'ee/approvals/components/rule_name.vue'; import RuleName from 'ee/approvals/components/rule_name.vue';
import Rules from 'ee/approvals/components/rules.vue';
import UnconfiguredSecurityRules from 'ee/approvals/components/security_configuration/unconfigured_security_rules.vue'; import UnconfiguredSecurityRules from 'ee/approvals/components/security_configuration/unconfigured_security_rules.vue';
import { createStoreOptions } from 'ee/approvals/stores'; import { createStoreOptions } from 'ee/approvals/stores';
import projectSettingsModule from 'ee/approvals/stores/modules/project_settings'; import projectSettingsModule from 'ee/approvals/stores/modules/project_settings';
...@@ -57,7 +58,10 @@ describe('Approvals ProjectRules', () => { ...@@ -57,7 +58,10 @@ describe('Approvals ProjectRules', () => {
it('renders row for each rule', () => { it('renders row for each rule', () => {
factory(); factory();
const rows = wrapper.findAll('tbody tr').filter((tr, index) => index !== 0); const rows = wrapper
.findComponent(Rules)
.findAll('tbody tr')
.filter((tr, index) => index !== 0);
const data = rows.wrappers.map(getRowData); const data = rows.wrappers.map(getRowData);
expect(data).toEqual( expect(data).toEqual(
...@@ -68,7 +72,7 @@ describe('Approvals ProjectRules', () => { ...@@ -68,7 +72,7 @@ describe('Approvals ProjectRules', () => {
})), })),
); );
expect(wrapper.findAll(RuleName).length).toBe(rows.length); expect(wrapper.findComponent(Rules).findAll(RuleName).length).toBe(rows.length);
}); });
it('should always have any_approver rule', () => { it('should always have any_approver rule', () => {
...@@ -91,12 +95,12 @@ describe('Approvals ProjectRules', () => { ...@@ -91,12 +95,12 @@ describe('Approvals ProjectRules', () => {
factory(); factory();
row = wrapper.find('tbody tr'); row = wrapper.findComponent(Rules).find('tbody tr');
}); });
it('does not render name', () => { it('does not render name', () => {
expect(findCell(row, 'name').exists()).toBe(false); expect(findCell(row, 'name').exists()).toBe(false);
expect(wrapper.find(RuleName).exists()).toBe(false); expect(wrapper.findComponent(Rules).find(RuleName).exists()).toBe(false);
}); });
it('should only display 1 rule', () => { it('should only display 1 rule', () => {
...@@ -116,7 +120,7 @@ describe('Approvals ProjectRules', () => { ...@@ -116,7 +120,7 @@ describe('Approvals ProjectRules', () => {
beforeEach(() => { beforeEach(() => {
factory(); factory();
rows = wrapper.findAll('tbody tr'); rows = wrapper.find(Rules).findAll('tbody tr');
}); });
it('should not render the popover for a standard approval group', () => { it('should not render the popover for a standard approval group', () => {
...@@ -126,35 +130,23 @@ describe('Approvals ProjectRules', () => { ...@@ -126,35 +130,23 @@ describe('Approvals ProjectRules', () => {
expect(nameCell.find('.js-help').exists()).toBeFalsy(); expect(nameCell.find('.js-help').exists()).toBeFalsy();
}); });
it('should not render the unconfigured-security-rules component', () => { it('should render the unconfigured-security-rules component', () => {
expect(wrapper.find(UnconfiguredSecurityRules).exists()).toBe(false); expect(wrapper.find(UnconfiguredSecurityRules).exists()).toBe(true);
}); });
}); });
describe.each([true, false])( describe('approval suggestions', () => {
'when the approvalSuggestions feature flag is %p', beforeEach(() => {
(approvalSuggestions) => { const rules = createProjectRules();
beforeEach(() => { rules[0].name = 'Vulnerability-Check';
const rules = createProjectRules(); store.modules.approvals.state.rules = rules;
rules[0].name = 'Vulnerability-Check'; store.state.settings.allowMultiRule = true;
store.modules.approvals.state.rules = rules;
store.state.settings.allowMultiRule = true; factory();
});
factory(
{}, it(`should render the unconfigured-security-rules component`, () => {
{ expect(wrapper.find(UnconfiguredSecurityRules).exists()).toBe(true);
provide: { });
glFeatures: { approvalSuggestions }, });
},
},
);
});
it(`should ${
approvalSuggestions ? '' : 'not'
} render the unconfigured-security-rules component`, () => {
expect(wrapper.find(UnconfiguredSecurityRules).exists()).toBe(approvalSuggestions);
});
},
);
}); });
...@@ -534,7 +534,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -534,7 +534,7 @@ describe('EE Approvals RuleForm', () => {
}); });
}); });
describe('with approvalSuggestions enabled', () => { describe('with approval suggestions', () => {
describe.each` describe.each`
defaultRuleName | expectedDisabledAttribute defaultRuleName | expectedDisabledAttribute
${'Vulnerability-Check'} | ${'disabled'} ${'Vulnerability-Check'} | ${'disabled'}
...@@ -544,17 +544,10 @@ describe('EE Approvals RuleForm', () => { ...@@ -544,17 +544,10 @@ describe('EE Approvals RuleForm', () => {
'with defaultRuleName set to $defaultRuleName', 'with defaultRuleName set to $defaultRuleName',
({ defaultRuleName, expectedDisabledAttribute }) => { ({ defaultRuleName, expectedDisabledAttribute }) => {
beforeEach(() => { beforeEach(() => {
createComponent( createComponent({
{ initRule: null,
initRule: null, defaultRuleName,
defaultRuleName, });
},
{
provide: {
glFeatures: { approvalSuggestions: true },
},
},
);
}); });
it(`it ${ it(`it ${
......
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