Merge branch 'revert-a065afb2' into 'master'

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

See merge request gitlab-org/gitlab!20084

changelogs/unreleased/ff-user-ids-per-scope-fe.yml

----
-title: Make User IDs work per scope in Feature Flags
-merge_request: 19399
-author:
-type: added

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

 <script>
 import Vue from 'vue';
 import _ from 'underscore';
-import {
-  GlButton,
-  GlBadge,
-  GlTooltip,
-  GlTooltipDirective,
-  GlFormTextarea,
-  GlFormCheckbox,
-} from '@gitlab/ui';
+import { GlButton, GlBadge, GlTooltip, GlTooltipDirective } 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';
@@ -17,7 +10,6 @@ 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';
@@ -28,8 +20,6 @@ export default {
   components: {
     GlButton,
     GlBadge,
-    GlFormCheckbox,
-    GlFormTextarea,
     GlTooltip,
     ToggleButton,
     Icon,
@@ -87,7 +77,6 @@ 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$/,
@@ -113,6 +102,11 @@ export default {
     permissionsFlag() {
       return this.glFeatures.featureFlagPermissions;
     },
+
+    userIds() {
+      const scope = this.formScopes.find(s => Array.isArray(s.rolloutUserIds)) || {};
+      return scope.rolloutUserIds || [];
+    },
   },
   methods: {
     isAllEnvironment(name) {
@@ -160,9 +154,18 @@ 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);
     }),
@@ -184,17 +187,6 @@ 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>
@@ -249,7 +241,7 @@ export default {
             <div
               v-for="(scope, index) in filteredScopes"
               :key="scope.id"
-              class="gl-responsive-table-row align-items-start"
+              class="gl-responsive-table-row"
               role="row"
             >
               <div class="table-section section-30" role="gridcell">
@@ -259,7 +251,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 mb-0">
+                  <p v-if="isAllEnvironment(scope.environmentScope)" class="js-scope-all pl-3">
                     {{ $options.allEnvironmentsText }}
                   </p>
 
@@ -293,7 +285,7 @@ export default {
                 </div>
               </div>
 
-              <div class="table-section section-40 align-items-start" role="gridcell">
+              <div class="table-section section-40" role="gridcell">
                 <div class="table-mobile-header" role="rowheader">
                   {{ s__('FeatureFlags|Rollout Strategy') }}
                 </div>
@@ -308,15 +300,12 @@ 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_USER_ID">
-                        {{ s__('FeatureFlags|User IDs') }}
-                      </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>
                     </select>
                     <i aria-hidden="true" data-hidden="true" class="fa fa-chevron-down"></i>
                   </div>
@@ -353,24 +342,6 @@ 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>
 
@@ -442,6 +413,7 @@ export default {
         </div>
       </div>
     </fieldset>
+    <user-with-id :value="userIds" @input="updateUserIds" />
 
     <div class="form-actions">
       <gl-button

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

@@ -29,10 +29,7 @@ export const mapToScopesViewModel = scopesFromRails =>
       strat => strat.name === ROLLOUT_STRATEGY_USER_ID,
     );
 
-    const rolloutUserIds = (fetchUserIdParams(userStrategy) || '')
-      .split(',')
-      .filter(id => id)
-      .join(', ');
+    const rolloutUserIds = (fetchUserIdParams(userStrategy) || '').split(',').filter(id => id);
 
     return {
       id: s.id,
@@ -46,7 +43,6 @@ export const mapToScopesViewModel = scopesFromRails =>
 
       // eslint-disable-next-line no-underscore-dangle
       shouldBeDestroyed: Boolean(s._destroy),
-      shouldIncludeUserIds: rolloutUserIds.length > 0,
     };
   });
 /**
@@ -63,8 +59,8 @@ export const mapFromScopesViewModel = params => {
     }
 
     const userIdParameters = {};
-    if (s.shouldIncludeUserIds || s.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID) {
-      userIdParameters.userIds = (s.rolloutUserIds || '').replace(/, /g, ',');
+    if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) {
+      userIdParameters.userIds = s.rolloutUserIds.join(',');
     }
 
     // Strip out any internal IDs
@@ -117,7 +113,7 @@ export const createNewEnvironmentScope = (overrides = {}, featureFlagPermissions
     id: _.uniqueId(INTERNAL_ID_PREFIX),
     rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
     rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-    rolloutUserIds: '',
+    rolloutUserIds: [],
   };
 
   const newScope = {

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

@@ -51,9 +51,8 @@ describe('feature flags helpers spec', () => {
           protected: true,
           rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
           rolloutPercentage: '56',
-          rolloutUserIds: '123, 234',
+          rolloutUserIds: ['123', '234'],
           shouldBeDestroyed: true,
-          shouldIncludeUserIds: true,
         },
       ];
 
@@ -103,8 +102,7 @@ describe('feature flags helpers spec', () => {
             shouldBeDestroyed: true,
             rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
             rolloutPercentage: '48',
-            rolloutUserIds: '123, 234',
-            shouldIncludeUserIds: true,
+            rolloutUserIds: ['123', '234'],
           },
         ],
       };
@@ -181,7 +179,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();
@@ -201,7 +199,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);

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

 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';
@@ -102,8 +101,6 @@ describe('feature flag form', () => {
             protected: false,
             rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
             rolloutPercentage: '54',
-            rolloutUserIds: '123',
-            shouldIncludeUserIds: true,
           },
           {
             id: 2,
@@ -113,8 +110,6 @@ describe('feature flag form', () => {
             protected: true,
             rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
             rolloutPercentage: '54',
-            rolloutUserIds: '123',
-            shouldIncludeUserIds: true,
           },
         ],
       });
@@ -129,16 +124,6 @@ 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', () => {
@@ -262,7 +247,7 @@ describe('feature flag form', () => {
               active: false,
               rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
               rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-              rolloutUserIds: '',
+              rolloutUserIds: [],
             },
           ],
         });
@@ -316,7 +301,7 @@ describe('feature flag form', () => {
                 protected: true,
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: '55',
-                rolloutUserIds: '',
+                rolloutUserIds: [],
               },
               {
                 id: jasmine.any(String),
@@ -326,7 +311,7 @@ describe('feature flag form', () => {
                 protected: false,
                 rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
                 rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-                rolloutUserIds: '',
+                rolloutUserIds: [],
               },
               {
                 id: jasmine.any(String),
@@ -336,7 +321,7 @@ describe('feature flag form', () => {
                 protected: false,
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-                rolloutUserIds: '',
+                rolloutUserIds: [],
               },
             ]);
           })
@@ -345,4 +330,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,7 +66,7 @@ describe('New feature flag form', () => {
         active: true,
         rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
         rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-        rolloutUserIds: '',
+        rolloutUserIds: [],
       },
     ]);
 

locale/gitlab.pot

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