Merge branch 'ff-users-per-environment-behind-feature-flag' into 'master'

Place the New Users per Environment Behind a FF

See merge request gitlab-org/gitlab!20088

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

 <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,
+    GlFormTextarea,
+    GlFormCheckbox,
     GlTooltip,
     ToggleButton,
     Icon,
@@ -107,6 +117,9 @@ export default {
       const scope = this.formScopes.find(s => Array.isArray(s.rolloutUserIds)) || {};
       return scope.rolloutUserIds || [];
     },
+    shouldShowUsersPerEnvironment() {
+      return this.glFeatures.featureFlagsUsersPerEnvironment;
+    },
   },
   methods: {
     isAllEnvironment(name) {
@@ -187,6 +200,18 @@ 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>
@@ -300,12 +325,18 @@ 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
+                        v-if="shouldShowUsersPerEnvironment"
+                        :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 +373,27 @@ export default {
                     </gl-tooltip>
                     <span class="ml-1">%</span>
                   </div>
+                  <div
+                    v-if="shouldShowUsersPerEnvironment"
+                    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>
 
@@ -415,6 +467,8 @@ export default {
     </fieldset>
     <user-with-id :value="userIds" @input="updateUserIds" />
 
+    <user-with-id v-if="!shouldShowUsersPerEnvironment" :value="userIds" @input="updateUserIds" />
+
     <div class="form-actions">
       <gl-button
         ref="submitButton"

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

@@ -10,6 +10,16 @@ import {
   fetchUserIdParams,
 } 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
  * objects that is easier/nicer to bind to in Vue.
@@ -29,7 +39,16 @@ export const mapToScopesViewModel = scopesFromRails =>
       strat => strat.name === ROLLOUT_STRATEGY_USER_ID,
     );
 
-    const rolloutUserIds = (fetchUserIdParams(userStrategy) || '').split(',').filter(id => id);
+    let rolloutUserIds = '';
+
+    if (shouldShowUsersPerEnvironment()) {
+      rolloutUserIds = (fetchUserIdParams(userStrategy) || '')
+        .split(',')
+        .filter(id => id)
+        .join(', ');
+    } else {
+      rolloutUserIds = (fetchUserIdParams(userStrategy) || '').split(',').filter(id => id);
+    }
 
     return {
       id: s.id,
@@ -43,6 +62,7 @@ export const mapToScopesViewModel = scopesFromRails =>
 
       // eslint-disable-next-line no-underscore-dangle
       shouldBeDestroyed: Boolean(s._destroy),
+      shouldIncludeUserIds: rolloutUserIds.length > 0,
     };
   });
 /**
@@ -59,7 +79,12 @@ export const mapFromScopesViewModel = params => {
     }
 
     const userIdParameters = {};
-    if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) {
+
+    const hasUsers = s.shouldIncludeUserIds || s.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID;
+
+    if (shouldShowUsersPerEnvironment() && hasUsers) {
+      userIdParameters.userIds = (s.rolloutUserIds || '').replace(/, /g, ',');
+    } else if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) {
       userIdParameters.userIds = s.rolloutUserIds.join(',');
     }
 
@@ -113,7 +138,7 @@ export const createNewEnvironmentScope = (overrides = {}, featureFlagPermissions
     id: _.uniqueId(INTERNAL_ID_PREFIX),
     rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
     rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-    rolloutUserIds: [],
+    rolloutUserIds: shouldShowUsersPerEnvironment() ? '' : [],
   };
 
   const newScope = {

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

@@ -43,7 +43,7 @@ describe('feature flags helpers spec', () => {
       ];
 
       const expected = [
-        {
+        expect.objectContaining({
           id: 3,
           environmentScope: 'environment_scope',
           active: true,
@@ -53,7 +53,7 @@ describe('feature flags helpers spec', () => {
           rolloutPercentage: '56',
           rolloutUserIds: ['123', '234'],
           shouldBeDestroyed: true,
-        },
+        }),
       ];
 
       const actual = mapToScopesViewModel(input);
@@ -85,6 +85,66 @@ describe('feature flags helpers spec', () => {
       expect(mapToScopesViewModel(null)).toEqual([]);
       expect(mapToScopesViewModel(undefined)).toEqual([]);
     });
+
+    describe('with user IDs per environment', () => {
+      let oldGon;
+
+      beforeEach(() => {
+        oldGon = window.gon;
+        window.gon = { features: { featureFlagsUsersPerEnvironment: true } };
+      });
+
+      afterEach(() => {
+        window.gon = oldGon;
+      });
+
+      it('sets the user IDs as a comma separated string', () => {
+        const input = [
+          {
+            id: 3,
+            environment_scope: 'environment_scope',
+            active: true,
+            can_update: true,
+            protected: true,
+            strategies: [
+              {
+                name: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+                parameters: {
+                  percentage: '56',
+                },
+              },
+              {
+                name: ROLLOUT_STRATEGY_USER_ID,
+                parameters: {
+                  userIds: '123,234',
+                },
+              },
+            ],
+
+            _destroy: true,
+          },
+        ];
+
+        const expected = [
+          {
+            id: 3,
+            environmentScope: 'environment_scope',
+            active: true,
+            canUpdate: true,
+            protected: true,
+            rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+            rolloutPercentage: '56',
+            rolloutUserIds: '123, 234',
+            shouldBeDestroyed: true,
+            shouldIncludeUserIds: true,
+          },
+        ];
+
+        const actual = mapToScopesViewModel(input);
+
+        expect(actual).toEqual(expected);
+      });
+    });
   });
 
   describe('mapFromScopesViewModel', () => {
@@ -169,6 +229,75 @@ describe('feature flags helpers spec', () => {
 
       expect(actualScopes).toEqual([]);
     });
+    describe('with user IDs per environment', () => {
+      let oldGon;
+
+      beforeEach(() => {
+        oldGon = window.gon;
+        window.gon = { features: { featureFlagsUsersPerEnvironment: true } };
+      });
+
+      afterEach(() => {
+        window.gon = oldGon;
+      });
+
+      it('sets the user IDs as a comma separated string', () => {
+        const input = {
+          name: 'name',
+          description: 'description',
+          scopes: [
+            {
+              id: 4,
+              environmentScope: 'environmentScope',
+              active: true,
+              canUpdate: true,
+              protected: true,
+              shouldBeDestroyed: true,
+              rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+              rolloutPercentage: '48',
+              rolloutUserIds: '123, 234',
+              shouldIncludeUserIds: true,
+            },
+          ],
+        };
+
+        const expected = {
+          operations_feature_flag: {
+            name: 'name',
+            description: 'description',
+            scopes_attributes: [
+              {
+                id: 4,
+                environment_scope: 'environmentScope',
+                active: true,
+                can_update: true,
+                protected: true,
+                _destroy: true,
+                strategies: [
+                  {
+                    name: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+                    parameters: {
+                      groupId: PERCENT_ROLLOUT_GROUP_ID,
+                      percentage: '48',
+                    },
+                  },
+                  {
+                    name: ROLLOUT_STRATEGY_USER_ID,
+                    parameters: {
+                      userIds: '123,234',
+                    },
+                  },
+                ],
+              },
+            ],
+          },
+        };
+
+        const actual = mapFromScopesViewModel(input);
+
+        expect(actual).toEqual(expected);
+      });
+    });
   });
 
   describe('createNewEnvironmentScope', () => {

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

 import _ from 'underscore';
 import { createLocalVue, mount } from '@vue/test-utils';
+import { GlFormTextarea, GlFormCheckbox } 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';
@@ -13,12 +14,22 @@ import { featureFlag } from '../mock_data';
 
 describe('feature flag form', () => {
   let wrapper;
+  let oldGon;
   const requiredProps = {
     cancelPath: 'feature_flags',
     submitText: 'Create',
     environmentsEndpoint: '/environments.json',
   };
 
+  beforeEach(() => {
+    oldGon = window.gon;
+    window.gon = { features: { featureFlagsUsersPerEnvironment: true } };
+  });
+
+  afterEach(() => {
+    window.gon = oldGon;
+  });
+
   const factory = (props = {}) => {
     const localVue = createLocalVue();
 
@@ -26,7 +37,7 @@ describe('feature flag form', () => {
       localVue,
       propsData: props,
       provide: {
-        glFeatures: { featureFlagPermissions: true },
+        glFeatures: { featureFlagPermissions: true, featureFlagsUsersPerEnvironment: true },
       },
       sync: false,
     });
@@ -101,6 +112,8 @@ describe('feature flag form', () => {
             protected: false,
             rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
             rolloutPercentage: '54',
+            rolloutUserIds: '123',
+            shouldIncludeUserIds: true,
           },
           {
             id: 2,
@@ -110,6 +123,8 @@ describe('feature flag form', () => {
             protected: true,
             rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
             rolloutPercentage: '54',
+            rolloutUserIds: '123',
+            shouldIncludeUserIds: true,
           },
         ],
       });
@@ -124,6 +139,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 +272,7 @@ describe('feature flag form', () => {
               active: false,
               rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
               rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-              rolloutUserIds: [],
+              rolloutUserIds: '',
             },
           ],
         });
@@ -301,7 +326,7 @@ describe('feature flag form', () => {
                 protected: true,
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: '55',
-                rolloutUserIds: [],
+                rolloutUserIds: '',
               },
               {
                 id: jasmine.any(String),
@@ -311,7 +336,7 @@ describe('feature flag form', () => {
                 protected: false,
                 rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
                 rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-                rolloutUserIds: [],
+                rolloutUserIds: '',
               },
               {
                 id: jasmine.any(String),
@@ -321,7 +346,7 @@ describe('feature flag form', () => {
                 protected: false,
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-                rolloutUserIds: [],
+                rolloutUserIds: '',
               },
             ]);
           })
@@ -330,87 +355,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([]);
-    });
-  });
 });

locale/gitlab.pot

@@ -7348,6 +7348,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 ""