Merge branch 'user-id-ff-strategy-fe' into 'master'

Add Ability to Enable Feature Flags by User ID - Frontend

See merge request gitlab-org/gitlab-ee!14596

ee/app/assets/javascripts/feature_flags/components/form.vue

@@ -13,6 +13,7 @@ import {
   INTERNAL_ID_PREFIX,
 } from '../constants';
 import { createNewEnvironmentScope } from '../store/modules/helpers';
+import UserWithId from './strategies/user_with_id.vue';
 
 export default {
   components: {
@@ -22,6 +23,7 @@ export default {
     ToggleButton,
     Icon,
     EnvironmentsDropdown,
+    UserWithId,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -98,6 +100,11 @@ export default {
     permissionsFlag() {
       return gon && gon.features && gon.features.featureFlagPermissions;
     },
+
+    userIds() {
+      const scope = this.formScopes.find(s => Array.isArray(s.rolloutUserIds)) || {};
+      return scope.rolloutUserIds || [];
+    },
   },
   methods: {
     isAllEnvironment(name) {
@@ -146,6 +153,13 @@ export default {
       });
     },
 
+    updateUserIds(userIds) {
+      this.formScopes = this.formScopes.map(s => ({
+        ...s,
+        rolloutUserIds: userIds,
+      }));
+    },
+
     canUpdateScope(scope) {
       return !this.permissionsFlag || scope.canUpdate;
     },
@@ -397,6 +411,7 @@ export default {
         </div>
       </div>
     </fieldset>
+    <user-with-id :value="userIds" @input="updateUserIds" />
 
     <div class="form-actions">
       <gl-button
@@ -405,14 +420,12 @@ export default {
         variant="success"
         class="js-ff-submit col-xs-12"
         @click="handleSubmit"
-        >{{ submitText }}</gl-button
-      >
-      <gl-button
-        :href="cancelPath"
-        variant="secondary"
-        class="js-ff-cancel col-xs-12 float-right"
-        >{{ __('Cancel') }}</gl-button
       >
+        {{ submitText }}
+      </gl-button>
+      <gl-button :href="cancelPath" variant="secondary" class="js-ff-cancel col-xs-12 float-right">
+        {{ __('Cancel') }}
+      </gl-button>
     </div>
   </form>
 </template>

ee/app/assets/javascripts/feature_flags/components/strategies/user_with_id.vue

+<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>

ee/app/assets/javascripts/feature_flags/constants.js

+import _ from 'underscore';
+
 export const ROLLOUT_STRATEGY_ALL_USERS = 'default';
 export const ROLLOUT_STRATEGY_PERCENT_ROLLOUT = 'gradualRolloutUserId';
+export const ROLLOUT_STRATEGY_USER_ID = 'userWithId';
 
 export const PERCENT_ROLLOUT_GROUP_ID = 'default';
 
@@ -8,3 +11,6 @@ export const DEFAULT_PERCENT_ROLLOUT = '100';
 export const ALL_ENVIRONMENTS_NAME = '*';
 
 export const INTERNAL_ID_PREFIX = 'internal_';
+
+export const fetchPercentageParams = _.property(['parameters', 'percentage']);
+export const fetchUserIdParams = _.property(['parameters', 'userIds']);

ee/app/assets/javascripts/feature_flags/store/modules/helpers.js

@@ -2,9 +2,12 @@ import _ from 'underscore';
 import {
   ROLLOUT_STRATEGY_ALL_USERS,
   ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+  ROLLOUT_STRATEGY_USER_ID,
   INTERNAL_ID_PREFIX,
   DEFAULT_PERCENT_ROLLOUT,
   PERCENT_ROLLOUT_GROUP_ID,
+  fetchPercentageParams,
+  fetchUserIdParams,
 } from '../../constants';
 
 /**
@@ -14,14 +17,19 @@ import {
  */
 export const mapToScopesViewModel = scopesFromRails =>
   (scopesFromRails || []).map(s => {
-    const [strategy] = s.strategies || [];
+    const percentStrategy = (s.strategies || []).find(
+      strat => strat.name === ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+    );
 
-    const rolloutStrategy = strategy ? strategy.name : ROLLOUT_STRATEGY_ALL_USERS;
+    const rolloutStrategy = percentStrategy ? percentStrategy.name : ROLLOUT_STRATEGY_ALL_USERS;
 
-    let rolloutPercentage = DEFAULT_PERCENT_ROLLOUT;
-    if (strategy && strategy.parameters && strategy.parameters.percentage) {
-      rolloutPercentage = strategy.parameters.percentage;
-    }
+    const rolloutPercentage = fetchPercentageParams(percentStrategy) || DEFAULT_PERCENT_ROLLOUT;
+
+    const userStrategy = (s.strategies || []).find(
+      strat => strat.name === ROLLOUT_STRATEGY_USER_ID,
+    );
+
+    const rolloutUserIds = (fetchUserIdParams(userStrategy) || '').split(',').filter(id => id);
 
     return {
       id: s.id,
@@ -31,12 +39,12 @@ export const mapToScopesViewModel = scopesFromRails =>
       protected: Boolean(s.protected),
       rolloutStrategy,
       rolloutPercentage,
+      rolloutUserIds,
 
       // eslint-disable-next-line no-underscore-dangle
       shouldBeDestroyed: Boolean(s._destroy),
     };
   });
-
 /**
  * Converts the parameters emitted by the Vue component into
  * the shape that the Rails API expects.
@@ -44,15 +52,31 @@ export const mapToScopesViewModel = scopesFromRails =>
  */
 export const mapFromScopesViewModel = params => {
   const scopes = (params.scopes || []).map(s => {
-    const parameters = {};
+    const percentParameters = {};
     if (s.rolloutStrategy === ROLLOUT_STRATEGY_PERCENT_ROLLOUT) {
-      parameters.groupId = PERCENT_ROLLOUT_GROUP_ID;
-      parameters.percentage = s.rolloutPercentage;
+      percentParameters.groupId = PERCENT_ROLLOUT_GROUP_ID;
+      percentParameters.percentage = s.rolloutPercentage;
+    }
+
+    const userIdParameters = {};
+    if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) {
+      userIdParameters.userIds = s.rolloutUserIds.join(',');
     }
 
     // Strip out any internal IDs
     const id = _.isString(s.id) && s.id.startsWith(INTERNAL_ID_PREFIX) ? undefined : s.id;
 
+    const strategies = [
+      {
+        name: s.rolloutStrategy,
+        parameters: percentParameters,
+      },
+    ];
+
+    if (!_.isEmpty(userIdParameters)) {
+      strategies.push({ name: ROLLOUT_STRATEGY_USER_ID, parameters: userIdParameters });
+    }
+
     return {
       id,
       environment_scope: s.environmentScope,
@@ -60,12 +84,7 @@ export const mapFromScopesViewModel = params => {
       can_update: s.canUpdate,
       protected: s.protected,
       _destroy: s.shouldBeDestroyed,
-      strategies: [
-        {
-          name: s.rolloutStrategy,
-          parameters,
-        },
-      ],
+      strategies,
     };
   });
 
@@ -94,6 +113,7 @@ export const createNewEnvironmentScope = (overrides = {}) => {
     id: _.uniqueId(INTERNAL_ID_PREFIX),
     rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
     rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
+    rolloutUserIds: [],
   };
 
   const newScope = {

ee/changelogs/unreleased/user-id-ff-strategy-fe.yml

+---
+title: Add Ability to Enable Feature Flags by User ID
+merge_request: 14596
+author:
+type: added

ee/spec/frontend/feature_flags/components/strategies/user_with_id_spec.js

+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();
+    });
+  });
+});

ee/spec/frontend/feature_flags/store/helpers_spec.js

@@ -7,6 +7,7 @@ import {
 import {
   ROLLOUT_STRATEGY_ALL_USERS,
   ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+  ROLLOUT_STRATEGY_USER_ID,
   PERCENT_ROLLOUT_GROUP_ID,
   INTERNAL_ID_PREFIX,
   DEFAULT_PERCENT_ROLLOUT,
@@ -29,6 +30,12 @@ describe('feature flags helpers spec', () => {
                 percentage: '56',
               },
             },
+            {
+              name: ROLLOUT_STRATEGY_USER_ID,
+              parameters: {
+                userIds: '123,234',
+              },
+            },
           ],
 
           _destroy: true,
@@ -44,6 +51,7 @@ describe('feature flags helpers spec', () => {
           protected: true,
           rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
           rolloutPercentage: '56',
+          rolloutUserIds: ['123', '234'],
           shouldBeDestroyed: true,
         },
       ];
@@ -94,6 +102,7 @@ describe('feature flags helpers spec', () => {
             shouldBeDestroyed: true,
             rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
             rolloutPercentage: '48',
+            rolloutUserIds: ['123', '234'],
           },
         ],
       };
@@ -118,6 +127,12 @@ describe('feature flags helpers spec', () => {
                     percentage: '48',
                   },
                 },
+                {
+                  name: ROLLOUT_STRATEGY_USER_ID,
+                  parameters: {
+                    userIds: '123,234',
+                  },
+                },
               ],
             },
           ],
@@ -164,6 +179,7 @@ describe('feature flags helpers spec', () => {
         id: expect.stringContaining(INTERNAL_ID_PREFIX),
         rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
         rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
+        rolloutUserIds: [],
       };
 
       const actual = createNewEnvironmentScope();
@@ -183,6 +199,7 @@ describe('feature flags helpers spec', () => {
         id: expect.stringContaining(INTERNAL_ID_PREFIX),
         rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
         rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
+        rolloutUserIds: [],
       };
 
       const actual = createNewEnvironmentScope(overrides);

ee/spec/javascripts/feature_flags/components/form_spec.js

@@ -252,6 +252,7 @@ describe('feature flag form', () => {
               active: false,
               rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
               rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
+              rolloutUserIds: [],
             },
           ],
         });
@@ -305,6 +306,7 @@ describe('feature flag form', () => {
                 protected: true,
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: '55',
+                rolloutUserIds: [],
               },
               {
                 id: jasmine.any(String),
@@ -314,6 +316,7 @@ describe('feature flag form', () => {
                 protected: false,
                 rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
                 rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
+                rolloutUserIds: [],
               },
               {
                 id: jasmine.any(String),
@@ -323,6 +326,7 @@ describe('feature flag form', () => {
                 protected: false,
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
+                rolloutUserIds: [],
               },
             ]);
           })
@@ -331,4 +335,87 @@ 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([]);
+    });
+  });
 });

ee/spec/javascripts/feature_flags/components/new_feature_flag_spec.js

@@ -66,6 +66,7 @@ describe('New feature flag form', () => {
         active: true,
         rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
         rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
+        rolloutUserIds: [],
       },
     ]);
 

locale/gitlab.pot

@@ -6191,9 +6191,15 @@ msgstr ""
 msgid "FeatureFlags|Active"
 msgstr ""
 
+msgid "FeatureFlags|Add"
+msgstr ""
+
 msgid "FeatureFlags|All users"
 msgstr ""
 
+msgid "FeatureFlags|Clear all"
+msgstr ""
+
 msgid "FeatureFlags|Configure"
 msgstr ""
 
@@ -6215,6 +6221,9 @@ msgstr ""
 msgid "FeatureFlags|Edit Feature Flag"
 msgstr ""
 
+msgid "FeatureFlags|Enter comma separated list of user IDs"
+msgstr ""
+
 msgid "FeatureFlags|Environment Spec"
 msgstr ""
 
@@ -6287,9 +6296,15 @@ msgstr ""
 msgid "FeatureFlags|Status"
 msgstr ""
 
+msgid "FeatureFlags|Target Users"
+msgstr ""
+
 msgid "FeatureFlags|Target environments"
 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"
 msgstr ""
 
@@ -6302,6 +6317,9 @@ msgstr ""
 msgid "FeatureFlags|Try again in a few moments or contact your support team."
 msgstr ""
 
+msgid "FeatureFlags|User IDs"
+msgstr ""
+
 msgid "Feb"
 msgstr ""