Skip to content

G
gitlab-ce
Commit f7097cf1 authored by Andrew Fontaine's avatar Andrew Fontaine
Browse files
Options

Remove Legacy Flags from Feature Flag Table 

This removes all the version checking and code required to display
legacy feature flags, as they are no longer available.

Changelog: removed
parent 5ea97472
Hide whitespace changes
Inline Side-by-side
Showing with 121 additions and 236 deletions
app/assets/javascripts/feature_flags/components/feature_flags_table.vue
View file @ f7097cf1
<script>
import { GlBadge, GlButton, GlTooltipDirective, GlModal, GlToggle, GlIcon } from '@gitlab/ui';
import { GlBadge, GlButton, GlTooltipDirective, GlModal, GlToggle } from '@gitlab/ui';
import { __, s__, sprintf } from '~/locale';
import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { ROLLOUT_STRATEGY_PERCENT_ROLLOUT, NEW_VERSION_FLAG, LEGACY_FLAG } from '../constants';
import { labelForStrategy } from '../utils';
export default {
......@@ -14,7 +13,6 @@ export default {
components: {
GlBadge,
GlButton,
GlIcon,
GlModal,
GlToggle,
},
......@@ -35,13 +33,7 @@ export default {
deleteFeatureFlagName: null,
};
},
translations: {
legacyFlagReadOnlyAlert: s__('FeatureFlags|Flag is read-only'),
},
computed: {
permissions() {
return this.glFeatures.featureFlagPermissions;
},
modalTitle() {
return sprintf(s__('FeatureFlags|Delete %{name}?'), {
name: this.deleteFeatureFlagName,
......@@ -57,12 +49,6 @@ export default {
},
},
methods: {
isLegacyFlag(flag) {
return flag.version !== NEW_VERSION_FLAG;
},
statusToggleDisabled(flag) {
return flag.version === LEGACY_FLAG;
},
scopeTooltipText(scope) {
return !scope.active
? sprintf(s__('FeatureFlags|Inactive flag for %{scope}'), {
......@@ -70,22 +56,6 @@ export default {
})
: '';
},
badgeText(scope) {
const displayName =
scope.environmentScope === '*'
? s__('FeatureFlags|* (All environments)')
: scope.environmentScope;
const displayPercentage =
scope.rolloutStrategy === ROLLOUT_STRATEGY_PERCENT_ROLLOUT
? `: ${scope.rolloutPercentage}%`
: '';
return `${displayName}${displayPercentage}`;
},
badgeVariant(scope) {
return scope.active ? 'info' : 'muted';
},
strategyBadgeText(strategy) {
return labelForStrategy(strategy);
},
......@@ -142,7 +112,6 @@ export default {
<gl-toggle
v-if="featureFlag.update_path"
:value="featureFlag.active"
:disabled="statusToggleDisabled(featureFlag)"
:label="$options.i18n.toggleLabel"
label-position="hidden"
data-testid="feature-flag-status-toggle"
......@@ -169,12 +138,6 @@ export default {
<div class="feature-flag-name text-monospace text-truncate">
{{ featureFlag.name }}
</div>
<gl-icon
v-if="isLegacyFlag(featureFlag)"
v-gl-tooltip.hover="$options.translations.legacyFlagReadOnlyAlert"
class="gl-ml-3"
name="information-o"
/>
</div>
<div class="feature-flag-description text-secondary text-truncate">
{{ featureFlag.description }}
......@@ -189,27 +152,14 @@ export default {
<div
class="table-mobile-content d-flex flex-wrap justify-content-end justify-content-md-start js-feature-flag-environments"
>
<template v-if="isLegacyFlag(featureFlag)">
<gl-badge
v-for="scope in featureFlag.scopes"
:key="scope.id"
v-gl-tooltip.hover="scopeTooltipText(scope)"
:variant="badgeVariant(scope)"
:data-qa-selector="`feature-flag-scope-${badgeVariant(scope)}-badge`"
class="gl-mr-3 gl-mt-2"
>{{ badgeText(scope) }}</gl-badge
>
</template>
<template v-else>
<gl-badge
v-for="strategy in featureFlag.strategies"
:key="strategy.id"
data-testid="strategy-badge"
variant="info"
class="gl-mr-3 gl-mt-2 gl-white-space-normal gl-text-left gl-px-5"
>{{ strategyBadgeText(strategy) }}</gl-badge
>
</template>
<gl-badge
v-for="strategy in featureFlag.strategies"
:key="strategy.id"
data-testid="strategy-badge"
variant="info"
class="gl-mr-3 gl-mt-2 gl-white-space-normal gl-text-left gl-px-5"
>{{ strategyBadgeText(strategy) }}</gl-badge
>
</div>
</div>
......
app/assets/javascripts/feature_flags/store/index/mutations.js
View file @ f7097cf1
import Vue from 'vue';
import { parseIntPagination, normalizeHeaders } from '~/lib/utils/common_utils';
import { mapToScopesViewModel } from '../helpers';
import * as types from './mutation_types';
const mapFlag = (flag) => ({ ...flag, scopes: mapToScopesViewModel(flag.scopes || []) });
const updateFlag = (state, flag) => {
const index = state.featureFlags.findIndex(({ id }) => id === flag.id);
Vue.set(state.featureFlags, index, flag);
......@@ -31,7 +28,7 @@ export default {
[types.RECEIVE_FEATURE_FLAGS_SUCCESS](state, response) {
state.isLoading = false;
state.hasError = false;
state.featureFlags = (response.data.feature_flags || []).map(mapFlag);
state.featureFlags = response.data.feature_flags || [];
const paginationInfo = createPaginationInfo(response.headers);
state.count = paginationInfo?.total ?? state.featureFlags.length;
......@@ -58,7 +55,7 @@ export default {
updateFlag(state, flag);
},
[types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](state, data) {
updateFlag(state, mapFlag(data));
updateFlag(state, data);
},
[types.RECEIVE_UPDATE_FEATURE_FLAG_ERROR](state, i) {
const flag = state.featureFlags.find(({ id }) => i === id);
......
locale/gitlab.pot
View file @ f7097cf1
......@@ -13763,9 +13763,6 @@ msgstr ""
msgid "FeatureFlags|* (All Environments)"
msgstr ""
msgid "FeatureFlags|* (All environments)"
msgstr ""
msgid "FeatureFlags|API URL"
msgstr ""
......@@ -13847,9 +13844,6 @@ msgstr ""
msgid "FeatureFlags|Feature flags limit reached (%{featureFlagsLimit}). Delete one or more feature flags before adding new ones."
msgstr ""
msgid "FeatureFlags|Flag is read-only"
msgstr ""
msgid "FeatureFlags|Get started with feature flags"
msgstr ""
......
spec/features/projects/feature_flags/user_sees_feature_flag_list_spec.rb
View file @ f7097cf1
......@@ -16,33 +16,63 @@ RSpec.describe 'User sees feature flag list', :js do
sign_in(user)
end
context 'with legacy feature flags' do
context 'with feature flags' do
before do
create_flag(project, 'ci_live_trace', false, version: :legacy_flag).tap do |feature_flag|
create_scope(feature_flag, 'review/*', true)
create_flag(project, 'ci_live_trace', false).tap do |feature_flag|
create_strategy(feature_flag).tap do |strat|
create(:operations_scope, strategy: strat, environment_scope: '*')
create(:operations_scope, strategy: strat, environment_scope: 'review/*')
end
end
create_flag(project, 'drop_legacy_artifacts', false, version: :legacy_flag)
create_flag(project, 'mr_train', true, version: :legacy_flag).tap do |feature_flag|
create_scope(feature_flag, 'production', false)
create_flag(project, 'drop_legacy_artifacts', false)
create_flag(project, 'mr_train', true).tap do |feature_flag|
create_strategy(feature_flag).tap do |strat|
create(:operations_scope, strategy: strat, environment_scope: 'production')
end
end
create(:operations_feature_flag, :new_version_flag, project: project,
name: 'my_flag', active: false)
end
it 'shows empty page' do
it 'shows the user the first flag' do
visit(project_feature_flags_path(project))
expect(page).to have_text 'Get started with feature flags'
expect(page).to have_selector('.btn-confirm', text: 'New feature flag')
expect(page).to have_selector('[data-qa-selector="configure_feature_flags_button"]', text: 'Configure')
within_feature_flag_row(1) do
expect(page.find('.js-feature-flag-id')).to have_content('^1')
expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
expect_status_toggle_button_not_to_be_checked
within_feature_flag_scopes do
expect(page.find('[data-testid="strategy-badge"]')).to have_content('All Users: All Environments, review/*')
end
end
end
end
context 'with new version flags' do
before do
create(:operations_feature_flag, :new_version_flag, project: project,
name: 'my_flag', active: false)
it 'shows the user the second flag' do
visit(project_feature_flags_path(project))
within_feature_flag_row(2) do
expect(page.find('.js-feature-flag-id')).to have_content('^2')
expect(page.find('.feature-flag-name')).to have_content('drop_legacy_artifacts')
expect_status_toggle_button_not_to_be_checked
end
end
it 'shows the user the third flag' do
visit(project_feature_flags_path(project))
within_feature_flag_row(3) do
expect(page.find('.js-feature-flag-id')).to have_content('^3')
expect(page.find('.feature-flag-name')).to have_content('mr_train')
expect_status_toggle_button_to_be_checked
within_feature_flag_scopes do
expect(page.find('[data-testid="strategy-badge"]')).to have_content('All Users: production')
end
end
end
it 'user updates the status toggle' do
it 'allows the user to update the status toggle' do
visit(project_feature_flags_path(project))
within_feature_flag_row(1) do
......@@ -58,7 +88,7 @@ RSpec.describe 'User sees feature flag list', :js do
visit(project_feature_flags_path(project))
end
it 'shows empty page' do
it 'shows the empty page' do
expect(page).to have_text 'Get started with feature flags'
expect(page).to have_selector('.btn-confirm', text: 'New feature flag')
expect(page).to have_selector('[data-qa-selector="configure_feature_flags_button"]', text: 'Configure')
......
spec/frontend/feature_flags/components/feature_flags_table_spec.js
View file @ f7097cf1
......@@ -8,9 +8,6 @@ import {
ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
ROLLOUT_STRATEGY_USER_ID,
ROLLOUT_STRATEGY_GITLAB_USER_LIST,
NEW_VERSION_FLAG,
LEGACY_FLAG,
DEFAULT_PERCENT_ROLLOUT,
} from '~/feature_flags/constants';
const getDefaultProps = () => ({
......@@ -23,17 +20,28 @@ const getDefaultProps = () => ({
description: 'flag description',
destroy_path: 'destroy/path',
edit_path: 'edit/path',
version: LEGACY_FLAG,
scopes: [
scopes: [],
strategies: [
{
id: 1,
active: true,
environmentScope: 'scope',
canUpdate: true,
protected: false,
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
shouldBeDestroyed: false,
name: ROLLOUT_STRATEGY_ALL_USERS,
parameters: {},
scopes: [{ environment_scope: '*' }],
},
{
name: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
parameters: { percentage: '50' },
scopes: [{ environment_scope: 'production' }, { environment_scope: 'staging' }],
},
{
name: ROLLOUT_STRATEGY_USER_ID,
parameters: { userIds: '1,2,3,4' },
scopes: [{ environment_scope: 'review/*' }],
},
{
name: ROLLOUT_STRATEGY_GITLAB_USER_LIST,
parameters: {},
user_list: { name: 'test list' },
scopes: [{ environment_scope: '*' }],
},
],
},
......@@ -43,6 +51,7 @@ const getDefaultProps = () => ({
describe('Feature flag table', () => {
let wrapper;
let props;
let badges;
const createWrapper = (propsData, opts = {}) => {
wrapper = shallowMount(FeatureFlagsTable, {
......@@ -54,6 +63,15 @@ describe('Feature flag table', () => {
});
};
beforeEach(() => {
props = getDefaultProps();
createWrapper(props, {
provide: { csrfToken: 'fakeToken' },
});
badges = wrapper.findAll('[data-testid="strategy-badge"]');
});
beforeEach(() => {
props = getDefaultProps();
});
......@@ -97,17 +115,10 @@ describe('Feature flag table', () => {
);
});
it('should render an environments specs column', () => {
const envColumn = wrapper.find('.js-feature-flag-environments');
expect(envColumn).toBeDefined();
expect(trimText(envColumn.text())).toBe('scope');
});
it('should render an environments specs badge with active class', () => {
const envColumn = wrapper.find('.js-feature-flag-environments');
expect(trimText(envColumn.find(GlBadge).text())).toBe('scope');
expect(trimText(envColumn.find(GlBadge).text())).toBe('All Users: All Environments');
});
it('should render an actions column', () => {
......@@ -120,11 +131,13 @@ describe('Feature flag table', () => {
describe('when active and with an update toggle', () => {
let toggle;
let spy;
beforeEach(() => {
props.featureFlags[0].update_path = props.featureFlags[0].destroy_path;
createWrapper(props);
toggle = wrapper.find(GlToggle);
spy = mockTracking('_category_', toggle.element, jest.spyOn);
});
it('should have a toggle', () => {
......@@ -143,123 +156,40 @@ describe('Feature flag table', () => {
expect(wrapper.emitted('toggle-flag')).toEqual([[flag]]);
});
});
});
describe('with an active scope and a percentage rollout strategy', () => {
beforeEach(() => {
props.featureFlags[0].scopes[0].rolloutStrategy = ROLLOUT_STRATEGY_PERCENT_ROLLOUT;
props.featureFlags[0].scopes[0].rolloutPercentage = '54';
createWrapper(props);
});
it('should render an environments specs badge with percentage', () => {
const envColumn = wrapper.find('.js-feature-flag-environments');
it('tracks a click', () => {
toggle.trigger('click');
expect(trimText(envColumn.find(GlBadge).text())).toBe('scope: 54%');
expect(spy).toHaveBeenCalledWith('_category_', 'click_button', {
label: 'feature_flag_toggle',
});
});
});
describe('with an inactive scope', () => {
beforeEach(() => {
props.featureFlags[0].scopes[0].active = false;
createWrapper(props);
});
it('should render an environments specs badge with inactive class', () => {
const envColumn = wrapper.find('.js-feature-flag-environments');
expect(trimText(envColumn.find(GlBadge).text())).toBe('scope');
});
it('shows All Environments if the environment scope is *', () => {
expect(badges.at(0).text()).toContain('All Environments');
});
describe('with a new version flag', () => {
let toggle;
let spy;
let badges;
beforeEach(() => {
const newVersionProps = {
...props,
featureFlags: [
{
id: 1,
iid: 1,
active: true,
name: 'flag name',
description: 'flag description',
destroy_path: 'destroy/path',
edit_path: 'edit/path',
update_path: 'update/path',
version: NEW_VERSION_FLAG,
scopes: [],
strategies: [
{
name: ROLLOUT_STRATEGY_ALL_USERS,
parameters: {},
scopes: [{ environment_scope: '*' }],
},
{
name: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
parameters: { percentage: '50' },
scopes: [{ environment_scope: 'production' }, { environment_scope: 'staging' }],
},
{
name: ROLLOUT_STRATEGY_USER_ID,
parameters: { userIds: '1,2,3,4' },
scopes: [{ environment_scope: 'review/*' }],
},
{
name: ROLLOUT_STRATEGY_GITLAB_USER_LIST,
parameters: {},
user_list: { name: 'test list' },
scopes: [{ environment_scope: '*' }],
},
],
},
],
};
createWrapper(newVersionProps, {
provide: { csrfToken: 'fakeToken', glFeatures: { featureFlagsNewVersion: true } },
});
toggle = wrapper.find(GlToggle);
spy = mockTracking('_category_', toggle.element, jest.spyOn);
badges = wrapper.findAll('[data-testid="strategy-badge"]');
});
it('shows All Environments if the environment scope is *', () => {
expect(badges.at(0).text()).toContain('All Environments');
});
it('shows the environment scope if another is set', () => {
expect(badges.at(1).text()).toContain('production');
expect(badges.at(1).text()).toContain('staging');
expect(badges.at(2).text()).toContain('review/*');
});
it('shows All Users for the default strategy', () => {
expect(badges.at(0).text()).toContain('All Users');
});
it('shows the percent for a percent rollout', () => {
expect(badges.at(1).text()).toContain('Percent of users - 50%');
});
it('shows the environment scope if another is set', () => {
expect(badges.at(1).text()).toContain('production');
expect(badges.at(1).text()).toContain('staging');
expect(badges.at(2).text()).toContain('review/*');
});
it('shows the number of users for users with ID', () => {
expect(badges.at(2).text()).toContain('User IDs - 4 users');
});
it('shows All Users for the default strategy', () => {
expect(badges.at(0).text()).toContain('All Users');
});
it('shows the name of a user list for user list', () => {
expect(badges.at(3).text()).toContain('User List - test list');
});
it('shows the percent for a percent rollout', () => {
expect(badges.at(1).text()).toContain('Percent of users - 50%');
});
it('tracks a click', () => {
toggle.trigger('click');
it('shows the number of users for users with ID', () => {
expect(badges.at(2).text()).toContain('User IDs - 4 users');
});
expect(spy).toHaveBeenCalledWith('_category_', 'click_button', {
label: 'feature_flag_toggle',
});
});
it('shows the name of a user list for user list', () => {
expect(badges.at(3).text()).toContain('User List - test list');
});
it('renders a feature flag without an iid', () => {
......
spec/frontend/feature_flags/store/index/actions_spec.js
View file @ f7097cf1
import MockAdapter from 'axios-mock-adapter';
import testAction from 'helpers/vuex_action_helper';
import { TEST_HOST } from 'spec/test_constants';
import { mapToScopesViewModel } from '~/feature_flags/store/helpers';
import {
requestFeatureFlags,
receiveFeatureFlagsSuccess,
......@@ -255,7 +254,6 @@ describe('Feature flags actions', () => {
beforeEach(() => {
mockedState.featureFlags = getRequestData.feature_flags.map((flag) => ({
...flag,
scopes: mapToScopesViewModel(flag.scopes || []),
}));
mock = new MockAdapter(axios);
});
......@@ -314,7 +312,6 @@ describe('Feature flags actions', () => {
beforeEach(() => {
mockedState.featureFlags = getRequestData.feature_flags.map((f) => ({
...f,
scopes: mapToScopesViewModel(f.scopes || []),
}));
});
......@@ -338,7 +335,6 @@ describe('Feature flags actions', () => {
beforeEach(() => {
mockedState.featureFlags = getRequestData.feature_flags.map((f) => ({
...f,
scopes: mapToScopesViewModel(f.scopes || []),
}));
});
......@@ -362,7 +358,6 @@ describe('Feature flags actions', () => {
beforeEach(() => {
mockedState.featureFlags = getRequestData.feature_flags.map((f) => ({
...f,
scopes: mapToScopesViewModel(f.scopes || []),
}));
});
......
spec/frontend/feature_flags/store/index/mutations_spec.js
View file @ f7097cf1
import { mapToScopesViewModel } from '~/feature_flags/store/helpers';
import * as types from '~/feature_flags/store/index/mutation_types';
import mutations from '~/feature_flags/store/index/mutations';
import state from '~/feature_flags/store/index/state';
......@@ -49,15 +48,6 @@ describe('Feature flags store Mutations', () => {
expect(stateCopy.hasError).toEqual(false);
});
it('should set featureFlags with the transformed data', () => {
const expected = getRequestData.feature_flags.map((flag) => ({
...flag,
scopes: mapToScopesViewModel(flag.scopes || []),
}));
expect(stateCopy.featureFlags).toEqual(expected);
});
it('should set count with the given data', () => {
expect(stateCopy.count).toEqual(37);
});
......@@ -131,13 +121,11 @@ describe('Feature flags store Mutations', () => {
beforeEach(() => {
stateCopy.featureFlags = getRequestData.feature_flags.map((flag) => ({
...flag,
scopes: mapToScopesViewModel(flag.scopes || []),
}));
stateCopy.count = { featureFlags: 1, userLists: 0 };
mutations[types.UPDATE_FEATURE_FLAG](stateCopy, {
...featureFlag,
scopes: mapToScopesViewModel(featureFlag.scopes || []),
active: false,
});
});
......@@ -146,7 +134,6 @@ describe('Feature flags store Mutations', () => {
expect(stateCopy.featureFlags).toEqual([
{
...featureFlag,
scopes: mapToScopesViewModel(featureFlag.scopes || []),
active: false,
},
]);
......@@ -158,7 +145,6 @@ describe('Feature flags store Mutations', () => {
stateCopy.featureFlags = getRequestData.feature_flags.map((flag) => ({
...flag,
...flagState,
scopes: mapToScopesViewModel(flag.scopes || []),
}));
stateCopy.count = stateCount;
......@@ -174,7 +160,6 @@ describe('Feature flags store Mutations', () => {
expect(stateCopy.featureFlags).toEqual([
{
...featureFlag,
scopes: mapToScopesViewModel(featureFlag.scopes || []),
active: false,
},
]);
......@@ -185,7 +170,6 @@ describe('Feature flags store Mutations', () => {
beforeEach(() => {
stateCopy.featureFlags = getRequestData.feature_flags.map((flag) => ({
...flag,
scopes: mapToScopesViewModel(flag.scopes || []),
}));
mutations[types.RECEIVE_UPDATE_FEATURE_FLAG_ERROR](stateCopy, featureFlag.id);
});
......@@ -194,7 +178,6 @@ describe('Feature flags store Mutations', () => {
expect(stateCopy.featureFlags).toEqual([
{
...featureFlag,
scopes: mapToScopesViewModel(featureFlag.scopes || []),
active: false,
},
]);
......
spec/support/helpers/feature_flag_helpers.rb
View file @ f7097cf1
......@@ -14,6 +14,12 @@ module FeatureFlagHelpers
strategies: strategies)
end
def create_strategy(feature_flag, name = 'default', parameters = {})
create(:operations_strategy,
feature_flag: feature_flag,
name: name)
end
def within_feature_flag_row(index)
within ".gl-responsive-table-row:nth-child(#{index + 1})" do
yield
......
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