Commit 9b8d478f authored by Jason Goodman's avatar Jason Goodman Committed by Nick Thomas

Update backend for target users feature and enable frontend

Migrate operations_feature_flag_scopes that are disabled with a list
of user ids - save an enabled scope discarding all strategies but
the userWithId strategy
Return strategies as they appear in the database. Revert change
introduced in MR 15500
Enable new frontend for target users feature
parent 1c854797
---
title: Add feature to allow specifying userWithId strategies per environment spec
merge_request: 20325
author:
type: added
# frozen_string_literal: true
class MigrateOpsFeatureFlagsScopesTargetUserIds < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
class OperationsFeatureFlagScope < ActiveRecord::Base
include EachBatch
self.table_name = 'operations_feature_flag_scopes'
self.inheritance_column = :_type_disabled
end
###
# 2019-11-26
#
# There are about 1000 rows in the operations_feature_flag_scopes table on gitlab.com.
# This migration will update about 30 of them.
# https://gitlab.com/gitlab-org/gitlab/merge_requests/20325#note_250742098
#
# This should take a few seconds to run.
# https://gitlab.com/gitlab-org/gitlab/merge_requests/20325#note_254871603
#
###
def up
OperationsFeatureFlagScope.where("strategies @> ?", [{ 'name': 'userWithId' }].to_json).each_batch do |scopes|
scopes.each do |scope|
if scope.active
default_strategy = scope.strategies.find { |s| s['name'] == 'default' }
if default_strategy.present?
scope.update({ strategies: [default_strategy] })
end
else
user_with_id_strategy = scope.strategies.find { |s| s['name'] == 'userWithId' }
scope.update({
active: true,
strategies: [user_with_id_strategy]
})
end
end
end
end
def down
# This is not reversible.
# The old Target Users feature required the same list of user ids to be applied to each environment scope.
# Now we allow the list of user ids to differ for each scope.
end
end
...@@ -22,7 +22,6 @@ import { ...@@ -22,7 +22,6 @@ import {
INTERNAL_ID_PREFIX, INTERNAL_ID_PREFIX,
} from '../constants'; } from '../constants';
import { createNewEnvironmentScope } from '../store/modules/helpers'; import { createNewEnvironmentScope } from '../store/modules/helpers';
import UserWithId from './strategies/user_with_id.vue';
export default { export default {
components: { components: {
...@@ -34,7 +33,6 @@ export default { ...@@ -34,7 +33,6 @@ export default {
ToggleButton, ToggleButton,
Icon, Icon,
EnvironmentsDropdown, EnvironmentsDropdown,
UserWithId,
}, },
directives: { directives: {
GlTooltip: GlTooltipDirective, GlTooltip: GlTooltipDirective,
...@@ -92,6 +90,7 @@ export default { ...@@ -92,6 +90,7 @@ export default {
ROLLOUT_STRATEGY_ALL_USERS, ROLLOUT_STRATEGY_ALL_USERS,
ROLLOUT_STRATEGY_PERCENT_ROLLOUT, ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
ROLLOUT_STRATEGY_USER_ID,
// Matches numbers 0 through 100 // Matches numbers 0 through 100
rolloutPercentageRegex: /^[0-9]$|^[1-9][0-9]$|^100$/, rolloutPercentageRegex: /^[0-9]$|^[1-9][0-9]$|^100$/,
...@@ -117,14 +116,6 @@ export default { ...@@ -117,14 +116,6 @@ export default {
permissionsFlag() { permissionsFlag() {
return this.glFeatures.featureFlagPermissions; return this.glFeatures.featureFlagPermissions;
}, },
userIds() {
const scope = this.formScopes.find(s => Array.isArray(s.rolloutUserIds)) || {};
return scope.rolloutUserIds || [];
},
shouldShowUsersPerEnvironment() {
return this.glFeatures.featureFlagsUsersPerEnvironment;
},
}, },
methods: { methods: {
isAllEnvironment(name) { isAllEnvironment(name) {
...@@ -174,13 +165,6 @@ export default { ...@@ -174,13 +165,6 @@ export default {
}); });
}, },
updateUserIds(userIds) {
this.formScopes = this.formScopes.map(s => ({
...s,
rolloutUserIds: userIds,
}));
},
canUpdateScope(scope) { canUpdateScope(scope) {
return !this.permissionsFlag || scope.canUpdate; return !this.permissionsFlag || scope.canUpdate;
}, },
...@@ -337,10 +321,7 @@ export default { ...@@ -337,10 +321,7 @@ export default {
<option :value="$options.ROLLOUT_STRATEGY_PERCENT_ROLLOUT"> <option :value="$options.ROLLOUT_STRATEGY_PERCENT_ROLLOUT">
{{ s__('FeatureFlags|Percent rollout (logged in users)') }} {{ s__('FeatureFlags|Percent rollout (logged in users)') }}
</option> </option>
<option <option :value="$options.ROLLOUT_STRATEGY_USER_ID">
v-if="shouldShowUsersPerEnvironment"
:value="$options.ROLLOUT_STRATEGY_USER_ID"
>
{{ s__('FeatureFlags|User IDs') }} {{ s__('FeatureFlags|User IDs') }}
</option> </option>
</select> </select>
...@@ -379,10 +360,7 @@ export default { ...@@ -379,10 +360,7 @@ export default {
</gl-tooltip> </gl-tooltip>
<span class="ml-1">%</span> <span class="ml-1">%</span>
</div> </div>
<div <div class="d-flex flex-column align-items-start mt-2 w-100">
v-if="shouldShowUsersPerEnvironment"
class="d-flex flex-column align-items-start mt-2 w-100"
>
<gl-form-checkbox <gl-form-checkbox
v-if="shouldDisplayIncludeUserIds(scope)" v-if="shouldDisplayIncludeUserIds(scope)"
v-model="scope.shouldIncludeUserIds" v-model="scope.shouldIncludeUserIds"
...@@ -476,8 +454,6 @@ export default { ...@@ -476,8 +454,6 @@ export default {
</div> </div>
</fieldset> </fieldset>
<user-with-id v-if="!shouldShowUsersPerEnvironment" :value="userIds" @input="updateUserIds" />
<div class="form-actions"> <div class="form-actions">
<gl-button <gl-button
ref="submitButton" ref="submitButton"
......
<script>
import _ from 'underscore';
import { GlFormGroup, GlFormInput, GlBadge, GlButton } from '@gitlab/ui';
import Icon from '~/vue_shared/components/icon.vue';
import { sprintf, s__ } from '~/locale';
export default {
targetUsersHeader: s__('FeatureFlags|Target Users'),
userIdLabel: s__('FeatureFlags|User IDs'),
userIdHelp: s__('FeatureFlags|Enter comma separated list of user IDs'),
addButtonLabel: s__('FeatureFlags|Add'),
clearAllButtonLabel: s__('FeatureFlags|Clear all'),
targetUsersHtml: sprintf(
s__(
'FeatureFlags|Target user behaviour is built up by creating a list of active user IDs. These IDs should be the users in the system in which the feature flag is set, not GitLab ids. Target users apply across %{strong_start}All Environments%{strong_end} and are not affected by Target Environment rules.',
),
{
strong_start: '<strong>',
strong_end: '</strong>',
},
false,
),
components: {
GlFormGroup,
GlFormInput,
GlBadge,
GlButton,
Icon,
},
props: {
value: {
type: Array,
required: true,
},
},
data() {
return {
userId: '',
};
},
computed: {},
methods: {
/**
* @description Given a comma-separated list of IDs, append it to current
* list of user IDs. IDs are only added if they are new, i.e., the list
* contains only unique IDs, and those IDs must also be a truthy value,
* i.e., they cannot be empty strings. The result is then emitted to
* parent component via the 'input' event.
* @param {string} value - A list of user IDs comma-separated ("1,2,3")
*/
updateUserIds(value = this.userId) {
this.userId = '';
this.$emit(
'input',
_.uniq([
...this.value,
...value
.split(',')
.filter(x => x)
.map(x => x.trim()),
]),
);
},
/**
* @description Removes the given ID from the current list of IDs. and
* emits the result via the `input` event.
* @param {string} id - The ID to remove.
*/
removeUser(id) {
this.$emit('input', this.value.filter(i => i !== id));
},
/**
* @description Clears both the user ID list via the 'input' event as well
* as the value of the comma-separated list
*/
clearAll() {
this.$emit('input', []);
this.userId = '';
},
/**
* @description Updates the list of user IDs with those in the
* comma-separated list.
* @see {@link updateUserIds}
*/
onClickAdd() {
this.updateUserIds(this.userId);
},
},
};
</script>
<template>
<fieldset class="mb-5">
<h4>{{ $options.targetUsersHeader }}</h4>
<p v-html="$options.targetUsersHtml"></p>
<gl-form-group
:label="$options.userIdLabel"
:description="$options.userIdHelp"
label-for="userId"
>
<div class="d-flex">
<gl-form-input
id="userId"
v-model="userId"
class="col-md-4 mr-2"
@keyup.enter.native="updateUserIds()"
/>
<gl-button variant="success" class="btn-inverted mr-1" @click="onClickAdd">
{{ $options.addButtonLabel }}
</gl-button>
<gl-button variant="danger" class="btn btn-inverted" @click="clearAll">
{{ $options.clearAllButtonLabel }}
</gl-button>
</div>
</gl-form-group>
<div class="d-flex flex-wrap">
<gl-badge v-for="id in value" :key="id" :pill="true" class="m-1 d-flex align-items-center">
<p class="ws-normal m-1 text-break text-left">{{ id }}</p>
<span @click="removeUser(id)"><icon name="close"/></span>
</gl-badge>
</div>
</fieldset>
</template>
...@@ -10,16 +10,6 @@ import { ...@@ -10,16 +10,6 @@ import {
fetchUserIdParams, fetchUserIdParams,
} from '../../constants'; } from '../../constants';
/*
* Part of implementing https://gitlab.com/gitlab-org/gitlab/issues/34363
* involves moving the current Array-based list of user IDs (as it is stored as
* a list of tokens) to a String-based list of user IDs, editable in a text area
* per environment.
*/
const shouldShowUsersPerEnvironment = () =>
(window.gon && window.gon.features && window.gon.features.featureFlagsUsersPerEnvironment) ||
false;
/** /**
* Converts raw scope objects fetched from the API into an array of scope * Converts raw scope objects fetched from the API into an array of scope
* objects that is easier/nicer to bind to in Vue. * objects that is easier/nicer to bind to in Vue.
...@@ -31,24 +21,21 @@ export const mapToScopesViewModel = scopesFromRails => ...@@ -31,24 +21,21 @@ export const mapToScopesViewModel = scopesFromRails =>
strat => strat.name === ROLLOUT_STRATEGY_PERCENT_ROLLOUT, strat => strat.name === ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
); );
const rolloutStrategy = percentStrategy ? percentStrategy.name : ROLLOUT_STRATEGY_ALL_USERS;
const rolloutPercentage = fetchPercentageParams(percentStrategy) || DEFAULT_PERCENT_ROLLOUT; const rolloutPercentage = fetchPercentageParams(percentStrategy) || DEFAULT_PERCENT_ROLLOUT;
const userStrategy = (s.strategies || []).find( const userStrategy = (s.strategies || []).find(
strat => strat.name === ROLLOUT_STRATEGY_USER_ID, strat => strat.name === ROLLOUT_STRATEGY_USER_ID,
); );
let rolloutUserIds = ''; const rolloutStrategy =
(percentStrategy && percentStrategy.name) ||
(userStrategy && userStrategy.name) ||
ROLLOUT_STRATEGY_ALL_USERS;
if (shouldShowUsersPerEnvironment()) { const rolloutUserIds = (fetchUserIdParams(userStrategy) || '')
rolloutUserIds = (fetchUserIdParams(userStrategy) || '') .split(',')
.split(',') .filter(id => id)
.filter(id => id) .join(', ');
.join(', ');
} else {
rolloutUserIds = (fetchUserIdParams(userStrategy) || '').split(',').filter(id => id);
}
return { return {
id: s.id, id: s.id,
...@@ -80,9 +67,7 @@ export const mapFromScopesViewModel = params => { ...@@ -80,9 +67,7 @@ export const mapFromScopesViewModel = params => {
const userIdParameters = {}; const userIdParameters = {};
const hasUsers = s.shouldIncludeUserIds || s.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID; if (s.shouldIncludeUserIds || s.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID) {
if (shouldShowUsersPerEnvironment() && hasUsers) {
userIdParameters.userIds = (s.rolloutUserIds || '').replace(/, /g, ','); userIdParameters.userIds = (s.rolloutUserIds || '').replace(/, /g, ',');
} else if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) { } else if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) {
userIdParameters.userIds = s.rolloutUserIds.join(','); userIdParameters.userIds = s.rolloutUserIds.join(',');
...@@ -141,7 +126,7 @@ export const createNewEnvironmentScope = (overrides = {}, featureFlagPermissions ...@@ -141,7 +126,7 @@ export const createNewEnvironmentScope = (overrides = {}, featureFlagPermissions
id: _.uniqueId(INTERNAL_ID_PREFIX), id: _.uniqueId(INTERNAL_ID_PREFIX),
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS, rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT, rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: shouldShowUsersPerEnvironment() ? '' : [], rolloutUserIds: '',
}; };
const newScope = { const newScope = {
......
...@@ -26,12 +26,6 @@ module Operations ...@@ -26,12 +26,6 @@ module Operations
scope :enabled, -> { where(active: true) } scope :enabled, -> { where(active: true) }
scope :disabled, -> { where(active: false) } scope :disabled, -> { where(active: false) }
def userwithid_strategy
strong_memoize(:userwithid_strategy) do
strategies.select { |s| s['name'] == FeatureFlagStrategiesValidator::STRATEGY_USERWITHID }
end
end
def self.with_name_and_description def self.with_name_and_description
joins(:feature_flag) joins(:feature_flag)
.select(FeatureFlag.arel_table[:name], FeatureFlag.arel_table[:description]) .select(FeatureFlag.arel_table[:name], FeatureFlag.arel_table[:description])
......
...@@ -783,22 +783,8 @@ module EE ...@@ -783,22 +783,8 @@ module EE
class UnleashFeature < Grape::Entity class UnleashFeature < Grape::Entity
expose :name expose :name
expose :description, unless: ->(feature) { feature.description.nil? } expose :description, unless: ->(feature) { feature.description.nil? }
# The UI has a single field for user ids for whom the feature flag should be enabled across all scopes. expose :active, as: :enabled
# Each scope is given a userWithId strategy with the list of user ids. expose :strategies
# However, the user can also directly toggle the active field of a scope.
# So if the user has entered user ids, and disabled the scope, we need to send an enabled scope with
# the list of user ids.
# See: https://gitlab.com/gitlab-org/gitlab/issues/14011
expose :active, as: :enabled do |feature|
feature.active || feature.userwithid_strategy.present?
end
expose :strategies do |feature|
if !feature.active && feature.userwithid_strategy.present?
feature.userwithid_strategy
else
feature.strategies
end
end
end end
class GitlabSubscription < Grape::Entity class GitlabSubscription < Grape::Entity
......
...@@ -63,7 +63,7 @@ describe('New feature flag form', () => { ...@@ -63,7 +63,7 @@ describe('New feature flag form', () => {
active: true, active: true,
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS, rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT, rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [], rolloutUserIds: '',
}; };
expect(wrapper.vm.scopes).toEqual([defaultScope]); expect(wrapper.vm.scopes).toEqual([defaultScope]);
......
import { shallowMount, createLocalVue } from '@vue/test-utils';
import { GlFormInput } from '@gitlab/ui';
import component from 'ee/feature_flags/components/strategies/user_with_id.vue';
const localVue = createLocalVue();
describe('User With ID', () => {
const Component = localVue.extend(component);
let wrapper;
let propsData;
afterEach(() => wrapper.destroy());
beforeEach(() => {
propsData = {
value: [],
};
wrapper = shallowMount(Component, {
propsData,
localVue,
});
});
describe('input change', () => {
it('should split a value by comma', () => {
wrapper.vm.updateUserIds('123,456,789');
expect(wrapper.emitted('input')).toContainEqual([['123', '456', '789']]);
});
it('should clear the value of the userId', () => {
wrapper.vm.userId = '123';
wrapper.vm.updateUserIds('123');
expect(wrapper.vm.userId).toBe('');
});
it('should add new ids to the array of user ids', () => {
wrapper.setProps({ value: ['123', '456', '789'] });
wrapper.vm.updateUserIds('321,654,987');
expect(wrapper.emitted('input')).toContainEqual([['123', '456', '789', '321', '654', '987']]);
});
it('should dedupe newly added IDs', () => {
wrapper.vm.updateUserIds('123,123,123');
expect(wrapper.emitted('input')).toContainEqual([['123']]);
});
it('should only allow the addition of new IDs', () => {
wrapper.vm.updateUserIds('123,123,123');
expect(wrapper.emitted('input')).toContainEqual([['123']]);
wrapper.vm.updateUserIds('123,123,123,456');
expect(wrapper.emitted('input')).toContainEqual([['123', '456']]);
});
it('should only allow the addition of truthy values', () => {
wrapper.vm.updateUserIds(',,,,,,');
expect(wrapper.vm.value).toEqual([]);
});
it('should be called on the input change event', () => {
wrapper.setMethods({ updateUserIds: jest.fn() });
wrapper.find(GlFormInput).trigger('keyup', { keyCode: 13 });
expect(wrapper.vm.updateUserIds).toHaveBeenCalled();
});
});
describe('remove', () => {
it('should remove the given ID', () => {
wrapper.setProps({ value: ['0', '1', '2', '3'] });
wrapper.vm.removeUser('1');
expect(wrapper.emitted('input')[0]).toEqual([['0', '2', '3']]);
});
it('should not do anything if the ID is not present', () => {
wrapper.setProps({ value: ['0', '1', '2', '3'] });
wrapper.vm.removeUser('-1');
wrapper.vm.removeUser('6');
expect(wrapper.emitted('input')[0]).toEqual([['0', '1', '2', '3']]);
expect(wrapper.emitted('input')[1]).toEqual([['0', '1', '2', '3']]);
});
it('should be bound to the remove button on a badge', () => {
wrapper.setProps({ value: ['0', '1', '2', '3'] });
wrapper.setMethods({ removeUser: jest.fn() });
wrapper.find('span').trigger('click');
expect(wrapper.vm.removeUser).toHaveBeenCalled();
});
});
describe('clearAll', () => {
it('should reset the user ids to an empty array', () => {
wrapper.setProps({ value: ['0', '1', '2', '3'] });
wrapper.vm.clearAll();
expect(wrapper.emitted('input')).toContainEqual([[]]);
});
it('should be bound to the clear all button', () => {
wrapper.setMethods({ clearAll: jest.fn() });
wrapper.find('[variant="danger"]').vm.$emit('click');
expect(wrapper.vm.clearAll).toHaveBeenCalled();
});
});
});
...@@ -51,7 +51,7 @@ describe('feature flags helpers spec', () => { ...@@ -51,7 +51,7 @@ describe('feature flags helpers spec', () => {
protected: true, protected: true,
rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT, rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
rolloutPercentage: '56', rolloutPercentage: '56',
rolloutUserIds: ['123', '234'], rolloutUserIds: '123, 234',
shouldBeDestroyed: true, shouldBeDestroyed: true,
}), }),
]; ];
...@@ -310,7 +310,7 @@ describe('feature flags helpers spec', () => { ...@@ -310,7 +310,7 @@ describe('feature flags helpers spec', () => {
id: expect.stringContaining(INTERNAL_ID_PREFIX), id: expect.stringContaining(INTERNAL_ID_PREFIX),
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS, rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT, rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [], rolloutUserIds: '',
}; };
const actual = createNewEnvironmentScope(); const actual = createNewEnvironmentScope();
...@@ -330,7 +330,7 @@ describe('feature flags helpers spec', () => { ...@@ -330,7 +330,7 @@ describe('feature flags helpers spec', () => {
id: expect.stringContaining(INTERNAL_ID_PREFIX), id: expect.stringContaining(INTERNAL_ID_PREFIX),
rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS, rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
rolloutPercentage: DEFAULT_PERCENT_ROLLOUT, rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
rolloutUserIds: [], rolloutUserIds: '',
}; };
const actual = createNewEnvironmentScope(overrides); const actual = createNewEnvironmentScope(overrides);
......
...@@ -248,38 +248,16 @@ describe API::Unleash do ...@@ -248,38 +248,16 @@ describe API::Unleash do
end end
context "with an inactive scope" do context "with an inactive scope" do
let!(:scope) { create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: false, strategies: strategies) } let!(:scope) { create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: false, strategies: [{ name: "default", parameters: {} }]) }
let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } } let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } }
context 'with userWithId strategy' do it 'returns a disabled feature' do
let(:strategies) { [{ name: "default", parameters: {} }, { name: "userWithId", parameters: { userIds: "fred" } }] } get api(features_url), headers: headers
it 'returns an enabled feature with only the userWithId strategy' do
get api(features_url), headers: headers
expect(response).to have_gitlab_http_status(:ok)
feature_json = json_response['features'].first
expect(feature_json['enabled']).to eq(true)
expect(feature_json['strategies']).to eq([{
'name' => 'userWithId',
'parameters' => {
'userIds' => 'fred'
}
}])
end
end
context 'with default strategy' do
let(:strategies) { [{ name: "default", parameters: {} }] }
it 'returns a disabled feature that does not contain a userWithId strategy' do expect(response).to have_gitlab_http_status(:ok)
get api(features_url), headers: headers feature_json = json_response['features'].first
expect(feature_json['enabled']).to eq(false)
expect(response).to have_gitlab_http_status(:ok) expect(feature_json['strategies']).to eq([{ 'name' => 'default', 'parameters' => {} }])
feature_json = json_response['features'].first
expect(feature_json['enabled']).to eq(false)
expect(feature_json['strategies']).to eq([{ 'name' => 'default', 'parameters' => {} }])
end
end end
end end
end end
......
...@@ -7437,15 +7437,9 @@ msgstr "" ...@@ -7437,15 +7437,9 @@ msgstr ""
msgid "FeatureFlags|Active" msgid "FeatureFlags|Active"
msgstr "" msgstr ""
msgid "FeatureFlags|Add"
msgstr ""
msgid "FeatureFlags|All users" msgid "FeatureFlags|All users"
msgstr "" msgstr ""
msgid "FeatureFlags|Clear all"
msgstr ""
msgid "FeatureFlags|Configure" msgid "FeatureFlags|Configure"
msgstr "" msgstr ""
...@@ -7467,9 +7461,6 @@ msgstr "" ...@@ -7467,9 +7461,6 @@ msgstr ""
msgid "FeatureFlags|Edit Feature Flag" msgid "FeatureFlags|Edit Feature Flag"
msgstr "" msgstr ""
msgid "FeatureFlags|Enter comma separated list of user IDs"
msgstr ""
msgid "FeatureFlags|Environment Spec" msgid "FeatureFlags|Environment Spec"
msgstr "" msgstr ""
...@@ -7551,15 +7542,9 @@ msgstr "" ...@@ -7551,15 +7542,9 @@ msgstr ""
msgid "FeatureFlags|Status" msgid "FeatureFlags|Status"
msgstr "" msgstr ""
msgid "FeatureFlags|Target Users"
msgstr ""
msgid "FeatureFlags|Target environments" msgid "FeatureFlags|Target environments"
msgstr "" msgstr ""
msgid "FeatureFlags|Target user behaviour is built up by creating a list of active user IDs. These IDs should be the users in the system in which the feature flag is set, not GitLab ids. Target users apply across %{strong_start}All Environments%{strong_end} and are not affected by Target Environment rules."
msgstr ""
msgid "FeatureFlags|There are no active feature flags" msgid "FeatureFlags|There are no active feature flags"
msgstr "" msgstr ""
......
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20191118211629_migrate_ops_feature_flags_scopes_target_user_ids.rb')
describe MigrateOpsFeatureFlagsScopesTargetUserIds, :migration do
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:flags) { table(:operations_feature_flags) }
let(:scopes) { table(:operations_feature_flag_scopes) }
def setup
namespace = namespaces.create!(name: 'foo', path: 'foo')
project = projects.create!(namespace_id: namespace.id)
flag = flags.create!(project_id: project.id, active: true, name: 'test_flag')
flag
end
it 'migrates successfully when there are no scopes in the database' do
setup
disable_migrations_output { migrate! }
expect(scopes.count).to eq(0)
end
it 'migrates a disabled scope with gradualRolloutUserId and userWithId strategies' do
flag = setup
scope = scopes.create!(feature_flag_id: flag.id, active: false, strategies: [
{ name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '50' } },
{ name: 'userWithId', parameters: { userIds: '5' } }
])
disable_migrations_output { migrate! }
scope.reload
expect(scope.active).to eq(true)
expect(scope.strategies).to eq([{ 'name' => 'userWithId', 'parameters' => { 'userIds' => '5' } }])
end
it 'migrates a disabled scope with default and userWithId strategies' do
flag = setup
scope = scopes.create!(feature_flag_id: flag.id, active: false, strategies: [
{ name: 'default', parameters: {} },
{ name: 'userWithId', parameters: { userIds: 'amy@gmail.com,karen@gmail.com' } }
])
disable_migrations_output { migrate! }
scope.reload
expect(scope.active).to eq(true)
expect(scope.strategies).to eq([{ 'name' => 'userWithId', 'parameters' => { 'userIds' => 'amy@gmail.com,karen@gmail.com' } }])
end
it 'migrates an enabled scope with default and userWithId strategies' do
flag = setup
scope = scopes.create!(feature_flag_id: flag.id, active: true, strategies: [
{ name: 'default', parameters: {} },
{ name: 'userWithId', parameters: { userIds: 'tim' } }
])
disable_migrations_output { migrate! }
scope.reload
expect(scope.active).to eq(true)
expect(scope.strategies).to eq([{ 'name' => 'default', 'parameters' => {} }])
end
it 'does not alter an enabled scope with gradualRolloutUserId and userWithId strategies' do
flag = setup
scope = scopes.create!(feature_flag_id: flag.id, active: true, strategies: [
{ name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '50' } },
{ name: 'userWithId', parameters: { userIds: '5' } }
])
disable_migrations_output { migrate! }
scope.reload
expect(scope.active).to eq(true)
expect(scope.strategies).to eq([
{ 'name' => 'gradualRolloutUserId', 'parameters' => { 'groupId' => 'default', 'percentage' => '50' } },
{ 'name' => 'userWithId', 'parameters' => { 'userIds' => '5' } }
])
end
it 'does not alter a disabled scope without a userWithId strategy' do
flag = setup
scope = scopes.create!(feature_flag_id: flag.id, active: false, strategies: [
{ name: 'gradualRolloutUserId', parameters: { percentage: '60' } }
])
disable_migrations_output { migrate! }
scope.reload
expect(scope.active).to eq(false)
expect(scope.strategies).to eq([
{ 'name' => 'gradualRolloutUserId', 'parameters' => { 'percentage' => '60' } }
])
end
it 'does not alter an enabled scope without a userWithId strategy' do
flag = setup
scope = scopes.create!(feature_flag_id: flag.id, active: true, strategies: [
{ name: 'default', parameters: {} }
])
disable_migrations_output { migrate! }
scope.reload
expect(scope.active).to eq(true)
expect(scope.strategies).to eq([
{ 'name' => 'default', 'parameters' => {} }
])
end
it 'migrates multiple scopes' do
flag = setup
scope_a = scopes.create!(feature_flag_id: flag.id, active: false, strategies: [
{ name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '50' } },
{ name: 'userWithId', parameters: { userIds: '5,6,7' } }
])
scope_b = scopes.create!(feature_flag_id: flag.id, active: false, environment_scope: 'production', strategies: [
{ name: 'default', parameters: {} },
{ name: 'userWithId', parameters: { userIds: 'lisa,carol' } }
])
disable_migrations_output { migrate! }
scope_a.reload
scope_b.reload
expect(scope_a.active).to eq(true)
expect(scope_a.strategies).to eq([{ 'name' => 'userWithId', 'parameters' => { 'userIds' => '5,6,7' } }])
expect(scope_b.active).to eq(true)
expect(scope_b.strategies).to eq([{ 'name' => 'userWithId', 'parameters' => { 'userIds' => 'lisa,carol' } }])
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