Automatic merge of gitlab-org/gitlab master

app/assets/javascripts/nav/components/top_nav_app.vue

 <script>
-import { GlNav, GlNavItemDropdown, GlDropdownForm, GlTooltip } from '@gitlab/ui';
-import { s__ } from '~/locale';
+import { GlNav, GlNavItemDropdown, GlDropdownForm } from '@gitlab/ui';
 import TopNavDropdownMenu from './top_nav_dropdown_menu.vue';
 
-const TOOLTIP = s__('TopNav|Switch to...');
-
 export default {
   components: {
     GlNav,
     GlNavItemDropdown,
     GlDropdownForm,
-    GlTooltip,
     TopNavDropdownMenu,
   },
   props: {
@@ -19,15 +15,6 @@ export default {
       required: true,
     },
   },
-  methods: {
-    findTooltipTarget() {
-      // ### Why use a target function instead of `v-gl-tooltip`?
-      // To get the tooltip to align correctly, we need it to target the actual
-      // toggle button which we don't directly render.
-      return this.$el.querySelector('.js-top-nav-dropdown-toggle');
-    },
-  },
-  TOOLTIP,
 };
 </script>
 
@@ -48,12 +35,5 @@ export default {
         />
       </gl-dropdown-form>
     </gl-nav-item-dropdown>
-    <gl-tooltip
-      boundary="window"
-      :boundary-padding="0"
-      :target="findTooltipTarget"
-      placement="right"
-      :title="$options.TOOLTIP"
-    />
   </gl-nav>
 </template>

app/finders/feature_flags_finder.rb

@@ -24,7 +24,11 @@ class FeatureFlagsFinder
   private
 
   def feature_flags
-    project.operations_feature_flags
+    if exclude_legacy_flags?
+      project.operations_feature_flags.new_version_only
+    else
+      project.operations_feature_flags
+    end
   end
 
   def by_scope(items)
@@ -37,4 +41,9 @@ class FeatureFlagsFinder
       items
     end
   end
+
+  def exclude_legacy_flags?
+    Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) &&
+      Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml)
+  end
 end

app/models/operations/feature_flag.rb

@@ -49,6 +49,8 @@ module Operations
     scope :enabled, -> { where(active: true) }
     scope :disabled, -> { where(active: false) }
 
+    scope :new_version_only, -> { where(version: :new_version_flag)}
+
     enum version: {
       legacy_flag: 1,
       new_version_flag: 2

config/feature_flags/development/remove_legacy_flags.yml

+---
+name: remove_legacy_flags
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62484
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332243
+milestone: '14.0'
+type: development
+group: group::release
+default_enabled: false

config/feature_flags/development/remove_legacy_flags_override.yml

+---
+name: remove_legacy_flags_override
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/62484
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/332243
+milestone: '14.0'
+type: development
+group: group::release
+default_enabled: false

doc/development/feature_flags/controls.md

@@ -213,9 +213,6 @@ actors.
 Feature.enabled?(:some_feature, group)
 ```
 
-**Percentage of time** rollout is not a good idea if what you want is to make sure a feature
-is always on or off to the users. In that case, **Percentage of actors** rollout is a better method.
-
 Lastly, to verify that the feature is deemed stable in as many cases as possible,
 you should fully roll out the feature by enabling the flag **globally** by running:
 
@@ -226,6 +223,15 @@ you should fully roll out the feature by enabling the flag **globally** by runni
 This changes the feature flag state to be **enabled** always, which overrides the
 existing gates (e.g. `--group=gitlab-org`) in the above processes.
 
+##### Percentage of actors vs percentage of time rollouts
+
+If you want to make sure a feature is always on or off for users, use a **Percentage of actors**
+rollout. Avoid using percentage of _time_ rollouts in this case.
+
+A percentage of _time_ rollout can introduce inconsistent behavior when `Feature.enabled?`
+is used multiple times in the code because the feature flag value is randomized each time
+`Feature.enabled?` is called on your code path.
+
 ##### Disabling feature flags
 
 To disable a feature flag that has been globally enabled you can run:

lib/api/unleash.rb

@@ -69,11 +69,22 @@ module API
       def feature_flags
         return [] unless unleash_app_name.present?
 
-        legacy_flags = Operations::FeatureFlagScope.for_unleash_client(project, unleash_app_name)
+        legacy_flags =
+          if exclude_legacy_flags?
+            []
+          else
+            Operations::FeatureFlagScope.for_unleash_client(project, unleash_app_name)
+          end
+
         new_version_flags = Operations::FeatureFlag.for_unleash_client(project, unleash_app_name)
 
         legacy_flags + new_version_flags
       end
+
+      def exclude_legacy_flags?
+        Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) &&
+          Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml)
+      end
     end
   end
 end

locale/gitlab.pot

@@ -34333,9 +34333,6 @@ msgstr ""
 msgid "Too many projects enabled. You will need to manage them via the console or the API."
 msgstr ""
 
-msgid "TopNav|Switch to..."
-msgstr ""
-
 msgid "Topics (optional)"
 msgstr ""
 

spec/finders/feature_flags_finder_spec.rb

@@ -24,6 +24,10 @@ RSpec.describe FeatureFlagsFinder do
     let!(:feature_flag_2) { create(:operations_feature_flag, name: 'flag-b', project: project) }
     let(:args) { {} }
 
+    before do
+      stub_feature_flags(remove_legacy_flags: false)
+    end
+
     it 'returns feature flags ordered by name' do
       is_expected.to eq([feature_flag_1, feature_flag_2])
     end
@@ -79,6 +83,16 @@ RSpec.describe FeatureFlagsFinder do
       it 'returns new and legacy flags' do
         is_expected.to eq([feature_flag_1, feature_flag_2, feature_flag_3])
       end
+
+      context 'when legacy flags are disabled' do
+        before do
+          stub_feature_flags(remove_legacy_flags_override: false, remove_legacy_flags: true)
+        end
+
+        it 'returns only new flags' do
+          is_expected.to eq([feature_flag_3])
+        end
+      end
     end
   end
 end

spec/frontend/nav/components/top_nav_app_spec.js

-import { GlNavItemDropdown, GlTooltip } from '@gitlab/ui';
-import { shallowMount, mount } from '@vue/test-utils';
+import { GlNavItemDropdown } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
 import TopNavApp from '~/nav/components/top_nav_app.vue';
 import TopNavDropdownMenu from '~/nav/components/top_nav_dropdown_menu.vue';
 import { TEST_NAV_DATA } from '../mock_data';
@@ -7,8 +7,8 @@ import { TEST_NAV_DATA } from '../mock_data';
 describe('~/nav/components/top_nav_app.vue', () => {
   let wrapper;
 
-  const createComponent = (mountFn = shallowMount) => {
-    wrapper = mountFn(TopNavApp, {
+  const createComponent = () => {
+    wrapper = shallowMount(TopNavApp, {
       propsData: {
         navData: TEST_NAV_DATA,
       },
@@ -17,7 +17,6 @@ describe('~/nav/components/top_nav_app.vue', () => {
 
   const findNavItemDropdown = () => wrapper.findComponent(GlNavItemDropdown);
   const findMenu = () => wrapper.findComponent(TopNavDropdownMenu);
-  const findTooltip = () => wrapper.findComponent(GlTooltip);
 
   afterEach(() => {
     wrapper.destroy();
@@ -44,25 +43,5 @@ describe('~/nav/components/top_nav_app.vue', () => {
         views: TEST_NAV_DATA.views,
       });
     });
-
-    it('renders tooltip', () => {
-      expect(findTooltip().attributes()).toMatchObject({
-        'boundary-padding': '0',
-        placement: 'right',
-        title: TopNavApp.TOOLTIP,
-      });
-    });
-  });
-
-  describe('when full mounted', () => {
-    beforeEach(() => {
-      createComponent(mount);
-    });
-
-    it('has dropdown toggle as tooltip target', () => {
-      const targetFn = findTooltip().props('target');
-
-      expect(targetFn()).toBe(wrapper.find('.js-top-nav-dropdown-toggle').element);
-    });
   });
 });

spec/requests/api/unleash_spec.rb

@@ -590,6 +590,32 @@ RSpec.describe API::Unleash do
             }]
           }])
         end
+
+        it 'returns new flags when legacy flags are disabled' do
+          stub_feature_flags(remove_legacy_flags_override: false, remove_legacy_flags: true)
+
+          feature_flag_a = create(:operations_feature_flag, :new_version_flag, project: project,
+                                  name: 'feature_a', active: true)
+          strategy = create(:operations_strategy, feature_flag: feature_flag_a,
+                            name: 'userWithId', parameters: { userIds: 'user8' })
+          create(:operations_scope, strategy: strategy, environment_scope: 'staging')
+          feature_flag_b = create(:operations_feature_flag, :legacy_flag, project: project,
+                                  name: 'feature_b', active: true)
+          create(:operations_feature_flag_scope, feature_flag: feature_flag_b,
+                 active: true, strategies: [{ name: 'default', parameters: {} }], environment_scope: 'staging')
+
+          get api(features_url), headers: { 'UNLEASH-INSTANCEID' => client.token, 'UNLEASH-APPNAME' => 'staging' }
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['features'].sort_by {|f| f['name']}).to eq([{
+            'name' => 'feature_a',
+            'enabled' => true,
+            'strategies' => [{
+              'name' => 'userWithId',
+              'parameters' => { 'userIds' => 'user8' }
+            }]
+          }])
+        end
       end
     end
   end