From 768af6c68977968d5c31c52cb4f7cff59e54b45a Mon Sep 17 00:00:00 2001
From: Anna Vovchenko <avovchenko@gitlab.com>
Date: Mon, 1 Sep 2025 14:16:40 +0200
Subject: [PATCH] Fix strategy input rerendering on name update

Unique key is triggering the component rerender.
Use id or index instead.

Changelog: fixed
---
 .../feature_flags/components/form.vue         | 24 ++++---
 .../feature_flags/components/form_spec.js     | 71 +++++++++----------
 2 files changed, 50 insertions(+), 45 deletions(-)

diff --git a/app/assets/javascripts/feature_flags/components/form.vue b/app/assets/javascripts/feature_flags/components/form.vue
index 709210e350e250..5bc08ed3a962f7 100644
--- a/app/assets/javascripts/feature_flags/components/form.vue
+++ b/app/assets/javascripts/feature_flags/components/form.vue
@@ -1,7 +1,7 @@
 <!-- eslint-disable vue/multi-word-component-names -->
 <script>
 import { GlButton, GlFormTextarea } from '@gitlab/ui';
-import { cloneDeep, isNumber, uniqueId } from 'lodash';
+import { cloneDeep, isNumber } from 'lodash';
 import { s__ } from '~/locale';
 import RelatedIssuesRoot from '~/related_issues/components/related_issues_root.vue';
 import featureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
@@ -96,12 +96,12 @@ export default {
     },
   },
   methods: {
-    keyFor(strategy) {
+    keyFor(strategy, index) {
       if (strategy.id) {
-        return strategy.id;
+        return `id_${strategy.id}`;
       }
 
-      return uniqueId('strategy_');
+      return `index_${index}`;
     },
 
     addStrategy() {
@@ -156,6 +156,7 @@ export default {
           <input
             id="feature-flag-name"
             v-model="formName"
+            data-testid="feature-flag-name"
             class="form-control"
             required
             aria-required="true"
@@ -170,6 +171,7 @@ export default {
           <gl-form-textarea
             id="feature-flag-description"
             v-model="formDescription"
+            data-testid="feature-flag-description"
             class="form-control"
             rows="4"
             aria-required="false"
@@ -188,7 +190,12 @@ export default {
           <h4>{{ s__('FeatureFlags|Strategies') }}</h4>
           <div class="gl-flex gl-items-baseline gl-justify-between">
             <p class="gl-mr-5">{{ $options.translations.newHelpText }}</p>
-            <gl-button variant="confirm" category="secondary" @click="addStrategy">
+            <gl-button
+              variant="confirm"
+              category="secondary"
+              data-testid="add-strategy"
+              @click="addStrategy"
+            >
               {{ s__('FeatureFlags|Add strategy') }}
             </gl-button>
           </div>
@@ -197,7 +204,7 @@ export default {
       <div v-if="filteredStrategies.length > 0" data-testid="feature-flag-strategies">
         <strategy
           v-for="(strategy, index) in filteredStrategies"
-          :key="keyFor(strategy)"
+          :key="keyFor(strategy, index)"
           :strategy="strategy"
           :index="index"
           @change="onFormStrategyChange($event, index)"
@@ -214,11 +221,12 @@ export default {
         ref="submitButton"
         type="button"
         variant="confirm"
-        class="js-ff-submit gl-mr-2"
+        class="gl-mr-2"
+        data-testid="ff-submit"
         @click="handleSubmit"
         >{{ submitText }}</gl-button
       >
-      <gl-button :href="cancelPath" class="js-ff-cancel">
+      <gl-button :href="cancelPath" data-testid="ff-cancel">
         {{ __('Cancel') }}
       </gl-button>
     </div>
diff --git a/spec/frontend/feature_flags/components/form_spec.js b/spec/frontend/feature_flags/components/form_spec.js
index 68ed95e889ede9..1b64a9addbda38 100644
--- a/spec/frontend/feature_flags/components/form_spec.js
+++ b/spec/frontend/feature_flags/components/form_spec.js
@@ -1,4 +1,3 @@
-import { GlButton } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import { nextTick } from 'vue';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
@@ -26,6 +25,12 @@ describe('feature flag form', () => {
     projectId: '1',
   };
 
+  const mockStrategy = {
+    type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+    parameters: { percentage: '30' },
+    scopes: [],
+  };
+
   const factory = (props = {}, provide = {}) => {
     wrapper = extendedWrapper(
       shallowMount(Form, {
@@ -38,6 +43,15 @@ describe('feature flag form', () => {
     );
   };
 
+  const findSubmitButton = () => wrapper.findByTestId('ff-submit');
+  const findCancelButton = () => wrapper.findByTestId('ff-cancel');
+  const findAddStrategyButton = () => wrapper.findByTestId('add-strategy');
+  const findRelatedIssuesRoot = () => wrapper.findComponent(RelatedIssuesRoot);
+  const findNameInput = () => wrapper.findByTestId('feature-flag-name');
+  const findDescriptionInput = () => wrapper.findByTestId('feature-flag-description');
+  const findStrategies = () => wrapper.findAllComponents(Strategy);
+  const findFirstStrategy = () => findStrategies().at(0);
+
   beforeEach(() => {
     Api.fetchFeatureFlagUserLists.mockResolvedValue({ data: [] });
   });
@@ -45,19 +59,19 @@ describe('feature flag form', () => {
   it('should render provided submitText', () => {
     factory(requiredProps);
 
-    expect(wrapper.find('.js-ff-submit').text()).toEqual(requiredProps.submitText);
+    expect(findSubmitButton().text()).toEqual(requiredProps.submitText);
   });
 
   it('should render provided cancelPath', () => {
     factory(requiredProps);
 
-    expect(wrapper.find('.js-ff-cancel').attributes('href')).toEqual(requiredProps.cancelPath);
+    expect(findCancelButton().attributes('href')).toEqual(requiredProps.cancelPath);
   });
 
   it('does not render the related issues widget without the featureFlagIssuesEndpoint', () => {
     factory(requiredProps);
 
-    expect(wrapper.findComponent(RelatedIssuesRoot).exists()).toBe(false);
+    expect(findRelatedIssuesRoot().exists()).toBe(false);
   });
 
   it('renders the related issues widget when the featureFlagIssuesEndpoint is provided', () => {
@@ -69,7 +83,7 @@ describe('feature flag form', () => {
       },
     );
 
-    expect(wrapper.findComponent(RelatedIssuesRoot).exists()).toBe(true);
+    expect(findRelatedIssuesRoot().exists()).toBe(true);
   });
 
   describe('without provided data', () => {
@@ -78,11 +92,11 @@ describe('feature flag form', () => {
     });
 
     it('should render name input text', () => {
-      expect(wrapper.find('#feature-flag-name').exists()).toBe(true);
+      expect(findNameInput().exists()).toBe(true);
     });
 
     it('should render description textarea', () => {
-      expect(wrapper.find('#feature-flag-description').exists()).toBe(true);
+      expect(findDescriptionInput().exists()).toBe(true);
     });
   });
 
@@ -95,11 +109,7 @@ describe('feature flag form', () => {
         description: featureFlag.description,
         active: true,
         strategies: [
-          {
-            type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-            parameters: { percentage: '30' },
-            scopes: [],
-          },
+          mockStrategy,
           {
             type: ROLLOUT_STRATEGY_ALL_USERS,
             parameters: {},
@@ -110,39 +120,28 @@ describe('feature flag form', () => {
     });
 
     it('should show the strategy component', () => {
-      const strategy = wrapper.findComponent(Strategy);
-      expect(strategy.exists()).toBe(true);
-      expect(strategy.props('strategy')).toEqual({
-        type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-        parameters: { percentage: '30' },
-        scopes: [],
-      });
+      expect(findFirstStrategy().exists()).toBe(true);
+      expect(findFirstStrategy().props('strategy')).toEqual(mockStrategy);
     });
 
     it('should show one strategy component per strategy', () => {
-      expect(wrapper.findAllComponents(Strategy)).toHaveLength(2);
+      expect(findStrategies()).toHaveLength(2);
     });
 
     it('adds an all users strategy when clicking the Add button', async () => {
-      wrapper.findComponent(GlButton).vm.$emit('click');
+      findAddStrategyButton().vm.$emit('click');
 
       await nextTick();
-      const strategies = wrapper.findAllComponents(Strategy);
 
-      expect(strategies).toHaveLength(3);
-      expect(strategies.at(2).props('strategy')).toEqual(allUsersStrategy);
+      expect(findStrategies()).toHaveLength(3);
+      expect(findStrategies().at(2).props('strategy')).toEqual(allUsersStrategy);
     });
 
     it('should remove a strategy on delete', async () => {
-      const strategy = {
-        type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-        parameters: { percentage: '30' },
-        scopes: [],
-      };
-      wrapper.findComponent(Strategy).vm.$emit('delete');
+      findFirstStrategy().vm.$emit('delete');
       await nextTick();
-      expect(wrapper.findAllComponents(Strategy)).toHaveLength(1);
-      expect(wrapper.findComponent(Strategy).props('strategy')).not.toEqual(strategy);
+      expect(findStrategies()).toHaveLength(1);
+      expect(findFirstStrategy().props('strategy')).not.toEqual(mockStrategy);
     });
 
     it('should remove a strategy on delete with numbered id', async () => {
@@ -150,19 +149,17 @@ describe('feature flag form', () => {
         strategies: [
           {
             shouldBeDestroyed: false,
-            type: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-            parameters: { percentage: '30' },
-            scopes: [],
+            ...mockStrategy,
             id: 1,
           },
         ],
       });
 
-      wrapper.findComponent(Strategy).vm.$emit('delete');
+      findFirstStrategy().vm.$emit('delete');
 
       await nextTick();
 
-      expect(wrapper.findAllComponents(Strategy)).toHaveLength(0);
+      expect(findStrategies()).toHaveLength(0);
     });
   });
 });
-- 
GitLab