Merge branch 'userwithid-per-env-backend' into 'master'

Rollout UserWithId Per Environment Spec

See merge request gitlab-org/gitlab!20325

changelogs/unreleased/userwithid-per-env-backend.yml

+---
+title: Add feature to allow specifying userWithId strategies per environment spec
+merge_request: 20325
+author:
+type: added

db/post_migrate/20191118211629_migrate_ops_feature_flags_scopes_target_user_ids.rb

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

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

@@ -22,7 +22,6 @@ import {
   INTERNAL_ID_PREFIX,
 } from '../constants';
 import { createNewEnvironmentScope } from '../store/modules/helpers';
-import UserWithId from './strategies/user_with_id.vue';
 
 export default {
   components: {
@@ -34,7 +33,6 @@ export default {
     ToggleButton,
     Icon,
     EnvironmentsDropdown,
-    UserWithId,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -92,6 +90,7 @@ 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$/,
@@ -117,14 +116,6 @@ export default {
     permissionsFlag() {
       return this.glFeatures.featureFlagPermissions;
     },
-
-    userIds() {
-      const scope = this.formScopes.find(s => Array.isArray(s.rolloutUserIds)) || {};
-      return scope.rolloutUserIds || [];
-    },
-    shouldShowUsersPerEnvironment() {
-      return this.glFeatures.featureFlagsUsersPerEnvironment;
-    },
   },
   methods: {
     isAllEnvironment(name) {
@@ -174,13 +165,6 @@ export default {
       });
     },
 
-    updateUserIds(userIds) {
-      this.formScopes = this.formScopes.map(s => ({
-        ...s,
-        rolloutUserIds: userIds,
-      }));
-    },
-
     canUpdateScope(scope) {
       return !this.permissionsFlag || scope.canUpdate;
     },
@@ -337,10 +321,7 @@ export default {
                       <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"
-                      >
+                      <option :value="$options.ROLLOUT_STRATEGY_USER_ID">
                         {{ s__('FeatureFlags|User IDs') }}
                       </option>
                     </select>
@@ -379,10 +360,7 @@ 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"
-                  >
+                  <div class="d-flex flex-column align-items-start mt-2 w-100">
                     <gl-form-checkbox
                       v-if="shouldDisplayIncludeUserIds(scope)"
                       v-model="scope.shouldIncludeUserIds"
@@ -476,8 +454,6 @@ export default {
       </div>
     </fieldset>
 
-    <user-with-id v-if="!shouldShowUsersPerEnvironment" :value="userIds" @input="updateUserIds" />
-
     <div class="form-actions">
       <gl-button
         ref="submitButton"

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/store/modules/helpers.js

@@ -10,16 +10,6 @@ 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.
@@ -31,24 +21,21 @@ export const mapToScopesViewModel = scopesFromRails =>
       strat => strat.name === ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
     );
 
-    const rolloutStrategy = percentStrategy ? percentStrategy.name : ROLLOUT_STRATEGY_ALL_USERS;
-
     const rolloutPercentage = fetchPercentageParams(percentStrategy) || DEFAULT_PERCENT_ROLLOUT;
 
     const userStrategy = (s.strategies || []).find(
       strat => strat.name === ROLLOUT_STRATEGY_USER_ID,
     );
 
-    let rolloutUserIds = '';
+    const rolloutStrategy =
+      (percentStrategy && percentStrategy.name) ||
+      (userStrategy && userStrategy.name) ||
+      ROLLOUT_STRATEGY_ALL_USERS;
 
-    if (shouldShowUsersPerEnvironment()) {
-      rolloutUserIds = (fetchUserIdParams(userStrategy) || '')
-        .split(',')
-        .filter(id => id)
-        .join(', ');
-    } else {
-      rolloutUserIds = (fetchUserIdParams(userStrategy) || '').split(',').filter(id => id);
-    }
+    const rolloutUserIds = (fetchUserIdParams(userStrategy) || '')
+      .split(',')
+      .filter(id => id)
+      .join(', ');
 
     return {
       id: s.id,
@@ -80,9 +67,7 @@ export const mapFromScopesViewModel = params => {
 
     const userIdParameters = {};
 
-    const hasUsers = s.shouldIncludeUserIds || s.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID;
-
-    if (shouldShowUsersPerEnvironment() && hasUsers) {
+    if (s.shouldIncludeUserIds || s.rolloutStrategy === ROLLOUT_STRATEGY_USER_ID) {
       userIdParameters.userIds = (s.rolloutUserIds || '').replace(/, /g, ',');
     } else if (Array.isArray(s.rolloutUserIds) && s.rolloutUserIds.length > 0) {
       userIdParameters.userIds = s.rolloutUserIds.join(',');
@@ -141,7 +126,7 @@ export const createNewEnvironmentScope = (overrides = {}, featureFlagPermissions
     id: _.uniqueId(INTERNAL_ID_PREFIX),
     rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
     rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-    rolloutUserIds: shouldShowUsersPerEnvironment() ? '' : [],
+    rolloutUserIds: '',
   };
 
   const newScope = {

ee/app/models/operations/feature_flag_scope.rb

@@ -26,12 +26,6 @@ module Operations
     scope :enabled, -> { where(active: true) }
     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
       joins(:feature_flag)
         .select(FeatureFlag.arel_table[:name], FeatureFlag.arel_table[:description])

ee/lib/ee/api/entities.rb

@@ -783,22 +783,8 @@ module EE
       class UnleashFeature < Grape::Entity
         expose :name
         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.
-        # Each scope is given a userWithId strategy with the list of user ids.
-        # 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
+        expose :active, as: :enabled
+        expose :strategies
       end
 
       class GitlabSubscription < Grape::Entity

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

@@ -63,7 +63,7 @@ describe('New feature flag form', () => {
       active: true,
       rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
       rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-      rolloutUserIds: [],
+      rolloutUserIds: '',
     };
     expect(wrapper.vm.scopes).toEqual([defaultScope]);
 

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

@@ -51,7 +51,7 @@ describe('feature flags helpers spec', () => {
           protected: true,
           rolloutStrategy: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
           rolloutPercentage: '56',
-          rolloutUserIds: ['123', '234'],
+          rolloutUserIds: '123, 234',
           shouldBeDestroyed: true,
         }),
       ];
@@ -310,7 +310,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();
@@ -330,7 +330,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/requests/api/unleash_spec.rb

@@ -248,38 +248,16 @@ describe API::Unleash do
       end
 
       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" } }
 
-        context 'with userWithId strategy' do
-          let(:strategies) { [{ name: "default", parameters: {} }, { name: "userWithId", parameters: { userIds: "fred" } }] }
-
-          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' do
+          get api(features_url), headers: headers
 
-          it 'returns a disabled feature that does not contain a 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(false)
-            expect(feature_json['strategies']).to eq([{ 'name' => 'default', 'parameters' => {} }])
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          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

locale/gitlab.pot

@@ -7437,15 +7437,9 @@ msgstr ""
 msgid "FeatureFlags|Active"
 msgstr ""
 
-msgid "FeatureFlags|Add"
-msgstr ""
-
 msgid "FeatureFlags|All users"
 msgstr ""
 
-msgid "FeatureFlags|Clear all"
-msgstr ""
-
 msgid "FeatureFlags|Configure"
 msgstr ""
 
@@ -7467,9 +7461,6 @@ msgstr ""
 msgid "FeatureFlags|Edit Feature Flag"
 msgstr ""
 
-msgid "FeatureFlags|Enter comma separated list of user IDs"
-msgstr ""
-
 msgid "FeatureFlags|Environment Spec"
 msgstr ""
 
@@ -7551,15 +7542,9 @@ 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 ""
 

spec/migrations/migrate_ops_feature_flags_scopes_target_user_ids_spec.rb

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