Merge branch 'new-ff-delete-strategy' into 'master'

Delete Correct Strategy When Removed From New Feature Flag

Closes #222940

See merge request gitlab-org/gitlab!34889

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

 <script>
 import Vue from 'vue';
-import { memoize, isString, cloneDeep, isNumber } from 'lodash';
+import { memoize, isString, cloneDeep, isNumber, uniqueId } from 'lodash';
 import {
   GlDeprecatedButton,
   GlDeprecatedBadge as GlBadge,
@@ -143,7 +143,6 @@ export default {
     supportsStrategies() {
       return this.glFeatures.featureFlagsNewVersion && this.version === NEW_VERSION_FLAG;
     },
-
     canDeleteStrategy() {
       return this.formStrategies.length > 1;
     },
@@ -160,6 +159,14 @@ export default {
     }
   },
   methods: {
+    keyFor(strategy) {
+      if (strategy.id) {
+        return strategy.id;
+      }
+
+      return uniqueId('strategy_');
+    },
+
     addStrategy() {
       this.formStrategies.push({ name: '', parameters: {}, scopes: [] });
     },
@@ -321,7 +328,7 @@ export default {
         <div v-if="filteredStrategies.length > 0" data-testid="feature-flag-strategies">
           <strategy
             v-for="(strategy, index) in filteredStrategies"
-            :key="strategy.id"
+            :key="keyFor(strategy)"
             :strategy="strategy"
             :index="index"
             :endpoint="environmentsEndpoint"

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

@@ -273,7 +273,12 @@ export default {
       </div>
 
       <div class="align-self-end align-self-md-stretch order-first offset-md-0 order-md-0 ml-auto">
-        <gl-deprecated-button v-if="canDelete" variant="danger" @click="$emit('delete')">
+        <gl-deprecated-button
+          v-if="canDelete"
+          data-testid="delete-strategy-button"
+          variant="danger"
+          @click="$emit('delete')"
+        >
           <span class="d-md-none">
             {{ $options.translations.removeLabel }}
           </span>

ee/changelogs/unreleased/new-ff-delete-strategy.yml

+---
+title: Remove correct feature flag strategy when removing strategy that is not persisted
+merge_request: 34889
+author:
+type: fixed

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

@@ -50,6 +50,24 @@ RSpec.describe 'User creates feature flag', :js do
     end
   end
 
+  it 'removes the correct strategy when a strategy is deleted' do
+    visit(new_project_feature_flag_path(project))
+    click_button 'Add strategy'
+    within_strategy_row(1) do
+      select 'All users', from: 'Type'
+    end
+    within_strategy_row(2) do
+      select 'Percent rollout (logged in users)', from: 'Type'
+    end
+    within_strategy_row(1) do
+      delete_strategy_button.click
+    end
+
+    within_strategy_row(1) do
+      expect(page).to have_select('Type', selected: 'Percent rollout (logged in users)')
+    end
+  end
+
   context 'with new version flags disabled' do
     before do
       stub_feature_flags(feature_flags_new_version: false)

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

@@ -406,12 +406,12 @@ describe('feature flag form', () => {
         strategies: [
           {
             type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-            paramters: { percentage: '30' },
+            parameters: { percentage: '30' },
             scopes: [],
           },
           {
             type: ROLLOUT_STRATEGY_ALL_USERS,
-            paramters: {},
+            parameters: {},
             scopes: [{ environment_scope: 'review/*' }],
           },
         ],
@@ -429,7 +429,7 @@ describe('feature flag form', () => {
       expect(strategy.exists()).toBe(true);
       expect(strategy.props('strategy')).toEqual({
         type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-        paramters: { percentage: '30' },
+        parameters: { percentage: '30' },
         scopes: [],
       });
     });
@@ -446,7 +446,7 @@ describe('feature flag form', () => {
     it('should remove a strategy on delete', () => {
       const strategy = {
         type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-        paramters: { percentage: '30' },
+        parameters: { percentage: '30' },
         scopes: [],
       };
       wrapper.find(Strategy).vm.$emit('delete');

ee/spec/support/helpers/feature_flag_helpers.rb

@@ -60,6 +60,10 @@ module FeatureFlagHelpers
     find('.js-feature-flag-edit-button')
   end
 
+  def delete_strategy_button
+    find("button[data-testid='delete-strategy-button']")
+  end
+
   def status_toggle_button
     find('.js-feature-flag-status button')
   end