Merge branch 'afontaine/remove-legacy-flags-frontend-index' into 'master'

Remove Legacy Flags from Feature Flag Table

See merge request gitlab-org/gitlab!64007

app/assets/javascripts/feature_flags/components/feature_flags_table.vue

 <script>
-import { GlBadge, GlButton, GlTooltipDirective, GlModal, GlToggle, GlIcon } from '@gitlab/ui';
+import { GlBadge, GlButton, GlTooltipDirective, GlModal, GlToggle } from '@gitlab/ui';
 import { __, s__, sprintf } from '~/locale';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
-import { ROLLOUT_STRATEGY_PERCENT_ROLLOUT, NEW_VERSION_FLAG, LEGACY_FLAG } from '../constants';
 import { labelForStrategy } from '../utils';
 
 export default {
@@ -14,7 +13,6 @@ export default {
   components: {
     GlBadge,
     GlButton,
-    GlIcon,
     GlModal,
     GlToggle,
   },
@@ -35,13 +33,7 @@ export default {
       deleteFeatureFlagName: null,
     };
   },
-  translations: {
-    legacyFlagReadOnlyAlert: s__('FeatureFlags|Flag is read-only'),
-  },
   computed: {
-    permissions() {
-      return this.glFeatures.featureFlagPermissions;
-    },
     modalTitle() {
       return sprintf(s__('FeatureFlags|Delete %{name}?'), {
         name: this.deleteFeatureFlagName,
@@ -57,12 +49,6 @@ export default {
     },
   },
   methods: {
-    isLegacyFlag(flag) {
-      return flag.version !== NEW_VERSION_FLAG;
-    },
-    statusToggleDisabled(flag) {
-      return flag.version === LEGACY_FLAG;
-    },
     scopeTooltipText(scope) {
       return !scope.active
         ? sprintf(s__('FeatureFlags|Inactive flag for %{scope}'), {
@@ -70,22 +56,6 @@ export default {
           })
         : '';
     },
-    badgeText(scope) {
-      const displayName =
-        scope.environmentScope === '*'
-          ? s__('FeatureFlags|* (All environments)')
-          : scope.environmentScope;
-
-      const displayPercentage =
-        scope.rolloutStrategy === ROLLOUT_STRATEGY_PERCENT_ROLLOUT
-          ? `: ${scope.rolloutPercentage}%`
-          : '';
-
-      return `${displayName}${displayPercentage}`;
-    },
-    badgeVariant(scope) {
-      return scope.active ? 'info' : 'muted';
-    },
     strategyBadgeText(strategy) {
       return labelForStrategy(strategy);
     },
@@ -142,7 +112,6 @@ export default {
             <gl-toggle
               v-if="featureFlag.update_path"
               :value="featureFlag.active"
-              :disabled="statusToggleDisabled(featureFlag)"
               :label="$options.i18n.toggleLabel"
               label-position="hidden"
               data-testid="feature-flag-status-toggle"
@@ -169,12 +138,6 @@ export default {
               <div class="feature-flag-name text-monospace text-truncate">
                 {{ featureFlag.name }}
               </div>
-              <gl-icon
-                v-if="isLegacyFlag(featureFlag)"
-                v-gl-tooltip.hover="$options.translations.legacyFlagReadOnlyAlert"
-                class="gl-ml-3"
-                name="information-o"
-              />
             </div>
             <div class="feature-flag-description text-secondary text-truncate">
               {{ featureFlag.description }}
@@ -189,18 +152,6 @@ export default {
           <div
             class="table-mobile-content d-flex flex-wrap justify-content-end justify-content-md-start js-feature-flag-environments"
           >
-            <template v-if="isLegacyFlag(featureFlag)">
-              <gl-badge
-                v-for="scope in featureFlag.scopes"
-                :key="scope.id"
-                v-gl-tooltip.hover="scopeTooltipText(scope)"
-                :variant="badgeVariant(scope)"
-                :data-qa-selector="`feature-flag-scope-${badgeVariant(scope)}-badge`"
-                class="gl-mr-3 gl-mt-2"
-                >{{ badgeText(scope) }}</gl-badge
-              >
-            </template>
-            <template v-else>
             <gl-badge
               v-for="strategy in featureFlag.strategies"
               :key="strategy.id"
@@ -209,7 +160,6 @@ export default {
               class="gl-mr-3 gl-mt-2 gl-white-space-normal gl-text-left gl-px-5"
               >{{ strategyBadgeText(strategy) }}</gl-badge
             >
-            </template>
           </div>
         </div>
 

app/assets/javascripts/feature_flags/store/index/mutations.js

 import Vue from 'vue';
 import { parseIntPagination, normalizeHeaders } from '~/lib/utils/common_utils';
-import { mapToScopesViewModel } from '../helpers';
 import * as types from './mutation_types';
 
-const mapFlag = (flag) => ({ ...flag, scopes: mapToScopesViewModel(flag.scopes || []) });
-
 const updateFlag = (state, flag) => {
   const index = state.featureFlags.findIndex(({ id }) => id === flag.id);
   Vue.set(state.featureFlags, index, flag);
@@ -31,7 +28,7 @@ export default {
   [types.RECEIVE_FEATURE_FLAGS_SUCCESS](state, response) {
     state.isLoading = false;
     state.hasError = false;
-    state.featureFlags = (response.data.feature_flags || []).map(mapFlag);
+    state.featureFlags = response.data.feature_flags || [];
 
     const paginationInfo = createPaginationInfo(response.headers);
     state.count = paginationInfo?.total ?? state.featureFlags.length;
@@ -58,7 +55,7 @@ export default {
     updateFlag(state, flag);
   },
   [types.RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS](state, data) {
-    updateFlag(state, mapFlag(data));
+    updateFlag(state, data);
   },
   [types.RECEIVE_UPDATE_FEATURE_FLAG_ERROR](state, i) {
     const flag = state.featureFlags.find(({ id }) => i === id);

locale/gitlab.pot

@@ -13568,9 +13568,6 @@ msgstr ""
 msgid "FeatureFlags|* (All Environments)"
 msgstr ""
 
-msgid "FeatureFlags|* (All environments)"
-msgstr ""
-
 msgid "FeatureFlags|API URL"
 msgstr ""
 
@@ -13652,9 +13649,6 @@ msgstr ""
 msgid "FeatureFlags|Feature flags limit reached (%{featureFlagsLimit}). Delete one or more feature flags before adding new ones."
 msgstr ""
 
-msgid "FeatureFlags|Flag is read-only"
-msgstr ""
-
 msgid "FeatureFlags|Get started with feature flags"
 msgstr ""
 

spec/features/projects/feature_flags/user_sees_feature_flag_list_spec.rb

@@ -16,33 +16,63 @@ RSpec.describe 'User sees feature flag list', :js do
     sign_in(user)
   end
 
-  context 'with legacy feature flags' do
+  context 'with feature flags' do
     before do
-      create_flag(project, 'ci_live_trace', false, version: :legacy_flag).tap do |feature_flag|
-        create_scope(feature_flag, 'review/*', true)
+      create_flag(project, 'ci_live_trace', false).tap do |feature_flag|
+        create_strategy(feature_flag).tap do |strat|
+          create(:operations_scope, strategy: strat, environment_scope: '*')
+          create(:operations_scope, strategy: strat, environment_scope: 'review/*')
         end
-      create_flag(project, 'drop_legacy_artifacts', false, version: :legacy_flag)
-      create_flag(project, 'mr_train', true, version: :legacy_flag).tap do |feature_flag|
-        create_scope(feature_flag, 'production', false)
       end
+      create_flag(project, 'drop_legacy_artifacts', false)
+      create_flag(project, 'mr_train', true).tap do |feature_flag|
+        create_strategy(feature_flag).tap do |strat|
+          create(:operations_scope, strategy: strat, environment_scope: 'production')
+        end
+      end
+      create(:operations_feature_flag, :new_version_flag, project: project,
+             name: 'my_flag', active: false)
     end
 
-    it 'shows empty page' do
+    it 'shows the user the first flag' do
       visit(project_feature_flags_path(project))
 
-      expect(page).to have_text 'Get started with feature flags'
-      expect(page).to have_selector('.btn-confirm', text: 'New feature flag')
-      expect(page).to have_selector('[data-qa-selector="configure_feature_flags_button"]', text: 'Configure')
+      within_feature_flag_row(1) do
+        expect(page.find('.js-feature-flag-id')).to have_content('^1')
+        expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
+        expect_status_toggle_button_not_to_be_checked
+
+        within_feature_flag_scopes do
+          expect(page.find('[data-testid="strategy-badge"]')).to have_content('All Users: All Environments, review/*')
+        end
       end
     end
 
-  context 'with new version flags' do
-    before do
-      create(:operations_feature_flag, :new_version_flag, project: project,
-             name: 'my_flag', active: false)
+    it 'shows the user the second flag' do
+      visit(project_feature_flags_path(project))
+
+      within_feature_flag_row(2) do
+        expect(page.find('.js-feature-flag-id')).to have_content('^2')
+        expect(page.find('.feature-flag-name')).to have_content('drop_legacy_artifacts')
+        expect_status_toggle_button_not_to_be_checked
+      end
+    end
+
+    it 'shows the user the third flag' do
+      visit(project_feature_flags_path(project))
+
+      within_feature_flag_row(3) do
+        expect(page.find('.js-feature-flag-id')).to have_content('^3')
+        expect(page.find('.feature-flag-name')).to have_content('mr_train')
+        expect_status_toggle_button_to_be_checked
+
+        within_feature_flag_scopes do
+          expect(page.find('[data-testid="strategy-badge"]')).to have_content('All Users: production')
+        end
+      end
     end
 
-    it 'user updates the status toggle' do
+    it 'allows the user to update the status toggle' do
       visit(project_feature_flags_path(project))
 
       within_feature_flag_row(1) do
@@ -58,7 +88,7 @@ RSpec.describe 'User sees feature flag list', :js do
       visit(project_feature_flags_path(project))
     end
 
-    it 'shows empty page' do
+    it 'shows the empty page' do
       expect(page).to have_text 'Get started with feature flags'
       expect(page).to have_selector('.btn-confirm', text: 'New feature flag')
       expect(page).to have_selector('[data-qa-selector="configure_feature_flags_button"]', text: 'Configure')

spec/frontend/feature_flags/components/feature_flags_table_spec.js

@@ -8,9 +8,6 @@ import {
   ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
   ROLLOUT_STRATEGY_USER_ID,
   ROLLOUT_STRATEGY_GITLAB_USER_LIST,
-  NEW_VERSION_FLAG,
-  LEGACY_FLAG,
-  DEFAULT_PERCENT_ROLLOUT,
 } from '~/feature_flags/constants';
 
 const getDefaultProps = () => ({
@@ -23,17 +20,28 @@ const getDefaultProps = () => ({
       description: 'flag description',
       destroy_path: 'destroy/path',
       edit_path: 'edit/path',
-      version: LEGACY_FLAG,
-      scopes: [
+      scopes: [],
+      strategies: [
         {
-          id: 1,
-          active: true,
-          environmentScope: 'scope',
-          canUpdate: true,
-          protected: false,
-          rolloutStrategy: ROLLOUT_STRATEGY_ALL_USERS,
-          rolloutPercentage: DEFAULT_PERCENT_ROLLOUT,
-          shouldBeDestroyed: false,
+          name: ROLLOUT_STRATEGY_ALL_USERS,
+          parameters: {},
+          scopes: [{ environment_scope: '*' }],
+        },
+        {
+          name: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+          parameters: { percentage: '50' },
+          scopes: [{ environment_scope: 'production' }, { environment_scope: 'staging' }],
+        },
+        {
+          name: ROLLOUT_STRATEGY_USER_ID,
+          parameters: { userIds: '1,2,3,4' },
+          scopes: [{ environment_scope: 'review/*' }],
+        },
+        {
+          name: ROLLOUT_STRATEGY_GITLAB_USER_LIST,
+          parameters: {},
+          user_list: { name: 'test list' },
+          scopes: [{ environment_scope: '*' }],
         },
       ],
     },
@@ -43,6 +51,7 @@ const getDefaultProps = () => ({
 describe('Feature flag table', () => {
   let wrapper;
   let props;
+  let badges;
 
   const createWrapper = (propsData, opts = {}) => {
     wrapper = shallowMount(FeatureFlagsTable, {
@@ -54,6 +63,15 @@ describe('Feature flag table', () => {
     });
   };
 
+  beforeEach(() => {
+    props = getDefaultProps();
+    createWrapper(props, {
+      provide: { csrfToken: 'fakeToken' },
+    });
+
+    badges = wrapper.findAll('[data-testid="strategy-badge"]');
+  });
+
   beforeEach(() => {
     props = getDefaultProps();
   });
@@ -97,17 +115,10 @@ describe('Feature flag table', () => {
       );
     });
 
-    it('should render an environments specs column', () => {
-      const envColumn = wrapper.find('.js-feature-flag-environments');
-
-      expect(envColumn).toBeDefined();
-      expect(trimText(envColumn.text())).toBe('scope');
-    });
-
     it('should render an environments specs badge with active class', () => {
       const envColumn = wrapper.find('.js-feature-flag-environments');
 
-      expect(trimText(envColumn.find(GlBadge).text())).toBe('scope');
+      expect(trimText(envColumn.find(GlBadge).text())).toBe('All Users: All Environments');
     });
 
     it('should render an actions column', () => {
@@ -120,11 +131,13 @@ describe('Feature flag table', () => {
 
   describe('when active and with an update toggle', () => {
     let toggle;
+    let spy;
 
     beforeEach(() => {
       props.featureFlags[0].update_path = props.featureFlags[0].destroy_path;
       createWrapper(props);
       toggle = wrapper.find(GlToggle);
+      spy = mockTracking('_category_', toggle.element, jest.spyOn);
     });
 
     it('should have a toggle', () => {
@@ -143,88 +156,14 @@ describe('Feature flag table', () => {
         expect(wrapper.emitted('toggle-flag')).toEqual([[flag]]);
       });
     });
-  });
-
-  describe('with an active scope and a percentage rollout strategy', () => {
-    beforeEach(() => {
-      props.featureFlags[0].scopes[0].rolloutStrategy = ROLLOUT_STRATEGY_PERCENT_ROLLOUT;
-      props.featureFlags[0].scopes[0].rolloutPercentage = '54';
-      createWrapper(props);
-    });
-
-    it('should render an environments specs badge with percentage', () => {
-      const envColumn = wrapper.find('.js-feature-flag-environments');
-
-      expect(trimText(envColumn.find(GlBadge).text())).toBe('scope: 54%');
-    });
-  });
-
-  describe('with an inactive scope', () => {
-    beforeEach(() => {
-      props.featureFlags[0].scopes[0].active = false;
-      createWrapper(props);
-    });
 
-    it('should render an environments specs badge with inactive class', () => {
-      const envColumn = wrapper.find('.js-feature-flag-environments');
+    it('tracks a click', () => {
+      toggle.trigger('click');
 
-      expect(trimText(envColumn.find(GlBadge).text())).toBe('scope');
-    });
+      expect(spy).toHaveBeenCalledWith('_category_', 'click_button', {
+        label: 'feature_flag_toggle',
       });
-
-  describe('with a new version flag', () => {
-    let toggle;
-    let spy;
-    let badges;
-
-    beforeEach(() => {
-      const newVersionProps = {
-        ...props,
-        featureFlags: [
-          {
-            id: 1,
-            iid: 1,
-            active: true,
-            name: 'flag name',
-            description: 'flag description',
-            destroy_path: 'destroy/path',
-            edit_path: 'edit/path',
-            update_path: 'update/path',
-            version: NEW_VERSION_FLAG,
-            scopes: [],
-            strategies: [
-              {
-                name: ROLLOUT_STRATEGY_ALL_USERS,
-                parameters: {},
-                scopes: [{ environment_scope: '*' }],
-              },
-              {
-                name: ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
-                parameters: { percentage: '50' },
-                scopes: [{ environment_scope: 'production' }, { environment_scope: 'staging' }],
-              },
-              {
-                name: ROLLOUT_STRATEGY_USER_ID,
-                parameters: { userIds: '1,2,3,4' },
-                scopes: [{ environment_scope: 'review/*' }],
-              },
-              {
-                name: ROLLOUT_STRATEGY_GITLAB_USER_LIST,
-                parameters: {},
-                user_list: { name: 'test list' },
-                scopes: [{ environment_scope: '*' }],
-              },
-            ],
-          },
-        ],
-      };
-      createWrapper(newVersionProps, {
-        provide: { csrfToken: 'fakeToken', glFeatures: { featureFlagsNewVersion: true } },
     });
-
-      toggle = wrapper.find(GlToggle);
-      spy = mockTracking('_category_', toggle.element, jest.spyOn);
-      badges = wrapper.findAll('[data-testid="strategy-badge"]');
   });
 
   it('shows All Environments if the environment scope is *', () => {
@@ -253,15 +192,6 @@ describe('Feature flag table', () => {
     expect(badges.at(3).text()).toContain('User List - test list');
   });
 
-    it('tracks a click', () => {
-      toggle.trigger('click');
-
-      expect(spy).toHaveBeenCalledWith('_category_', 'click_button', {
-        label: 'feature_flag_toggle',
-      });
-    });
-  });
-
   it('renders a feature flag without an iid', () => {
     delete props.featureFlags[0].iid;
     createWrapper(props);

spec/frontend/feature_flags/store/index/actions_spec.js

 import MockAdapter from 'axios-mock-adapter';
 import testAction from 'helpers/vuex_action_helper';
 import { TEST_HOST } from 'spec/test_constants';
-import { mapToScopesViewModel } from '~/feature_flags/store/helpers';
 import {
   requestFeatureFlags,
   receiveFeatureFlagsSuccess,
@@ -255,7 +254,6 @@ describe('Feature flags actions', () => {
     beforeEach(() => {
       mockedState.featureFlags = getRequestData.feature_flags.map((flag) => ({
         ...flag,
-        scopes: mapToScopesViewModel(flag.scopes || []),
       }));
       mock = new MockAdapter(axios);
     });
@@ -314,7 +312,6 @@ describe('Feature flags actions', () => {
     beforeEach(() => {
       mockedState.featureFlags = getRequestData.feature_flags.map((f) => ({
         ...f,
-        scopes: mapToScopesViewModel(f.scopes || []),
       }));
     });
 
@@ -338,7 +335,6 @@ describe('Feature flags actions', () => {
     beforeEach(() => {
       mockedState.featureFlags = getRequestData.feature_flags.map((f) => ({
         ...f,
-        scopes: mapToScopesViewModel(f.scopes || []),
       }));
     });
 
@@ -362,7 +358,6 @@ describe('Feature flags actions', () => {
     beforeEach(() => {
       mockedState.featureFlags = getRequestData.feature_flags.map((f) => ({
         ...f,
-        scopes: mapToScopesViewModel(f.scopes || []),
       }));
     });
 

spec/frontend/feature_flags/store/index/mutations_spec.js

-import { mapToScopesViewModel } from '~/feature_flags/store/helpers';
 import * as types from '~/feature_flags/store/index/mutation_types';
 import mutations from '~/feature_flags/store/index/mutations';
 import state from '~/feature_flags/store/index/state';
@@ -49,15 +48,6 @@ describe('Feature flags store Mutations', () => {
       expect(stateCopy.hasError).toEqual(false);
     });
 
-    it('should set featureFlags with the transformed data', () => {
-      const expected = getRequestData.feature_flags.map((flag) => ({
-        ...flag,
-        scopes: mapToScopesViewModel(flag.scopes || []),
-      }));
-
-      expect(stateCopy.featureFlags).toEqual(expected);
-    });
-
     it('should set count with the given data', () => {
       expect(stateCopy.count).toEqual(37);
     });
@@ -131,13 +121,11 @@ describe('Feature flags store Mutations', () => {
     beforeEach(() => {
       stateCopy.featureFlags = getRequestData.feature_flags.map((flag) => ({
         ...flag,
-        scopes: mapToScopesViewModel(flag.scopes || []),
       }));
       stateCopy.count = { featureFlags: 1, userLists: 0 };
 
       mutations[types.UPDATE_FEATURE_FLAG](stateCopy, {
         ...featureFlag,
-        scopes: mapToScopesViewModel(featureFlag.scopes || []),
         active: false,
       });
     });
@@ -146,7 +134,6 @@ describe('Feature flags store Mutations', () => {
       expect(stateCopy.featureFlags).toEqual([
         {
           ...featureFlag,
-          scopes: mapToScopesViewModel(featureFlag.scopes || []),
           active: false,
         },
       ]);
@@ -158,7 +145,6 @@ describe('Feature flags store Mutations', () => {
       stateCopy.featureFlags = getRequestData.feature_flags.map((flag) => ({
         ...flag,
         ...flagState,
-        scopes: mapToScopesViewModel(flag.scopes || []),
       }));
       stateCopy.count = stateCount;
 
@@ -174,7 +160,6 @@ describe('Feature flags store Mutations', () => {
       expect(stateCopy.featureFlags).toEqual([
         {
           ...featureFlag,
-          scopes: mapToScopesViewModel(featureFlag.scopes || []),
           active: false,
         },
       ]);
@@ -185,7 +170,6 @@ describe('Feature flags store Mutations', () => {
     beforeEach(() => {
       stateCopy.featureFlags = getRequestData.feature_flags.map((flag) => ({
         ...flag,
-        scopes: mapToScopesViewModel(flag.scopes || []),
       }));
       mutations[types.RECEIVE_UPDATE_FEATURE_FLAG_ERROR](stateCopy, featureFlag.id);
     });
@@ -194,7 +178,6 @@ describe('Feature flags store Mutations', () => {
       expect(stateCopy.featureFlags).toEqual([
         {
           ...featureFlag,
-          scopes: mapToScopesViewModel(featureFlag.scopes || []),
           active: false,
         },
       ]);

spec/support/helpers/feature_flag_helpers.rb

@@ -14,6 +14,12 @@ module FeatureFlagHelpers
       strategies: strategies)
   end
 
+  def create_strategy(feature_flag, name = 'default', parameters = {})
+    create(:operations_strategy,
+      feature_flag: feature_flag,
+      name: name)
+  end
+
   def within_feature_flag_row(index)
     within ".gl-responsive-table-row:nth-child(#{index + 1})" do
       yield