Merge branch 'feature-flags-show-strategy-on-table' into 'master'

Display Strategy Information for New Feature Flags

See merge request gitlab-org/gitlab!38227

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

 <script>
-import { escape } from 'lodash';
-import { GlButton, GlTooltipDirective, GlModal, GlToggle, GlIcon } from '@gitlab/ui';
+import { GlBadge, GlButton, GlTooltipDirective, GlModal, GlToggle, GlIcon } from '@gitlab/ui';
 import { sprintf, s__ } from '~/locale';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { ROLLOUT_STRATEGY_PERCENT_ROLLOUT, NEW_VERSION_FLAG } from '../constants';
+import labelForStrategy from '../utils';
 
 export default {
   components: {
+    GlBadge,
     GlButton,
     GlIcon,
     GlModal,
@@ -43,22 +44,14 @@ export default {
       return this.glFeatures.featureFlagsNewVersion;
     },
     modalTitle() {
-      return sprintf(
-        s__('FeatureFlags|Delete %{name}?'),
-        {
-          name: escape(this.deleteFeatureFlagName),
-        },
-        false,
-      );
+      return sprintf(s__('FeatureFlags|Delete %{name}?'), {
+        name: this.deleteFeatureFlagName,
+      });
     },
     deleteModalMessage() {
-      return sprintf(
-        s__('FeatureFlags|Feature flag %{name} will be removed. Are you sure?'),
-        {
-          name: escape(this.deleteFeatureFlagName),
-        },
-        false,
-      );
+      return sprintf(s__('FeatureFlags|Feature flag %{name} will be removed. Are you sure?'), {
+        name: this.deleteFeatureFlagName,
+      });
     },
     modalId() {
       return 'delete-feature-flag';
@@ -66,7 +59,7 @@ export default {
   },
   methods: {
     isLegacyFlag(flag) {
-      return this.isNewVersionFlagsEnabled && flag.version !== NEW_VERSION_FLAG;
+      return !this.isNewVersionFlagsEnabled || flag.version !== NEW_VERSION_FLAG;
     },
     scopeTooltipText(scope) {
       return !scope.active
@@ -88,6 +81,12 @@ export default {
 
       return `${displayName}${displayPercentage}`;
     },
+    badgeVariant(scope) {
+      return scope.active ? 'info' : 'muted';
+    },
+    strategyBadgeText(strategy) {
+      return labelForStrategy(strategy);
+    },
     featureFlagIidText(featureFlag) {
       return featureFlag.iid ? `^${featureFlag.iid}` : '';
     },
@@ -145,10 +144,10 @@ export default {
               :value="featureFlag.active"
               @change="toggleFeatureFlag(featureFlag)"
             />
-            <span v-else-if="featureFlag.active" class="badge badge-success">
+            <gl-badge v-else-if="featureFlag.active" variant="success">
               {{ s__('FeatureFlags|Active') }}
-            </span>
-            <span v-else class="badge badge-danger">{{ s__('FeatureFlags|Inactive') }}</span>
+            </gl-badge>
+            <gl-badge v-else variant="danger">{{ s__('FeatureFlags|Inactive') }}</gl-badge>
           </div>
         </div>
 
@@ -181,17 +180,29 @@ export default {
           <div
             class="table-mobile-content d-flex flex-wrap justify-content-end justify-content-md-start js-feature-flag-environments"
           >
-            <span
+            <template v-if="isLegacyFlag(featureFlag)">
+              <gl-badge
                 v-for="scope in featureFlag.scopes"
                 :key="scope.id"
                 v-gl-tooltip.hover="scopeTooltipText(scope)"
-              class="badge gl-mr-3 gl-mt-2"
-              :class="{
-                'badge-active': scope.active,
-                'badge-inactive': !scope.active,
-              }"
-              >{{ badgeText(scope) }}</span
+                :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"
+                data-testid="strategy-badge"
+                variant="info"
+                class="gl-mr-3 gl-mt-2"
               >
+                {{ strategyBadgeText(strategy) }}
+              </gl-badge>
+            </template>
           </div>
         </div>
 

ee/app/assets/javascripts/feature_flags/utils.js

+import { s__, n__, sprintf } from '~/locale';
+import {
+  ALL_ENVIRONMENTS_NAME,
+  ROLLOUT_STRATEGY_ALL_USERS,
+  ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+  ROLLOUT_STRATEGY_USER_ID,
+  ROLLOUT_STRATEGY_GITLAB_USER_LIST,
+} from './constants';
+
+const badgeTextByType = {
+  [ROLLOUT_STRATEGY_ALL_USERS]: {
+    name: s__('FeatureFlags|All Users'),
+    parameters: null,
+  },
+  [ROLLOUT_STRATEGY_PERCENT_ROLLOUT]: {
+    name: s__('FeatureFlags|Percent of users'),
+    parameters: ({ parameters: { percentage } }) => `${percentage}%`,
+  },
+  [ROLLOUT_STRATEGY_USER_ID]: {
+    name: s__('FeatureFlags|User IDs'),
+    parameters: ({ parameters: { userIds } }) =>
+      sprintf(n__('FeatureFlags|%d user', 'FeatureFlags|%d users', userIds.split(',').length)),
+  },
+  [ROLLOUT_STRATEGY_GITLAB_USER_LIST]: {
+    name: s__('FeatureFlags|User List'),
+    parameters: ({ user_list: { name } }) => name,
+  },
+};
+
+const scopeName = ({ environment_scope: scope }) =>
+  scope === ALL_ENVIRONMENTS_NAME ? s__('FeatureFlags|All Environments') : scope;
+
+export default strategy => {
+  const { name, parameters } = badgeTextByType[strategy.name];
+
+  if (parameters) {
+    return sprintf('%{name} - %{parameters}: %{scopes}', {
+      name,
+      parameters: parameters(strategy),
+      scopes: strategy.scopes.map(scopeName).join(', '),
+    });
+  }
+
+  return sprintf('%{name}: %{scopes}', {
+    name,
+    scopes: strategy.scopes.map(scopeName).join(', '),
+  });
+};

ee/changelogs/unreleased/feature-flags-show-strategy-on-table.yml

+---
+title: Display Strategy Information for New Feature Flags
+merge_request: 38227
+author:
+type: added

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

@@ -87,8 +87,7 @@ RSpec.describe 'User creates feature flag', :js do
           expect(page).to have_css('.js-feature-flag-status button.is-checked')
 
           within_feature_flag_scopes do
-            expect(page.find('.badge:nth-child(1)')).to have_content('*')
-            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
+            expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
           end
         end
       end
@@ -118,8 +117,7 @@ RSpec.describe 'User creates feature flag', :js do
           expect(page).to have_css('.js-feature-flag-status button.is-checked')
 
           within_feature_flag_scopes do
-            expect(page.find('.badge:nth-child(1)')).to have_content('*')
-            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+            expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
           end
         end
       end
@@ -150,10 +148,8 @@ RSpec.describe 'User creates feature flag', :js do
           expect(page).to have_css('.js-feature-flag-status button.is-checked')
 
           within_feature_flag_scopes do
-            expect(page.find('.badge:nth-child(1)')).to have_content('*')
-            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
-            expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
-            expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active')
+            expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
+            expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(2)')).to have_content('review/*')
           end
         end
       end
@@ -182,10 +178,8 @@ RSpec.describe 'User creates feature flag', :js do
           expect(page).to have_css('.js-feature-flag-status button.is-checked')
 
           within_feature_flag_scopes do
-            expect(page.find('.badge:nth-child(1)')).to have_content('*')
-            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
-            expect(page.find('.badge:nth-child(2)')).to have_content('production')
-            expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+            expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
+            expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(2)')).to have_content('production')
           end
         end
       end

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

@@ -34,10 +34,8 @@ RSpec.describe 'User sees feature flag list', :js do
         expect(page).to have_css('.js-feature-flag-status button:not(.is-checked)')
 
         within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(1)')).to have_content('*')
-          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
-          expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
-          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-active')
+          expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
+          expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(2)')).to have_content('review/*')
         end
       end
     end
@@ -49,8 +47,7 @@ RSpec.describe 'User sees feature flag list', :js do
         expect(page).to have_css('.js-feature-flag-status button:not(.is-checked)')
 
         within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(1)')).to have_content('*')
-          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-inactive')
+          expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
         end
       end
     end
@@ -62,10 +59,8 @@ RSpec.describe 'User sees feature flag list', :js do
         expect(page).to have_css('.js-feature-flag-status button.is-checked')
 
         within_feature_flag_scopes do
-          expect(page.find('.badge:nth-child(1)')).to have_content('*')
-          expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
-          expect(page.find('.badge:nth-child(2)')).to have_content('production')
-          expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+          expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
+          expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(2)')).to have_content('production')
         end
       end
     end

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

@@ -107,10 +107,8 @@ RSpec.describe 'User updates feature flag', :js do
           expect(page).to have_css('.js-feature-flag-status button.is-checked')
 
           within_feature_flag_scopes do
-            expect(page.find('.badge:nth-child(1)')).to have_content('*')
-            expect(page.find('.badge:nth-child(1)')['class']).to include('badge-active')
-            expect(page.find('.badge:nth-child(2)')).to have_content('review/*')
-            expect(page.find('.badge:nth-child(2)')['class']).to include('badge-inactive')
+            expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
+            expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(2)')).to have_content('review/*')
           end
         end
       end
@@ -140,8 +138,7 @@ RSpec.describe 'User updates feature flag', :js do
       it 'shows the newly created scope' do
         within_feature_flag_row(1) do
           within_feature_flag_scopes do
-            expect(page.find('.badge:nth-child(3)')).to have_content('production')
-            expect(page.find('.badge:nth-child(3)')['class']).to include('badge-inactive')
+            expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(3)')).to have_content('production')
           end
         end
       end
@@ -168,8 +165,8 @@ RSpec.describe 'User updates feature flag', :js do
       it 'shows the updated feature flag' do
         within_feature_flag_row(1) do
           within_feature_flag_scopes do
-            expect(page).to have_css('.badge:nth-child(1)')
-            expect(page).not_to have_css('.badge:nth-child(2)')
+            expect(page).to have_css('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')
+            expect(page).not_to have_css('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(2)')
           end
         end
       end

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

 import FeatureFlagsTable from 'ee/feature_flags/components/feature_flags_table.vue';
 import { shallowMount } from '@vue/test-utils';
-import { GlToggle } from '@gitlab/ui';
+import { GlToggle, GlBadge } from '@gitlab/ui';
 import { trimText } from 'helpers/text_helper';
 import {
   ROLLOUT_STRATEGY_ALL_USERS,
   ROLLOUT_STRATEGY_PERCENT_ROLLOUT,
+  ROLLOUT_STRATEGY_USER_ID,
+  ROLLOUT_STRATEGY_GITLAB_USER_LIST,
+  NEW_VERSION_FLAG,
+  LEGACY_FLAG,
   DEFAULT_PERCENT_ROLLOUT,
 } from 'ee/feature_flags/constants';
 
@@ -18,6 +22,7 @@ const getDefaultProps = () => ({
       description: 'flag description',
       destroy_path: 'destroy/path',
       edit_path: 'edit/path',
+      version: LEGACY_FLAG,
       scopes: [
         {
           id: 1,
@@ -97,7 +102,7 @@ describe('Feature flag table', () => {
     it('should render an environments specs badge with active class', () => {
       const envColumn = wrapper.find('.js-feature-flag-environments');
 
-      expect(trimText(envColumn.find('.badge-active').text())).toBe('scope');
+      expect(trimText(envColumn.find(GlBadge).text())).toBe('scope');
     });
 
     it('should render an actions column', () => {
@@ -142,7 +147,7 @@ describe('Feature flag table', () => {
     it('should render an environments specs badge with percentage', () => {
       const envColumn = wrapper.find('.js-feature-flag-environments');
 
-      expect(trimText(envColumn.find('.badge').text())).toBe('scope: 54%');
+      expect(trimText(envColumn.find(GlBadge).text())).toBe('scope: 54%');
     });
   });
 
@@ -155,7 +160,82 @@ describe('Feature flag table', () => {
     it('should render an environments specs badge with inactive class', () => {
       const envColumn = wrapper.find('.js-feature-flag-environments');
 
-      expect(trimText(envColumn.find('.badge-inactive').text())).toBe('scope');
+      expect(trimText(envColumn.find(GlBadge).text())).toBe('scope');
+    });
+  });
+
+  describe('with a new version flag', () => {
+    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',
+            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: { glFeatures: { featureFlagsNewVersion: true } } });
+
+      badges = wrapper.findAll('[data-testid="strategy-badge"]');
+    });
+
+    it('shows All Environments if the environment scope is *', () => {
+      expect(badges.at(0).text()).toContain('All Environments');
+    });
+
+    it('shows the environment scope if another is set', () => {
+      expect(badges.at(1).text()).toContain('production');
+      expect(badges.at(1).text()).toContain('staging');
+      expect(badges.at(2).text()).toContain('review/*');
+    });
+
+    it('shows All Users for the default strategy', () => {
+      expect(badges.at(0).text()).toContain('All Users');
+    });
+
+    it('shows the percent for a percent rollout', () => {
+      expect(badges.at(1).text()).toContain('Percent of users - 50%');
+    });
+
+    it('shows the number of users for users with ID', () => {
+      expect(badges.at(2).text()).toContain('User IDs - 4 users');
+    });
+
+    it('shows the name of a user list for user list', () => {
+      expect(badges.at(3).text()).toContain('User List - test list');
     });
   });
 

locale/gitlab.pot

@@ -10269,6 +10269,11 @@ msgstr ""
 msgid "Feature flag was successfully removed."
 msgstr ""
 
+msgid "FeatureFlags|%d user"
+msgid_plural "FeatureFlags|%d users"
+msgstr[0] ""
+msgstr[1] ""
+
 msgid "FeatureFlags|* (All Environments)"
 msgstr ""
 
@@ -10284,6 +10289,12 @@ msgstr ""
 msgid "FeatureFlags|Add strategy"
 msgstr ""
 
+msgid "FeatureFlags|All Environments"
+msgstr ""
+
+msgid "FeatureFlags|All Users"
+msgstr ""
+
 msgid "FeatureFlags|All users"
 msgstr ""
 
@@ -10398,6 +10409,9 @@ msgstr ""
 msgid "FeatureFlags|New list"
 msgstr ""
 
+msgid "FeatureFlags|Percent of users"
+msgstr ""
+
 msgid "FeatureFlags|Percent rollout (logged in users)"
 msgstr ""
 
@@ -10440,6 +10454,9 @@ msgstr ""
 msgid "FeatureFlags|User IDs"
 msgstr ""
 
+msgid "FeatureFlags|User List"
+msgstr ""
+
 msgid "FeatureFlag|Delete strategy"
 msgstr ""