Merge branch 'legacy-read-only-flags-override' into 'master'

Add Override for Legacy Read-Only Feature Flags

See merge request gitlab-org/gitlab!40431

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

@@ -85,7 +85,10 @@ export default {
       return this.glFeatures.featureFlagsNewVersion;
     },
     hasLegacyReadOnlyFlags() {
-      return this.glFeatures.featureFlagsLegacyReadOnly;
+      return (
+        this.glFeatures.featureFlagsLegacyReadOnly &&
+        !this.glFeatures.featureFlagsLegacyReadOnlyOverride
+      );
     },
     shouldShowNewFlagAlert() {
       return !(this.hasNewVersionFlags || this.userDidDismissNewFlagAlert);
@@ -128,10 +131,11 @@ export default {
       <div class="gl-display-flex gl-align-items-center gl-mb-4 gl-mt-4">
         <gl-toggle
           :value="active"
+          data-testid="feature-flag-status-toggle"
           data-track-event="click_button"
           data-track-label="feature_flag_toggle"
           data-track-context="feature_flag_activity"
-          class="gl-mr-4 js-feature-flag-status"
+          class="gl-mr-4"
           @change="toggleActive"
         />
         <h3 class="page-title gl-m-0">{{ title }}</h3>

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

@@ -45,7 +45,10 @@ export default {
       return this.glFeatures.featureFlagsNewVersion;
     },
     isLegacyReadOnlyFlagsEnabled() {
-      return this.glFeatures.featureFlagsLegacyReadOnly;
+      return (
+        this.glFeatures.featureFlagsLegacyReadOnly &&
+        !this.glFeatures.featureFlagsLegacyReadOnlyOverride
+      );
     },
     modalTitle() {
       return sprintf(s__('FeatureFlags|Delete %{name}?'), {
@@ -150,17 +153,22 @@ export default {
         </div>
         <div class="table-section section-10" role="gridcell">
           <div class="table-mobile-header" role="rowheader">{{ s__('FeatureFlags|Status') }}</div>
-          <div class="table-mobile-content js-feature-flag-status">
+          <div class="table-mobile-content">
             <gl-toggle
               v-if="featureFlag.update_path"
               :value="featureFlag.active"
               :disabled="statusToggleDisabled(featureFlag)"
+              data-testid="feature-flag-status-toggle"
               data-track-event="click_button"
               data-track-label="feature_flag_toggle"
               data-track-context="feature_flag_activity"
               @change="toggleFeatureFlag(featureFlag)"
             />
-            <gl-badge v-else-if="featureFlag.active" variant="success">
+            <gl-badge
+              v-else-if="featureFlag.active"
+              variant="success"
+              data-testid="feature-flag-status-badge"
+            >
               {{ s__('FeatureFlags|Active') }}
             </gl-badge>
             <gl-badge v-else variant="danger">{{ s__('FeatureFlags|Inactive') }}</gl-badge>

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

@@ -155,6 +155,7 @@ export default {
       return (
         this.glFeatures.featureFlagsNewVersion &&
         this.glFeatures.featureFlagsLegacyReadOnly &&
+        !this.glFeatures.featureFlagsLegacyReadOnlyOverride &&
         this.version === LEGACY_FLAG
       );
     },

ee/app/controllers/projects/feature_flags_controller.rb

@@ -16,6 +16,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
     push_frontend_feature_flag(:feature_flag_permissions)
     push_frontend_feature_flag(:feature_flags_new_version, project, default_enabled: true)
     push_frontend_feature_flag(:feature_flags_legacy_read_only, project)
+    push_frontend_feature_flag(:feature_flags_legacy_read_only_override, project)
   end
 
   def index
@@ -110,7 +111,9 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
   end
 
   def ensure_legacy_flags_writable!
-    if ::Feature.enabled?(:feature_flags_legacy_read_only, project) && feature_flag.legacy_flag?
+    if ::Feature.enabled?(:feature_flags_legacy_read_only, project) &&
+        ::Feature.disabled?(:feature_flags_legacy_read_only_override, project) &&
+        feature_flag.legacy_flag?
       render_error_json(['Legacy feature flags are read-only'])
     end
   end

ee/spec/controllers/projects/feature_flags_controller_spec.rb

@@ -915,7 +915,10 @@ RSpec.describe Projects::FeatureFlagsController do
     end
 
     before do
-      stub_feature_flags(feature_flags_legacy_read_only: false)
+      stub_feature_flags(
+        feature_flags_legacy_read_only: false,
+        feature_flags_legacy_read_only_override: false
+      )
     end
 
     subject { put(:update, params: params, format: :json) }
@@ -1306,6 +1309,18 @@ RSpec.describe Projects::FeatureFlagsController do
         expect(response).to have_gitlab_http_status(:bad_request)
         expect(json_response['message']).to eq(["Legacy feature flags are read-only"])
       end
+
+      it 'updates the flag if the legacy read-only override is enabled for a particular project' do
+        stub_feature_flags(
+          feature_flags_legacy_read_only: true,
+          feature_flags_legacy_read_only_override: project
+        )
+
+        put_request(feature_flag, name: 'ci_new_live_trace')
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['name']).to eq('ci_new_live_trace')
+      end
     end
 
     context 'with a version 2 feature flag' do

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

@@ -84,7 +84,7 @@ RSpec.describe 'User creates feature flag', :js do
       it 'shows the created feature flag' do
         within_feature_flag_row(1) do
           expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
-          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+          expect_status_toggle_button_to_be_checked
 
           within_feature_flag_scopes do
             expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
@@ -114,7 +114,7 @@ RSpec.describe 'User creates feature flag', :js do
       it 'shows the created feature flag' do
         within_feature_flag_row(1) do
           expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
-          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+          expect_status_toggle_button_to_be_checked
 
           within_feature_flag_scopes do
             expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
@@ -145,7 +145,7 @@ RSpec.describe 'User creates feature flag', :js do
       it 'shows the created feature flag' do
         within_feature_flag_row(1) do
           expect(page.find('.feature-flag-name')).to have_content('mr_train')
-          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+          expect_status_toggle_button_to_be_checked
 
           within_feature_flag_scopes do
             expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
@@ -175,7 +175,7 @@ RSpec.describe 'User creates feature flag', :js do
       it 'shows the created feature flag' do
         within_feature_flag_row(1) do
           expect(page.find('.feature-flag-name')).to have_content('mr_train')
-          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+          expect_status_toggle_button_to_be_checked
 
           within_feature_flag_scopes do
             expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')

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

@@ -26,6 +26,7 @@ RSpec.describe 'User sees feature flag list', :js do
       create_flag(project, 'mr_train', true).tap do |feature_flag|
         create_scope(feature_flag, 'production', false)
       end
+      stub_feature_flags(feature_flags_legacy_read_only_override: false)
     end
 
     it 'user sees the first flag' do
@@ -34,7 +35,7 @@ RSpec.describe 'User sees feature flag list', :js do
       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(page).to have_css('.js-feature-flag-status button:not(.is-checked)')
+        expect_status_toggle_button_not_to_be_checked
 
         within_feature_flag_scopes do
           expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
@@ -49,7 +50,7 @@ RSpec.describe 'User sees feature flag list', :js do
       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(page).to have_css('.js-feature-flag-status button:not(.is-checked)')
+        expect_status_toggle_button_not_to_be_checked
 
         within_feature_flag_scopes do
           expect(page.find('[data-qa-selector="feature-flag-scope-muted-badge"]:nth-child(1)')).to have_content('*')
@@ -63,7 +64,7 @@ RSpec.describe 'User sees feature flag list', :js do
       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(page).to have_css('.js-feature-flag-status button.is-checked')
+        expect_status_toggle_button_to_be_checked
 
         within_feature_flag_scopes do
           expect(page.find('[data-qa-selector="feature-flag-scope-info-badge"]:nth-child(1)')).to have_content('*')
@@ -76,7 +77,7 @@ RSpec.describe 'User sees feature flag list', :js do
       visit(project_feature_flags_path(project))
 
       within_feature_flag_row(1) do
-        expect(page).to have_css('.js-feature-flag-status button.is-disabled')
+        expect_status_toggle_button_to_be_disabled
       end
     end
 
@@ -89,9 +90,34 @@ RSpec.describe 'User sees feature flag list', :js do
         visit(project_feature_flags_path(project))
 
         within_feature_flag_row(1) do
-          page.find('.js-feature-flag-status button').click
+          status_toggle_button.click
+
+          expect_status_toggle_button_to_be_checked
+        end
+
+        visit(project_audit_events_path(project))
+
+        expect(page).to(
+          have_text('Updated feature flag ci_live_trace. Updated active from "false" to "true".')
+        )
+      end
+    end
+
+    context 'when legacy feature flags are read-only but the override is active for a project' do
+      before do
+        stub_feature_flags(
+          feature_flags_legacy_read_only: true,
+          feature_flags_legacy_read_only_override: project
+        )
+      end
+
+      it 'user updates the status toggle' do
+        visit(project_feature_flags_path(project))
+
+        within_feature_flag_row(1) do
+          status_toggle_button.click
 
-          expect(page).to have_css('.js-feature-flag-status button.is-checked')
+          expect_status_toggle_button_to_be_checked
         end
 
         visit(project_audit_events_path(project))
@@ -115,7 +141,7 @@ RSpec.describe 'User sees feature flag list', :js do
       within_feature_flag_row(1) do
         status_toggle_button.click
 
-        expect(page).to have_css('.js-feature-flag-status button.is-checked')
+        expect_status_toggle_button_to_be_checked
       end
 
       visit(project_audit_events_path(project))

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

@@ -14,7 +14,10 @@ RSpec.describe 'User updates feature flag', :js do
 
   before do
     stub_licensed_features(feature_flags: true)
-    stub_feature_flags(feature_flag_permissions: false)
+    stub_feature_flags(
+      feature_flag_permissions: false,
+      feature_flags_legacy_read_only_override: false
+    )
     sign_in(user)
   end
 
@@ -97,7 +100,7 @@ RSpec.describe 'User updates feature flag', :js do
         end
       end
 
-      context 'when user updates a status of a scope' do
+      context 'when user updates the status of a scope' do
         before do
           within_scope_row(2) do
             within_status { find('.project-feature-toggle').click }
@@ -110,7 +113,7 @@ RSpec.describe 'User updates feature flag', :js do
         it 'shows the updated feature flag' do
           within_feature_flag_row(1) do
             expect(page.find('.feature-flag-name')).to have_content('ci_live_trace')
-            expect(page).to have_css('.js-feature-flag-status button.is-checked')
+            expect_status_toggle_button_to_be_checked
 
             within_feature_flag_scopes do
               expect(page.find('.badge:nth-child(1)')).to have_content('*')
@@ -194,5 +197,20 @@ RSpec.describe 'User updates feature flag', :js do
         expect(page).to have_css('button.js-ff-submit.disabled')
       end
     end
+
+    context 'when legacy flags are read-only, but the override is active for one project' do
+      it 'the user can edit the flag' do
+        stub_feature_flags(feature_flags_legacy_read_only_override: project)
+
+        visit(edit_project_feature_flag_path(project, feature_flag))
+        status_toggle_button.click
+        click_button 'Save changes'
+
+        expect(page).to have_current_path(project_feature_flags_path(project))
+        within_feature_flag_row(1) do
+          expect_status_toggle_button_not_to_be_checked
+        end
+      end
+    end
   end
 end

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

@@ -80,8 +80,10 @@ describe('Feature flag table', () => {
     });
 
     it('Should render a status column', () => {
-      expect(wrapper.find('.js-feature-flag-status').exists()).toBe(true);
-      expect(trimText(wrapper.find('.js-feature-flag-status').text())).toEqual('Active');
+      const badge = wrapper.find('[data-testid="feature-flag-status-badge"]');
+
+      expect(badge.exists()).toBe(true);
+      expect(trimText(badge.text())).toEqual('Active');
     });
 
     it('Should render a feature flag column', () => {

ee/spec/support/helpers/feature_flag_helpers.rb

@@ -73,11 +73,19 @@ module FeatureFlagHelpers
   end
 
   def status_toggle_button
-    find('.js-feature-flag-status button')
+    find('[data-testid="feature-flag-status-toggle"] button')
   end
 
   def expect_status_toggle_button_to_be_checked
-    expect(page).to have_css('.js-feature-flag-status button.is-checked')
+    expect(page).to have_css('[data-testid="feature-flag-status-toggle"] button.is-checked')
+  end
+
+  def expect_status_toggle_button_not_to_be_checked
+    expect(page).to have_css('[data-testid="feature-flag-status-toggle"] button:not(.is-checked)')
+  end
+
+  def expect_status_toggle_button_to_be_disabled
+    expect(page).to have_css('[data-testid="feature-flag-status-toggle"] button.is-disabled')
   end
 
   def expect_user_to_see_feature_flags_index_page