Merge branch 'ff-sort-strategies-in-ui' into 'master'

Sort Feature Flag Strategies by ID for Display in UI

See merge request gitlab-org/gitlab!29629

ee/app/serializers/feature_flag_entity.rb

@@ -28,7 +28,9 @@ class FeatureFlagEntity < Grape::Entity
     feature_flag.scopes.sort_by(&:id)
   end
 
-  expose :strategies, with: FeatureFlags::StrategyEntity
+  expose :strategies, with: FeatureFlags::StrategyEntity do |feature_flag|
+    feature_flag.strategies.sort_by(&:id)
+  end
 
   private
 

ee/spec/controllers/projects/feature_flags_controller_spec.rb

@@ -358,6 +358,15 @@ describe Projects::FeatureFlagsController do
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
+
+      it 'returns strategies ordered by id' do
+        first_strategy = create(:operations_strategy, feature_flag: new_version_feature_flag)
+        second_strategy = create(:operations_strategy, feature_flag: new_version_feature_flag)
+
+        subject
+
+        expect(json_response['strategies'].map { |s| s['id'] }).to eq([first_strategy.id, second_strategy.id])
+      end
     end
   end
 

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

@@ -31,6 +31,25 @@ describe 'User creates feature flag', :js do
     expect(page).to have_text('test_feature')
   end
 
+  it 'user creates a flag with default environment scopes' do
+    visit(new_project_feature_flag_path(project))
+    set_feature_flag_info('test_flag', 'Test flag')
+    within_strategy_row(1) do
+      select 'All users', from: 'Type'
+    end
+    click_button 'Create feature flag'
+
+    expect_user_to_see_feature_flags_index_page
+    expect(page).to have_text('test_flag')
+
+    edit_feature_flag_button.click
+
+    within_strategy_row(1) do
+      expect(page).to have_text('All users')
+      expect(page).to have_text('All environments')
+    end
+  end
+
   context 'with new version flags disabled' do
     before do
       stub_feature_flags(feature_flags_new_version: false)

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

@@ -8,124 +8,166 @@ describe 'User updates feature flag', :js do
   let(:user) { create(:user) }
   let(:project) { create(:project, namespace: user.namespace) }
 
-  let!(:feature_flag) do
-    create_flag(project, 'ci_live_trace', true,
-                description: 'For live trace feature')
-  end
-
-  let!(:scope) { create_scope(feature_flag, 'review/*', true) }
-
   before do
     project.add_developer(user)
     stub_licensed_features(feature_flags: true)
     stub_feature_flags(feature_flag_permissions: false)
     sign_in(user)
-
-    visit(edit_project_feature_flag_path(project, feature_flag))
   end
 
-  it 'user sees persisted default scope' do
-    within_scope_row(1) do
-      within_environment_spec do
-        expect(page).to have_content('* (All Environments)')
+  context 'with a new version feature flag' do
+    let!(:feature_flag) do
+      create_flag(project, 'test_flag', true, version: Operations::FeatureFlag.versions['new_version_flag'],
+                  description: 'For testing')
+    end
+
+    let!(:strategy) do
+      create(:operations_strategy, feature_flag: feature_flag,
+             name: 'default', parameters: {})
+    end
+
+    let!(:scope) do
+      create(:operations_scope, strategy: strategy, environment_scope: '*')
+    end
+
+    it 'user adds a second strategy' do
+      visit(edit_project_feature_flag_path(project, feature_flag))
+      click_button 'Add strategy'
+      within_strategy_row(2) do
+        select 'Percent rollout (logged in users)', from: 'Type'
+        fill_in 'Percentage', with: '15'
       end
+      click_button 'Save changes'
+
+      edit_feature_flag_button.click
 
-      within_status do
-        expect(find('.project-feature-toggle')['aria-label'])
-          .to eq('Toggle Status: ON')
+      within_strategy_row(1) do
+        expect(page).to have_text 'All users'
+        expect(page).to have_text 'All environments'
+      end
+      within_strategy_row(2) do
+        expect(page).to have_text 'Percent rollout (logged in users)'
+        expect(page).to have_field 'Percentage', with: '15'
+        expect(page).to have_text 'All environments'
       end
     end
   end
 
-  context 'when user updates a status of a scope' do
-    before do
-      within_scope_row(2) do
-        within_status { find('.project-feature-toggle').click }
-      end
+  context 'with a legacy feature flag' do
+    let!(:feature_flag) do
+      create_flag(project, 'ci_live_trace', true,
+                  description: 'For live trace feature')
+    end
 
-      click_button 'Save changes'
-      expect(page).to have_current_path(project_feature_flags_path(project))
+    let!(:scope) { create_scope(feature_flag, 'review/*', true) }
+
+    before do
+      visit(edit_project_feature_flag_path(project, feature_flag))
     end
 
-    it 'shows the updated 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 'user sees persisted default scope' do
+      within_scope_row(1) do
+        within_environment_spec do
+          expect(page).to have_content('* (All Environments)')
+        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-inactive')
+        within_status do
+          expect(find('.project-feature-toggle')['aria-label'])
+            .to eq('Toggle Status: ON')
         end
       end
     end
 
-    it 'records audit event' do
-      visit(project_audit_events_path(project))
+    context 'when user updates a status of a scope' do
+      before do
+        within_scope_row(2) do
+          within_status { find('.project-feature-toggle').click }
+        end
 
-      expect(page).to(
-        have_text("Updated feature flag ci_live_trace. Updated rule review/* active state from true to false.")
-      )
-    end
-  end
+        click_button 'Save changes'
+        expect(page).to have_current_path(project_feature_flags_path(project))
+      end
 
-  context 'when user adds a new scope' do
-    before do
-      within_scope_row(3) do
-        within_environment_spec do
-          find('.js-env-input').set('production')
-          find('.js-create-button').click
+      it 'shows the updated 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')
+            expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
+            expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+          end
         end
       end
 
-      click_button 'Save changes'
-      expect(page).to have_current_path(project_feature_flags_path(project))
-    end
+      it 'records audit event' do
+        visit(project_audit_events_path(project))
 
-    it 'shows the newly created scope' do
-      within_feature_flag_row(1) do
-        within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(3)')).to have_content('production')
-          expect(page.find('.badge:nth-child(3)')['class']).to include('badge-inactive')
-        end
+        expect(page).to(
+          have_text("Updated feature flag ci_live_trace. Updated rule review/* active state from true to false.")
+        )
       end
     end
 
-    it 'records audit event' do
-      visit(project_audit_events_path(project))
+    context 'when user adds a new scope' do
+      before do
+        within_scope_row(3) do
+          within_environment_spec do
+            find('.js-env-input').set('production')
+            find('.js-create-button').click
+          end
+        end
 
-      expect(page).to(
-        have_text("Updated feature flag ci_live_trace")
-      )
-    end
-  end
+        click_button 'Save changes'
+        expect(page).to have_current_path(project_feature_flags_path(project))
+      end
 
-  context 'when user deletes a scope' do
-    before do
-      within_scope_row(2) do
-        within_delete { find('.js-delete-scope').click }
+      it 'shows the newly created scope' do
+        within_feature_flag_row(1) do
+          within_feature_flag_scopes do
+            expect(page.find('.badge:nth-child(3)')).to have_content('production')
+            expect(page.find('.badge:nth-child(3)')['class']).to include('badge-inactive')
+          end
+        end
       end
 
-      click_button 'Save changes'
-      expect(page).to have_current_path(project_feature_flags_path(project))
+      it 'records audit event' do
+        visit(project_audit_events_path(project))
+
+        expect(page).to(
+          have_text("Updated feature flag ci_live_trace")
+        )
+      end
     end
 
-    it 'shows the updated feature flag' do
-      within_feature_flag_row(1) do
-        within_feature_flag_scopes do
-          expect(page).to have_css('.badge:nth-child(1)')
-          expect(page).not_to have_css('.badge:nth-child(2)')
+    context 'when user deletes a scope' do
+      before do
+        within_scope_row(2) do
+          within_delete { find('.js-delete-scope').click }
         end
+
+        click_button 'Save changes'
+        expect(page).to have_current_path(project_feature_flags_path(project))
       end
-    end
 
-    it 'records audit event' do
-      visit(project_audit_events_path(project))
+      it 'shows the updated feature flag' do
+        within_feature_flag_row(1) do
+          within_feature_flag_scopes do
+            expect(page).to have_css('.badge:nth-child(1)')
+            expect(page).not_to have_css('.badge:nth-child(2)')
+          end
+        end
+      end
 
-      expect(page).to(
-        have_text("Updated feature flag ci_live_trace")
-      )
+      it 'records audit event' do
+        visit(project_audit_events_path(project))
+
+        expect(page).to(
+          have_text("Updated feature flag ci_live_trace")
+        )
+      end
     end
   end
 end

ee/spec/support/helpers/feature_flag_helpers.rb

 # frozen_string_literal: true
 
 module FeatureFlagHelpers
-  def create_flag(project, name, active = true, description: nil)
-    create(:operations_feature_flag, name: name, active: active,
+  def create_flag(project, name, active = true, description: nil, version: Operations::FeatureFlag.versions['legacy_flag'])
+    create(:operations_feature_flag, name: name, active: active, version: version,
                                      description: description, project: project)
   end
 
@@ -56,6 +56,10 @@ module FeatureFlagHelpers
     end
   end
 
+  def edit_feature_flag_button
+    find('.js-feature-flag-edit-button')
+  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')