Merge branch 'create-userwithid-new-version-flags' into 'master'

Map Feature Flag userWithId Strategy Parameters Between BE and FE

See merge request gitlab-org/gitlab!29193

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

@@ -69,7 +69,7 @@ export default {
 };
 </script>
 <template>
-  <gl-new-dropdown>
+  <gl-new-dropdown class="js-new-environments-dropdown">
     <template #button-content>
       <span class="d-md-none mr-1">
         {{ $options.translations.addEnvironmentsLabel }}

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

@@ -161,16 +161,30 @@ const mapStrategyScopesToView = scopes =>
     environmentScope: s.environment_scope,
   }));
 
+const mapStrategiesParametersToViewModel = params => {
+  if (params.userIds) {
+    return { ...params, userIds: params.userIds.split(',').join(', ') };
+  }
+  return params;
+};
+
 export const mapStrategiesToViewModel = strategiesFromRails =>
   (strategiesFromRails || []).map(s => ({
     id: s.id,
     name: s.name,
-    parameters: s.parameters,
+    parameters: mapStrategiesParametersToViewModel(s.parameters),
     // eslint-disable-next-line no-underscore-dangle
     shouldBeDestroyed: Boolean(s._destroy),
     scopes: mapStrategyScopesToView(s.scopes),
   }));
 
+const mapStrategiesParametersToRails = params => {
+  if (params.userIds) {
+    return { ...params, userIds: params.userIds.split(', ').join(',') };
+  }
+  return params;
+};
+
 export const mapStrategiesToRails = params => ({
   operations_feature_flag: {
     name: params.name,
@@ -179,7 +193,7 @@ export const mapStrategiesToRails = params => ({
     strategies_attributes: (params.strategies || []).map(s => ({
       id: s.id,
       name: s.name,
-      parameters: s.parameters,
+      parameters: mapStrategiesParametersToRails(s.parameters),
       _destroy: s.shouldBeDestroyed,
       scopes_attributes: mapStrategyScopesToRails(s.scopes || []),
     })),

ee/spec/features/projects/feature_flags/user_creates_feature_flag_spec.rb

@@ -12,123 +12,144 @@ describe 'User creates feature flag', :js do
     project.add_developer(user)
     stub_licensed_features(feature_flags: true)
     stub_feature_flags(feature_flag_permissions: false)
-    stub_feature_flags(feature_flags_new_version: false)
     sign_in(user)
   end
 
-  context 'when creates without changing scopes' do
+  it 'user creates a flag enabled for user ids' do
+    visit(new_project_feature_flag_path(project))
+    set_feature_flag_info('test_feature', 'Test feature')
+    within_strategy_row(1) do
+      select 'User IDs', from: 'Type'
+      fill_in 'User IDs', with: 'user1, user2'
+      environment_plus_button.click
+      environment_search_input.set('production')
+      environment_search_results.first.click
+    end
+    click_button 'Create feature flag'
+
+    expect_user_to_see_feature_flags_index_page
+    expect(page).to have_text('test_feature')
+  end
+
+  context 'with new version flags disabled' do
     before do
-      visit(new_project_feature_flag_path(project))
-      set_feature_flag_info('ci_live_trace', 'For live trace')
-      click_button 'Create feature flag'
-      expect(page).to have_current_path(project_feature_flags_path(project))
+      stub_feature_flags(feature_flags_new_version: false)
     end
 
-    it 'shows the created feature flag' do
-      within_feature_flag_row(1) do
-        expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
-        expect(page).to have_css('.js-feature-flag-status button.is-checked')
+    context 'when creates without changing scopes' do
+      before do
+        visit(new_project_feature_flag_path(project))
+        set_feature_flag_info('ci_live_trace', 'For live trace')
+        click_button 'Create feature flag'
+        expect(page).to have_current_path(project_feature_flags_path(project))
+      end
 
-        within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(1)')).to have_content('*')
-          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+      it 'shows the created feature flag' do
+        within_feature_flag_row(1) do
+          expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
+          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+
+          within_feature_flag_scopes do
+            expect(page.find('.badge:nth-child(1)')).to have_content('*')
+            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+          end
         end
       end
-    end
 
-    it 'records audit event' do
-      visit(project_audit_events_path(project))
+      it 'records audit event' do
+        visit(project_audit_events_path(project))
 
-      expect(page).to have_text("Created feature flag ci_live_trace with description \"For live trace\".")
+        expect(page).to have_text("Created feature flag ci_live_trace with description \"For live trace\".")
+      end
     end
-  end
 
-  context 'when creates with disabling the default scope' do
-    before do
-      visit(new_project_feature_flag_path(project))
-      set_feature_flag_info('ci_live_trace', 'For live trace')
+    context 'when creates with disabling the default scope' do
+      before do
+        visit(new_project_feature_flag_path(project))
+        set_feature_flag_info('ci_live_trace', 'For live trace')
 
-      within_scope_row(1) do
-        within_status { find('.project-feature-toggle').click }
-      end
+        within_scope_row(1) do
+          within_status { find('.project-feature-toggle').click }
+        end
 
-      click_button 'Create feature flag'
-    end
+        click_button 'Create feature flag'
+      end
 
-    it 'shows the created feature flag' do
-      within_feature_flag_row(1) do
-        expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
-        expect(page).to have_css('.js-feature-flag-status button.is-checked')
+      it 'shows the created feature flag' do
+        within_feature_flag_row(1) do
+          expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
+          expect(page).to have_css('.js-feature-flag-status button.is-checked')
 
-        within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(1)')).to have_content('*')
-          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+          within_feature_flag_scopes do
+            expect(page.find('.badge:nth-child(1)')).to have_content('*')
+            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+          end
         end
       end
     end
-  end
 
-  context 'when creates with an additional scope' do
-    before do
-      visit(new_project_feature_flag_path(project))
-      set_feature_flag_info('mr_train', '')
+    context 'when creates with an additional scope' do
+      before do
+        visit(new_project_feature_flag_path(project))
+        set_feature_flag_info('mr_train', '')
 
-      within_scope_row(2) do
-        within_environment_spec do
-          find('.js-env-input').set("review/*")
-          find('.js-create-button').click
+        within_scope_row(2) do
+          within_environment_spec do
+            find('.js-env-input').set("review/*")
+            find('.js-create-button').click
+          end
         end
-      end
 
-      within_scope_row(2) do
-        within_status { find('.project-feature-toggle').click }
-      end
-
-      click_button 'Create feature flag'
-    end
+        within_scope_row(2) do
+          within_status { find('.project-feature-toggle').click }
+        end
 
-    it 'shows the created feature flag' do
-      within_feature_flag_row(1) do
-        expect(page.find('.feature-flag-name')).to have_content('mr_train')
-        expect(page).to have_css('.js-feature-flag-status button.is-checked')
+        click_button 'Create feature flag'
+      end
 
-        within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(1)')).to have_content('*')
-          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
-          expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
-          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active')
+      it 'shows the created feature flag' do
+        within_feature_flag_row(1) do
+          expect(page.find('.feature-flag-name')).to have_content('mr_train')
+          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+
+          within_feature_flag_scopes do
+            expect(page.find('.badge:nth-child(1)')).to have_content('*')
+            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+            expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
+            expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active')
+          end
         end
       end
     end
-  end
 
-  context 'when searches an environment name for scope creation' do
-    let!(:environment) { create(:environment, name: 'production', project: project) }
+    context 'when searches an environment name for scope creation' do
+      let!(:environment) { create(:environment, name: 'production', project: project) }
 
-    before do
-      visit(new_project_feature_flag_path(project))
-      set_feature_flag_info('mr_train', '')
+      before do
+        visit(new_project_feature_flag_path(project))
+        set_feature_flag_info('mr_train', '')
 
-      within_scope_row(2) do
-        within_environment_spec do
-          find('.js-env-input').set('prod')
-          click_button 'production'
+        within_scope_row(2) do
+          within_environment_spec do
+            find('.js-env-input').set('prod')
+            click_button 'production'
+          end
         end
-      end
 
-      click_button 'Create feature flag'
-    end
-
-    it 'shows the created feature flag' do
-      within_feature_flag_row(1) do
-        expect(page.find('.feature-flag-name')).to have_content('mr_train')
-        expect(page).to have_css('.js-feature-flag-status button.is-checked')
+        click_button 'Create feature flag'
+      end
 
-        within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(1)')).to have_content('*')
-          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
-          expect(page.find('.badge:nth-child(2)')).to have_content('production')
-          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+      it 'shows the created feature flag' do
+        within_feature_flag_row(1) do
+          expect(page.find('.feature-flag-name')).to have_content('mr_train')
+          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+
+          within_feature_flag_scopes do
+            expect(page.find('.badge:nth-child(1)')).to have_content('*')
+            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+            expect(page.find('.badge:nth-child(2)')).to have_content('production')
+            expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+          end
         end
       end
     end
@@ -140,4 +161,16 @@ describe 'User creates feature flag', :js do
     fill_in 'Name', with: name
     fill_in 'Description', with: description
   end
+
+  def environment_plus_button
+    find('.js-new-environments-dropdown')
+  end
+
+  def environment_search_input
+    find('.js-new-environments-dropdown input')
+  end
+
+  def environment_search_results
+    all('.js-new-environments-dropdown button.dropdown-item')
+  end
 end

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

@@ -378,7 +378,23 @@ describe('feature flags helpers spec', () => {
         },
       ]);
     });
+
+    it('inserts spaces between user ids', () => {
+      const strategy = _.first(
+        mapStrategiesToViewModel([
+          {
+            id: '1',
+            name: 'userWithId',
+            parameters: { userIds: 'user1,user2,user3' },
+            scopes: [],
+          },
+        ]),
+      );
+
+      expect(strategy.parameters).toEqual({ userIds: 'user1, user2, user3' });
+    });
   });
+
   describe('mapStrategiesToRails', () => {
     it('should map rails casing to view model casing', () => {
       expect(
@@ -425,6 +441,7 @@ describe('feature flags helpers spec', () => {
         },
       });
     });
+
     it('should insert a default * scope if there are none', () => {
       expect(
         mapStrategiesToRails({
@@ -460,5 +477,24 @@ describe('feature flags helpers spec', () => {
         },
       });
     });
+
+    it('removes white space between user ids', () => {
+      const result = mapStrategiesToRails({
+        name: 'test',
+        version: NEW_VERSION_FLAG,
+        strategies: [
+          {
+            id: '1',
+            name: 'userWithId',
+            parameters: { userIds: 'user1, user2, user3' },
+            scopes: [],
+          },
+        ],
+      });
+
+      const strategyAttrs = _.first(result.operations_feature_flag.strategies_attributes);
+
+      expect(strategyAttrs.parameters).toEqual({ userIds: 'user1,user2,user3' });
+    });
   });
 });

ee/spec/support/helpers/feature_flag_helpers.rb

@@ -32,6 +32,12 @@ module FeatureFlagHelpers
     end
   end
 
+  def within_strategy_row(index)
+    within ".feature-flags-form > fieldset > div:nth-child(#{index + 3})" do
+      yield
+    end
+  end
+
   def within_environment_spec
     within '.table-section:nth-child(1)' do
       yield
@@ -49,4 +55,11 @@ module FeatureFlagHelpers
       yield
     end
   end
+
+  def expect_user_to_see_feature_flags_index_page
+    expect(page).to have_css('h3.page-title', text: 'Feature Flags')
+    expect(page).to have_text('All')
+    expect(page).to have_text('Enabled')
+    expect(page).to have_text('Disabled')
+  end
 end