Merge branch 'clear-users-on-strategy-change' into 'master'

Ensure User IDs are Removed When Moving to All Users Strategy

See merge request gitlab-org/gitlab!25190

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

@@ -202,6 +202,12 @@ export default {
     shouldDisplayUserIds(scope) {
       return scope.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID || scope.shouldIncludeUserIds;
     },
+    onStrategyChange(index) {
+      const scope = this.filteredScopes[index];
+      scope.shouldIncludeUserIds =
+        scope.rolloutUserIds.length > 0 &&
+        scope.rolloutStrategy === ROLLOUT_STRATEGY_PERCENT_ROLLOUT;
+    },
   },
 };
 </script>
@@ -256,6 +262,7 @@ export default {
             <div
               v-for="(scope, index) in filteredScopes"
               :key="scope.id"
+              ref="scopeRow"
               class="gl-responsive-table-row"
               role="row"
             >
@@ -281,9 +288,9 @@ export default {
                     @clearInput="env => (scope.environmentScope = '')"
                   />
 
-                  <gl-badge v-if="permissionsFlag && scope.protected" variant="success">{{
-                    s__('FeatureFlags|Protected')
-                  }}</gl-badge>
+                  <gl-badge v-if="permissionsFlag && scope.protected" variant="success">
+                    {{ s__('FeatureFlags|Protected') }}
+                  </gl-badge>
                 </div>
               </div>
 
@@ -314,6 +321,7 @@ export default {
                       v-model="scope.rolloutStrategy"
                       :disabled="!scope.active"
                       class="form-control select-control w-100 js-rollout-strategy"
+                      @change="onStrategyChange(index)"
                     >
                       <option :value="$options.ROLLOUT_STRATEGY_ALL_USERS">
                         {{ s__('FeatureFlags|All users') }}
@@ -433,9 +441,9 @@ export default {
                   {{ s__('FeatureFlags|Rollout Strategy') }}
                 </div>
                 <div class="table-mobile-content js-rollout-strategy form-inline">
-                  <label class="sr-only" for="new-rollout-strategy-placeholder">
-                    {{ s__('FeatureFlags|Rollout Strategy') }}
-                  </label>
+                  <label class="sr-only" for="new-rollout-strategy-placeholder">{{
+                    s__('FeatureFlags|Rollout Strategy')
+                  }}</label>
                   <div class="select-wrapper col-12 col-md-8 p-0">
                     <select
                       id="new-rollout-strategy-placeholder"

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

@@ -49,7 +49,7 @@ export const mapToScopesViewModel = scopesFromRails =>
 
       // eslint-disable-next-line no-underscore-dangle
       shouldBeDestroyed: Boolean(s._destroy),
-      shouldIncludeUserIds: rolloutUserIds.length > 0,
+      shouldIncludeUserIds: rolloutUserIds.length > 0 && percentStrategy !== null,
     };
   });
 /**

ee/changelogs/unreleased/clear-users-on-strategy-change.yml

+---
+title: Ensure User IDs are Removed When Moving to All Users Strategy
+merge_request: 25190
+author:
+type: fixed

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

@@ -14,29 +14,18 @@ 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 = {}) => {
     wrapper = shallowMount(Form, {
       propsData: props,
       provide: {
         glFeatures: {
           featureFlagPermissions: true,
-          featureFlagsUsersPerEnvironment: true,
         },
       },
     });
@@ -174,6 +163,15 @@ describe('feature flag form', () => {
             });
           });
         });
+        describe('on strategy change', () => {
+          it('should not include user IDs if All Users is selected', () => {
+            const scope = wrapper.find({ ref: 'scopeRow' });
+            scope.find('select').setValue(ROLLOUT_STRATEGY_ALL_USERS);
+            return wrapper.vm.$nextTick().then(() => {
+              expect(scope.find('#rollout-user-id-0').exists()).toBe(false);
+            });
+          });
+        });
       });
 
       describe('deleting an existing scope', () => {
@@ -354,6 +352,7 @@ describe('feature flag form', () => {
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: '55',
                 rolloutUserIds: '',
+                shouldIncludeUserIds: false,
               },
               {
                 id: expect.any(String),
@@ -374,6 +373,7 @@ describe('feature flag form', () => {
                 rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
                 rolloutUserIds: '',
+                shouldIncludeUserIds: false,
               },
             ]);
           });