Commit a065afb2 authored by Fatih Acet's avatar Fatih Acet

Merge branch 'ff-user-ids-per-scope-fe' into 'master'

Rework Users with ID for Feature Flags

See merge request gitlab-org/gitlab!19399
parents 7c8d275a cd89e30c
---
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: '',
},
]);
......
......@@ -7264,6 +7264,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