Fixes broken form for feature flags

The edit/remove feature was updating the scopes
based on the array's index.

This commit creates an internal id and uses it
to update the scopes.

Removes the internal id before sending data
to the server

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

 <script>
+import _ from 'underscore';
 import { GlButton } from '@gitlab/ui';
 import { s__, sprintf } from '~/locale';
 import ToggleButton from '~/vue_shared/components/toggle_button.vue';
 import Icon from '~/vue_shared/components/icon.vue';
 import EnvironmentsDropdown from './environments_dropdown.vue';
+import { internalKeyID } from '../store/modules/helpers';
 
 export default {
   components: {
@@ -83,7 +85,8 @@ export default {
      * @param {Number} index
      * @param {Boolean} status
      */
-    onUpdateScopeStatus(scope, index, status) {
+    onUpdateScopeStatus(scope, status) {
+      const index = this.formScopes.findIndex(el => el.id === scope.id);
       this.formScopes.splice(index, 1, Object.assign({}, scope, { active: status }));
     },
     /**
@@ -94,7 +97,8 @@ export default {
      * @param {Object} scope
      * @param {Number} index
      */
-    updateScope(name, scope, index) {
+    updateScope(name, scope) {
+      const index = this.formScopes.findIndex(el => el.id === scope.id);
       this.formScopes.splice(index, 1, Object.assign({}, scope, { environment_scope: name }));
     },
     /**
@@ -107,6 +111,7 @@ export default {
       this.formScopes.push({
         active: value,
         environment_scope: this.newScope,
+        id: _.uniqueId(internalKeyID),
       });
 
       this.newScope = '';
@@ -121,13 +126,15 @@ export default {
      * @param {Number} index
      * @param {Object} scope
      */
-    removeScope(index, scope) {
-      if (scope.id) {
-        this.formScopes.splice(index, 1, Object.assign({}, scope, { _destroy: true }));
-      } else {
+    removeScope(scope) {
+      const index = this.formScopes.findIndex(el => el.id === scope.id);
+      if (_.isString(scope.id) && scope.id.indexOf(internalKeyID) !== -1) {
         this.formScopes.splice(index, 1);
+      } else {
+        this.formScopes.splice(index, 1, Object.assign({}, scope, { _destroy: true }));
       }
     },
+
     /**
      * When the user selects a value or creates a new value in the environments
      * dropdown in the creation row, we push a new entry with the selected value.
@@ -135,7 +142,11 @@ export default {
      * @param {String}
      */
     createNewEnvironment(name) {
-      this.formScopes.push({ environment_scope: name, active: false });
+      this.formScopes.push({
+        environment_scope: name,
+        active: false,
+        id: _.uniqueId(internalKeyID),
+      });
       this.newScope = '';
     },
     /**
@@ -225,7 +236,7 @@ export default {
                 <div class="table-mobile-content js-feature-flag-status">
                   <toggle-button
                     :value="scope.active"
-                    @change="status => onUpdateScopeStatus(scope, index, status)"
+                    @change="status => onUpdateScopeStatus(scope, status)"
                   />
                 </div>
               </div>
@@ -238,7 +249,7 @@ export default {
                   <gl-button
                     v-if="!isAllEnvironment(scope.environment_scope)"
                     class="js-delete-scope btn-transparent"
-                    @click="removeScope(index, scope)"
+                    @click="removeScope(scope)"
                   >
                     <icon name="clear" />
                   </gl-button>

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

-// eslint-disable-next-line import/prefer-default-export
+import _ from 'underscore';
+
+export const internalKeyID = 'internal_';
+
 export const parseFeatureFlagsParams = params => ({
   operations_feature_flag: {
     name: params.name,
     description: params.description,
-    scopes_attributes: params.scopes,
+    scopes_attributes: params.scopes.map(scope => {
+      const scopeCopy = Object.assign({}, scope);
+      if (_.isString(scopeCopy.id) && scopeCopy.id.indexOf(internalKeyID) !== -1) {
+        delete scopeCopy.id;
+      }
+      return scopeCopy;
+    }),
   },
 });

ee/changelogs/unreleased/10233-ff-form.yml

+---
+title: Fixes Broken new/edit feature flag form
+merge_request:
+author:
+type: fixed

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

+import _ from 'underscore';
 import { createLocalVue, mount } from '@vue/test-utils';
 import Form from 'ee/feature_flags/components/form.vue';
 import ToggleButton from '~/vue_shared/components/toggle_button.vue';
 import EnvironmentsDropdown from 'ee/feature_flags/components/environments_dropdown.vue';
+import { internalKeyID } from 'ee/feature_flags/store/modules/helpers';
 
 describe('feature flag form', () => {
   let wrapper;
@@ -64,7 +66,10 @@ describe('feature flag form', () => {
           it('should add a new scope with the text value empty and the status', () => {
             wrapper.find(ToggleButton).vm.$emit('change', true);
 
-            expect(wrapper.vm.formScopes).toEqual([{ active: true, environment_scope: '' }]);
+            expect(wrapper.vm.formScopes.length).toEqual(1);
+            expect(wrapper.vm.formScopes[0].active).toEqual(true);
+            expect(wrapper.vm.formScopes[0].environment_scope).toEqual('');
+
             expect(wrapper.vm.newScope).toEqual('');
           });
         });
@@ -84,6 +89,11 @@ describe('feature flag form', () => {
             active: false,
             id: 2,
           },
+          {
+            environment_scope: 'review',
+            active: true,
+            id: 4,
+          },
         ],
       });
     });
@@ -104,6 +114,7 @@ describe('feature flag form', () => {
 
             expect(wrapper.vm.formScopes).toEqual([
               { active: true, environment_scope: 'production', id: 2 },
+              { active: true, environment_scope: 'review', id: 4 },
             ]);
 
             expect(wrapper.vm.newScope).toEqual('');
@@ -124,11 +135,14 @@ describe('feature flag form', () => {
               _destroy: true,
               id: 2,
             },
+            { active: true, environment_scope: 'review', id: 4 },
           ]);
         });
 
         it('should not render deleted scopes', () => {
-          expect(wrapper.vm.filteredScopes).toEqual([]);
+          expect(wrapper.vm.filteredScopes).toEqual([
+            { active: true, environment_scope: 'review', id: 4 },
+          ]);
         });
       });
 
@@ -142,6 +156,7 @@ describe('feature flag form', () => {
               {
                 environment_scope: 'new_scope',
                 active: false,
+                id: _.uniqueId(internalKeyID),
               },
             ],
           });
@@ -203,26 +218,15 @@ describe('feature flag form', () => {
 
         wrapper.vm.handleSubmit();
 
-        expect(wrapper.emitted().handleSubmit[0]).toEqual([
-          {
-            name: 'feature_flag_2',
-            description: 'this is a feature flag',
-            scopes: [
-              {
-                environment_scope: 'production',
-                active: false,
-              },
-              {
-                environment_scope: 'review',
-                active: false,
-              },
-              {
-                environment_scope: '',
-                active: true,
-              },
-            ],
-          },
-        ]);
+        const data = wrapper.emitted().handleSubmit[0][0];
+
+        expect(data.name).toEqual('feature_flag_2');
+        expect(data.description).toEqual('this is a feature flag');
+        expect(data.scopes.length).toEqual(3);
+        expect(data.scopes[0]).toEqual({
+          active: false,
+          environment_scope: 'production',
+        });
       });
     });
   });

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

+import _ from 'underscore';
+import { parseFeatureFlagsParams, internalKeyID } from 'ee/feature_flags/store/modules/helpers';
+
+describe('feature flags helpers spec', () => {
+  describe('parseFeatureFlagsParams', () => {
+    describe('with internalKeyId', () => {
+      it('removes id', () => {
+        const scopes = [
+          {
+            active: true,
+            created_at: '2019-01-17T17:22:07.625Z',
+            environment_scope: '*',
+            id: 2,
+            updated_at: '2019-01-17T17:22:07.625Z',
+          },
+          {
+            active: true,
+            created_at: '2019-03-11T11:18:42.709Z',
+            environment_scope: 'review',
+            id: 29,
+            updated_at: '2019-03-11T11:18:42.709Z',
+          },
+          {
+            active: true,
+            created_at: '2019-03-11T11:18:42.709Z',
+            environment_scope: 'review',
+            id: _.uniqueId(internalKeyID),
+            updated_at: '2019-03-11T11:18:42.709Z',
+          },
+        ];
+
+        const parsedScopes = parseFeatureFlagsParams({
+          name: 'review',
+          scopes,
+          description: 'feature flag',
+        });
+
+        expect(parsedScopes.operations_feature_flag.scopes_attributes[2].id).toEqual(undefined);
+      });
+    });
+  });
+});