Integrate Edit Feature Flag Page with New UI

By passing a version in to the form, the form can now handle new version
flags to edit. The update action has also been beefed up.

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

@@ -36,10 +36,12 @@ export default {
       'name',
       'description',
       'scopes',
+      'strategies',
       'isLoading',
       'hasError',
       'iid',
       'active',
+      'version',
     ]),
     title() {
       return this.iid
@@ -80,10 +82,12 @@ export default {
         :name="name"
         :description="description"
         :scopes="scopes"
+        :strategies="strategies"
         :cancel-path="path"
         :submit-text="__('Save changes')"
         :environments-endpoint="environmentsEndpoint"
         :active="active"
+        :version="version"
         @handleSubmit="data => updateFeatureFlag(data)"
       />
     </template>

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

 <script>
 import Vue from 'vue';
-import { memoize, isString, cloneDeep } from 'lodash';
+import { memoize, isString, cloneDeep, isNumber } from 'lodash';
 import {
   GlButton,
   GlBadge,
@@ -147,7 +147,7 @@ export default {
     },
 
     deleteStrategy(s) {
-      if (s.id) {
+      if (isNumber(s.id)) {
         Vue.set(s, 'shouldBeDestroyed', true);
       } else {
         this.formStrategies = this.formStrategies.filter(strategy => strategy !== s);

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

 <script>
 import Vue from 'vue';
-import { isString } from 'lodash';
+import { isNumber } from 'lodash';
 import {
   GlFormSelect,
   GlFormInput,
@@ -122,18 +122,18 @@ export default {
       return (
         this.filteredEnvironments.length === 0 ||
         (this.filteredEnvironments.length === 1 &&
-          this.filteredEnvironments[0].environment_scope === '*')
+          this.filteredEnvironments[0].environmentScope === '*')
       );
     },
     filteredEnvironments() {
-      return this.environments.filter(e => !e.shouldDestroy);
+      return this.environments.filter(e => !e.shouldBeDestroyed);
     },
   },
   methods: {
     addEnvironment(environment) {
       const allEnvironmentsScope = this.environments.find(scope => scope.environmentScope === '*');
       if (allEnvironmentsScope) {
-        allEnvironmentsScope.shouldDestroy = true;
+        allEnvironmentsScope.shouldBeDestroyed = true;
       }
       this.environments.push({ environmentScope: environment });
       this.onStrategyChange();
@@ -158,7 +158,7 @@ export default {
       });
     },
     removeScope(environment) {
-      if (isString(environment.id)) {
+      if (isNumber(environment.id)) {
         Vue.set(environment, 'shouldBeDestroyed', true);
       } else {
         this.environments = this.environments.filter(e => e !== environment);

ee/app/assets/javascripts/feature_flags/store/modules/edit/actions.js

@@ -3,7 +3,8 @@ import axios from '~/lib/utils/axios_utils';
 import { visitUrl } from '~/lib/utils/url_utility';
 import createFlash from '~/flash';
 import { __ } from '~/locale';
-import { mapFromScopesViewModel } from '../helpers';
+import { NEW_VERSION_FLAG } from '../../../constants';
+import { mapFromScopesViewModel, mapStrategiesToRails } from '../helpers';
 
 /**
  * Commits mutation to set the main endpoint
@@ -32,7 +33,12 @@ export const updateFeatureFlag = ({ state, dispatch }, params) => {
   dispatch('requestUpdateFeatureFlag');
 
   axios
-    .put(state.endpoint, mapFromScopesViewModel(params))
+    .put(
+      state.endpoint,
+      params.version === NEW_VERSION_FLAG
+        ? mapStrategiesToRails(params)
+        : mapFromScopesViewModel(params),
+    )
     .then(() => {
       dispatch('receiveUpdateFeatureFlagSuccess');
       visitUrl(state.path);

ee/app/assets/javascripts/feature_flags/store/modules/edit/mutations.js

 import * as types from './mutation_types';
-import { mapToScopesViewModel } from '../helpers';
+import { mapToScopesViewModel, mapStrategiesToViewModel } from '../helpers';
+import { LEGACY_FLAG } from '../../../constants';
 
 export default {
   [types.SET_ENDPOINT](state, endpoint) {
@@ -20,8 +21,8 @@ export default {
     state.iid = response.iid;
     state.active = response.active;
     state.scopes = mapToScopesViewModel(response.scopes);
-    state.strategies = response.strategies || [];
-    state.version = response.version || 1;
+    state.strategies = mapStrategiesToViewModel(response.strategies);
+    state.version = response.version || LEGACY_FLAG;
   },
   [types.RECEIVE_FEATURE_FLAG_ERROR](state) {
     state.isLoading = false;

ee/app/assets/javascripts/feature_flags/store/modules/edit/state.js

+import { LEGACY_FLAG } from '../../../constants';
+
 export default () => ({
   endpoint: null,
   path: null,
@@ -12,5 +14,5 @@ export default () => ({
   iid: null,
   active: true,
   strategies: [],
-  version: 1,
+  version: LEGACY_FLAG,
 });

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

@@ -181,7 +181,7 @@ export const mapStrategiesToRails = params => ({
       name: s.name,
       parameters: s.parameters,
       _destroy: s.shouldBeDestroyed,
-      scopes: mapStrategyScopesToRails(s.scopes || []),
+      scopes_attributes: mapStrategyScopesToRails(s.scopes || []),
     })),
   },
 });

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

@@ -2,6 +2,7 @@ import Vuex from 'vuex';
 import { createLocalVue, shallowMount } from '@vue/test-utils';
 import MockAdapter from 'axios-mock-adapter';
 import { GlToggle } from '@gitlab/ui';
+import { LEGACY_FLAG, NEW_VERSION_FLAG } from 'ee/feature_flags/constants';
 import Form from 'ee/feature_flags/components/form.vue';
 import editModule from 'ee/feature_flags/store/modules/edit';
 import EditFeatureFlag from 'ee/feature_flags/components/edit_feature_flag.vue';
@@ -22,21 +23,30 @@ describe('Edit feature flag form', () => {
   });
 
   const factory = () => {
+    if (wrapper) {
+      wrapper.destroy();
+      wrapper = null;
+    }
     wrapper = shallowMount(EditFeatureFlag, {
       localVue,
       propsData: {
-        endpoint: `${TEST_HOST}/feature_flags.json'`,
+        endpoint: `${TEST_HOST}/feature_flags.json`,
         path: '/feature_flags',
         environmentsEndpoint: 'environments.json',
       },
       store,
+      provide: {
+        glFeatures: {
+          featureFlagsNewVersion: true,
+        },
+      },
     });
   };
 
   beforeEach(done => {
     mock = new MockAdapter(axios);
 
-    mock.onGet(`${TEST_HOST}/feature_flags.json'`).replyOnce(200, {
+    mock.onGet(`${TEST_HOST}/feature_flags.json`).replyOnce(200, {
       id: 21,
       iid: 5,
       active: true,
@@ -44,6 +54,7 @@ describe('Edit feature flag form', () => {
       updated_at: '2019-01-17T17:27:39.778Z',
       name: 'feature_flag',
       description: '',
+      version: LEGACY_FLAG,
       edit_path: '/h5bp/html5-boilerplate/-/feature_flags/21/edit',
       destroy_path: '/h5bp/html5-boilerplate/-/feature_flags/21',
       scopes: [
@@ -98,5 +109,31 @@ describe('Edit feature flag form', () => {
     it('should render feature flag form', () => {
       expect(wrapper.find(Form).exists()).toEqual(true);
     });
+
+    it('should set the version of the form from the feature flag', () => {
+      expect(wrapper.find(Form).props('version')).toBe(LEGACY_FLAG);
+
+      mock.resetHandlers();
+
+      mock.onGet(`${TEST_HOST}/feature_flags.json`).replyOnce(200, {
+        id: 21,
+        iid: 5,
+        active: true,
+        created_at: '2019-01-17T17:27:39.778Z',
+        updated_at: '2019-01-17T17:27:39.778Z',
+        name: 'feature_flag',
+        description: '',
+        version: NEW_VERSION_FLAG,
+        edit_path: '/h5bp/html5-boilerplate/-/feature_flags/21/edit',
+        destroy_path: '/h5bp/html5-boilerplate/-/feature_flags/21',
+        strategies: [],
+      });
+
+      factory();
+
+      return axios.waitForAll().then(() => {
+        expect(wrapper.find(Form).props('version')).toBe(NEW_VERSION_FLAG);
+      });
+    });
   });
 });

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

@@ -142,7 +142,7 @@ describe('Feature flags strategy', () => {
                 name: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
                 parameters: { percentage: '50', groupId: PERCENT_ROLLOUT_GROUP_ID },
                 scopes: [
-                  { environmentScope: '*', shouldDestroy: true },
+                  { environmentScope: '*', shouldBeDestroyed: true },
                   { environmentScope: 'production' },
                 ],
               },

ee/spec/frontend/feature_flags/store/edit/actions_spec.js

@@ -13,6 +13,15 @@ import {
   toggleActive,
 } from 'ee/feature_flags/store/modules/edit/actions';
 import state from 'ee/feature_flags/store/modules/edit/state';
+import {
+  mapStrategiesToRails,
+  mapFromScopesViewModel,
+} from 'ee/feature_flags/store/modules/helpers';
+import {
+  NEW_VERSION_FLAG,
+  LEGACY_FLAG,
+  ROLLOUT_STRATEGY_ALL_USERS,
+} from 'ee/feature_flags/constants';
 import * as types from 'ee/feature_flags/store/modules/edit/mutation_types';
 import testAction from 'helpers/vuex_action_helper';
 import { TEST_HOST } from 'spec/test_constants';
@@ -67,15 +76,62 @@ describe('Feature flags Edit Module actions', () => {
 
     describe('success', () => {
       it('dispatches requestUpdateFeatureFlag and receiveUpdateFeatureFlagSuccess ', done => {
-        mock.onPut(mockedState.endpoint).replyOnce(200);
+        const featureFlag = {
+          name: 'feature_flag',
+          description: 'feature flag',
+          scopes: [
+            {
+              id: '1',
+              environmentScope: '*',
+              active: true,
+              shouldBeDestroyed: false,
+              canUpdate: true,
+              protected: false,
+              rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
+            },
+          ],
+          version: LEGACY_FLAG,
+          active: true,
+        };
+        mock.onPut(mockedState.endpoint, mapFromScopesViewModel(featureFlag)).replyOnce(200);
 
         testAction(
           updateFeatureFlag,
-          {
-            name: 'feature_flag',
-            description: 'feature flag',
-            scopes: [{ environmentScope: '*', active: true }],
-          },
+          featureFlag,
+          mockedState,
+          [],
+          [
+            {
+              type: 'requestUpdateFeatureFlag',
+            },
+            {
+              type: 'receiveUpdateFeatureFlagSuccess',
+            },
+          ],
+          done,
+        );
+      });
+      it('handles new version flags as well', done => {
+        const featureFlag = {
+          name: 'name',
+          description: 'description',
+          active: true,
+          version: NEW_VERSION_FLAG,
+          strategies: [
+            {
+              name: ROLLOUT_STRATEGY_ALL_USERS,
+              parameters: {},
+              id: 1,
+              scopes: [{ id: 1, environmentScope: 'environmentScope', shouldBeDestroyed: false }],
+              shouldBeDestroyed: false,
+            },
+          ],
+        };
+        mock.onPut(mockedState.endpoint, mapStrategiesToRails(featureFlag)).replyOnce(200);
+
+        testAction(
+          updateFeatureFlag,
+          featureFlag,
           mockedState,
           [],
           [

ee/spec/frontend/feature_flags/store/edit/mutations_spec.js

@@ -45,8 +45,10 @@ describe('Feature flags Edit Module Mutations', () => {
       description: 'All environments',
       scopes: [{ id: 1 }],
       iid: 5,
-      version: 2,
-      strategies: [{ scopes: [], type: 'default', paramters: {} }],
+      version: 'new_version_flag',
+      strategies: [
+        { id: 1, scopes: [{ environment_scope: '*' }], name: 'default', parameters: {} },
+      ],
     };
 
     beforeEach(() => {
@@ -78,11 +80,19 @@ describe('Feature flags Edit Module Mutations', () => {
     });
 
     it('should set the version to the provided one', () => {
-      expect(stateCopy.version).toBe(2);
+      expect(stateCopy.version).toBe('new_version_flag');
     });
 
     it('should set the strategies to the provided one', () => {
-      expect(stateCopy.strategies).toEqual([{ scopes: [], type: 'default', paramters: {} }]);
+      expect(stateCopy.strategies).toEqual([
+        {
+          id: 1,
+          scopes: [{ environmentScope: '*', shouldBeDestroyed: false }],
+          name: 'default',
+          parameters: {},
+          shouldBeDestroyed: false,
+        },
+      ]);
     });
   });
 

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

@@ -413,7 +413,7 @@ describe('feature flags helpers spec', () => {
               name: 'default',
               parameters: {},
               _destroy: true,
-              scopes: [
+              scopes_attributes: [
                 {
                   environment_scope: '*',
                   id: '1',
@@ -450,7 +450,7 @@ describe('feature flags helpers spec', () => {
               id: '1',
               name: 'default',
               parameters: {},
-              scopes: [
+              scopes_attributes: [
                 {
                   environment_scope: '*',
                 },