Commit cd89e30c authored by Andrew Fontaine's avatar Andrew Fontaine

Rework Users with ID for Feature Flags

Now defines list of feature flags per scope, instead of per flag.

This allows different scopes to enable feature flags for different
users!
parent 123bcddf
---
title: Make User IDs work per scope in Feature Flags
merge_request: 19399
author:
type: added
<script>
import Vue from 'vue';
import _ from 'underscore';
import { GlButton, GlBadge, GlTooltip, GlTooltipDirective } from '@gitlab/ui';
import {
GlButton,
GlBadge,
GlTooltip,
GlTooltipDirective,
GlFormTextarea,
GlFormCheckbox,
} from '@gitlab/ui';
import { s__, sprintf } from '~/locale';
import featureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import ToggleButton from '~/vue_shared/components/toggle_button.vue';
......@@ -10,6 +17,7 @@ import EnvironmentsDropdown from './environments_dropdown.vue';
import {
ROLLOUT_STRATEGY_ALL_USERS,
ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
ROLLOUT_STRATEGY_USER_ID,
ALL_ENVIRONMENTS_NAME,
INTERNAL_ID_PREFIX,
} from '../constants';
......@@ -20,6 +28,8 @@ export default {
components: {
GlButton,
GlBadge,
GlFormCheckbox,
GlFormTextarea,
GlTooltip,
ToggleButton,
Icon,
......@@ -77,6 +87,7 @@ export default {
ROLLOUT_STRATEGY_ALL_USERS,
ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
ROLLOUT_STRATEGY_USER_ID,
// Matches numbers 0 through 100
rolloutPercentageRegex: /^[0-9]$|^[1-9][0-9]$|^100$/,
......@@ -102,11 +113,6 @@ export default {
permissionsFlag() {
return this.glFeatures.featureFlagPermissions;
},
userIds() {
const scope = this.formScopes.find(s => Array.isArray(s.rolloutUserIds)) || {};
return scope.rolloutUserIds || [];
},
},
methods: {
isAllEnvironment(name) {
......@@ -154,18 +160,9 @@ export default {
scopes: this.formScopes,
});
},
updateUserIds(userIds) {
this.formScopes = this.formScopes.map(s => ({
...s,
rolloutUserIds: userIds,
}));
},
canUpdateScope(scope) {
return !this.permissionsFlag || scope.canUpdate;
},
isRolloutPercentageInvalid: _.memoize(function isRolloutPercentageInvalid(percentage) {
return !this.$options.rolloutPercentageRegex.test(percentage);
}),
......@@ -187,6 +184,17 @@ export default {
rolloutPercentageId(index) {
return `rollout-percentage-${index}`;
},
rolloutUserId(index) {
return `rollout-user-id-${index}`;
},
shouldDisplayIncludeUserIds(scope) {
return ![ROLLOUT_STRATEGY_ALL_USERS, ROLLOUT_STRATEGY_USER_ID].includes(
scope.rolloutStrategy,
);
},
shouldDisplayUserIds(scope) {
return scope.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID || scope.shouldIncludeUserIds;
},
},
};
</script>
......@@ -241,7 +249,7 @@ export default {
<div
v-for="(scope, index) in filteredScopes"
:key="scope.id"
class="gl-responsive-table-row"
class="gl-responsive-table-row align-items-start"
role="row"
>
<div class="table-section section-30" role="gridcell">
......@@ -251,7 +259,7 @@ export default {
<div
class="table-mobile-content js-feature-flag-status d-flex align-items-center justify-content-start"
>
<p v-if="isAllEnvironment(scope.environmentScope)" class="js-scope-all pl-3">
<p v-if="isAllEnvironment(scope.environmentScope)" class="js-scope-all pl-3 mb-0">
{{ $options.allEnvironmentsText }}
</p>
......@@ -285,7 +293,7 @@ export default {
</div>
</div>
<div class="table-section section-40" role="gridcell">
<div class="table-section section-40 align-items-start" role="gridcell">
<div class="table-mobile-header" role="rowheader">
{{ s__('FeatureFlags|Rollout Strategy') }}
</div>
......@@ -300,12 +308,15 @@ export default {
:disabled="!scope.active"
class="form-control select-control w-100 js-rollout-strategy"
>
<option :value="$options.ROLLOUT_STRATEGY_ALL_USERS">{{
s__('FeatureFlags|All users')
}}</option>
<option :value="$options.ROLLOUT_STRATEGY_PERCENT_ROLLOUT">{{
s__('FeatureFlags|Percent rollout (logged in users)')
}}</option>
<option :value="$options.ROLLOUT_STRATEGY_ALL_USERS">
{{ s__('FeatureFlags|All users') }}
</option>
<option :value="$options.ROLLOUT_STRATEGY_PERCENT_ROLLOUT">
{{ s__('FeatureFlags|Percent rollout (logged in users)') }}
</option>
<option :value="$options.ROLLOUT_STRATEGY_USER_ID">
{{ s__('FeatureFlags|User IDs') }}
</option>
</select>
<i aria-hidden="true" data-hidden="true" class="fa fa-chevron-down"></i>
</div>
......@@ -342,6 +353,24 @@ export default {
</gl-tooltip>
<span class="ml-1">%</span>
</div>
<div class="d-flex flex-column align-items-start mt-2 w-100">
<gl-form-checkbox
v-if="shouldDisplayIncludeUserIds(scope)"
v-model="scope.shouldIncludeUserIds"
>
{{ s__('FeatureFlags|Include additional user IDs') }}
</gl-form-checkbox>
<template v-if="shouldDisplayUserIds(scope)">
<label :for="rolloutUserId(index)" class="mb-2">
{{ s__('FeatureFlags|User IDs') }}
</label>
<gl-form-textarea
:id="rolloutUserId(index)"
v-model="scope.rolloutUserIds"
class="w-100"
/>
</template>
</div>
</div>
</div>
......@@ -413,7 +442,6 @@ export default {
</div>
</div>
</fieldset>
<user-with-id :value="userIds" @input="updateUserIds" />
<div class="form-actions">
<gl-button
......
......@@ -29,7 +29,10 @@ export const mapToScopesViewModel = scopesFromRails =>
strat => strat.name === ROLLOUT_STRATEGY_USER_ID,
);
const rolloutUserIds = (fetchUserIdParams(userStrategy) || '').split(',').filter(id => id);
const rolloutUserIds = (fetchUserIdParams(userStrategy) || '')
.split(',')
.filter(id => id)
.join(', ');
return {
id: s.id,
......@@ -43,6 +46,7 @@ export const mapToScopesViewModel = scopesFromRails =>
// eslint-disable-next-line no-underscore-dangle
shouldBeDestroyed: Boolean(s._destroy),
shouldIncludeUserIds: rolloutUserIds.length > 0,
};
});
/**
......@@ -59,8 +63,8 @@ export const mapFromScopesViewModel = params => {
}
const userIdParameters = {};
if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) {
userIdParameters.userIds = s.rolloutUserIds.join(',');
if (s.shouldIncludeUserIds || s.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID) {
userIdParameters.userIds = (s.rolloutUserIds || '').replace(/, /g, ',');
}
// Strip out any internal IDs
......@@ -113,7 +117,7 @@ export const createNewEnvironmentScope = (overrides = {}, featureFlagPermissions
id: _.uniqueId(INTERNAL_ID_PREFIX),
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [],
rolloutUserIds: '',
};
const newScope = {
......
......@@ -51,8 +51,9 @@ describe('feature flags helpers spec', () => {
protected: true,
rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
rolloutPercentage: '56',
rolloutUserIds: ['123', '234'],
rolloutUserIds: '123, 234',
shouldBeDestroyed: true,
shouldIncludeUserIds: true,
},
];
......@@ -102,7 +103,8 @@ describe('feature flags helpers spec', () => {
shouldBeDestroyed: true,
rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
rolloutPercentage: '48',
rolloutUserIds: ['123', '234'],
rolloutUserIds: '123, 234',
shouldIncludeUserIds: true,
},
],
};
......@@ -179,7 +181,7 @@ describe('feature flags helpers spec', () => {
id: expect.stringContaining(INTERNAL_ID_PREFIX),
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [],
rolloutUserIds: '',
};
const actual = createNewEnvironmentScope();
......@@ -199,7 +201,7 @@ describe('feature flags helpers spec', () => {
id: expect.stringContaining(INTERNAL_ID_PREFIX),
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [],
rolloutUserIds: '',
};
const actual = createNewEnvironmentScope(overrides);
......
import _ from 'underscore';
import { createLocalVue, mount } from '@vue/test-utils';
import { GlFormCheckbox, GlFormTextarea } from '@gitlab/ui';
import Form from 'ee/feature_flags/components/form.vue';
import ToggleButton from '~/vue_shared/components/toggle_button.vue';
import EnvironmentsDropdown from 'ee/feature_flags/components/environments_dropdown.vue';
......@@ -101,6 +102,8 @@ describe('feature flag form', () => {
protected: false,
rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
rolloutPercentage: '54',
rolloutUserIds: '123',
shouldIncludeUserIds: true,
},
{
id: 2,
......@@ -110,6 +113,8 @@ describe('feature flag form', () => {
protected: true,
rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
rolloutPercentage: '54',
rolloutUserIds: '123',
shouldIncludeUserIds: true,
},
],
});
......@@ -124,6 +129,16 @@ describe('feature flag form', () => {
expect(wrapper.find('.js-add-new-scope').exists()).toEqual(true);
});
it('renders the user id checkbox', () => {
expect(wrapper.find(GlFormCheckbox).exists()).toBe(true);
});
it('renders the user id text area', () => {
expect(wrapper.find(GlFormTextarea).exists()).toBe(true);
expect(wrapper.find(GlFormTextarea).vm.value).toBe('123');
});
describe('update scope', () => {
describe('on click on toggle', () => {
it('should update the scope', () => {
......@@ -247,7 +262,7 @@ describe('feature flag form', () => {
active: false,
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [],
rolloutUserIds: '',
},
],
});
......@@ -301,7 +316,7 @@ describe('feature flag form', () => {
protected: true,
rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
rolloutPercentage: '55',
rolloutUserIds: [],
rolloutUserIds: '',
},
{
id: jasmine.any(String),
......@@ -311,7 +326,7 @@ describe('feature flag form', () => {
protected: false,
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [],
rolloutUserIds: '',
},
{
id: jasmine.any(String),
......@@ -321,7 +336,7 @@ describe('feature flag form', () => {
protected: false,
rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [],
rolloutUserIds: '',
},
]);
})
......@@ -330,87 +345,4 @@ describe('feature flag form', () => {
});
});
});
describe('updateUserIds', () => {
beforeEach(() => {
factory({
...requiredProps,
name: 'feature_flag_1',
description: 'this is a feature flag',
scopes: [
{
environment_scope: 'production',
can_update: true,
protected: true,
active: false,
},
{
environment_scope: 'staging',
can_update: true,
protected: true,
active: false,
},
],
});
});
it('should set the user ids on all scopes', () => {
wrapper.vm.updateUserIds(['123', '456']);
wrapper.vm.formScopes.forEach(s => {
expect(s.rolloutUserIds).toEqual(['123', '456']);
});
});
});
describe('userIds', () => {
it('should get the user ids from the first scope with them', () => {
factory({
...requiredProps,
name: 'feature_flag_1',
description: 'this is a feature flag',
scopes: [
{
environment_scope: 'production',
can_update: true,
protected: true,
active: false,
rolloutUserIds: ['123', '456'],
},
{
environment_scope: 'staging',
can_update: true,
protected: true,
active: false,
rolloutUserIds: ['123', '456'],
},
],
});
expect(wrapper.vm.userIds).toEqual(['123', '456']);
});
it('should return an empty array if there are no user IDs set', () => {
factory({
...requiredProps,
name: 'feature_flag_1',
description: 'this is a feature flag',
scopes: [
{
environment_scope: 'production',
can_update: true,
protected: true,
active: false,
},
{
environment_scope: 'staging',
can_update: true,
protected: true,
active: false,
},
],
});
expect(wrapper.vm.userIds).toEqual([]);
});
});
});
......@@ -66,7 +66,7 @@ describe('New feature flag form', () => {
active: true,
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [],
rolloutUserIds: '',
},
]);
......
......@@ -7222,6 +7222,9 @@ msgstr ""
msgid "FeatureFlags|Inactive flag for %{scope}"
msgstr ""
msgid "FeatureFlags|Include additional user IDs"
msgstr ""
msgid "FeatureFlags|Install a %{docs_link_anchored_start}compatible client library%{docs_link_anchored_end} and specify the API URL, application name, and instance ID during the configuration setup. %{docs_link_start}More Information%{docs_link_end}"
msgstr ""
......
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