Merge branch 'afontaine/not-found-legacy-flags' into 'master'

No Longer Show Individual Legacy Feature Flags

See merge request gitlab-org/gitlab!63246

app/controllers/projects/feature_flags_controller.rb

@@ -63,6 +63,7 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
   end
 
   def edit
+    exclude_legacy_flags_check
   end
 
   def update
@@ -158,4 +159,12 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
     render json: { message: messages },
            status: status
   end
+
+  def exclude_legacy_flags_check
+    if Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) &&
+      Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml) &&
+      feature_flag.legacy_flag?
+      not_found
+    end
+  end
 end

lib/api/feature_flags.rb

@@ -90,6 +90,7 @@ module API
         end
         get do
           authorize_read_feature_flag!
+          exclude_legacy_flags_check!
 
           present_entity(feature_flag)
         end
@@ -104,6 +105,7 @@ module API
         end
         post :enable do
           not_found! unless Feature.enabled?(:feature_flag_api, user_project)
+          exclude_legacy_flags_check!
           render_api_error!('Version 2 flags not supported', :unprocessable_entity) if new_version_flag_present?
 
           result = ::FeatureFlags::EnableService
@@ -127,6 +129,7 @@ module API
         end
         post :disable do
           not_found! unless Feature.enabled?(:feature_flag_api, user_project)
+          exclude_legacy_flags_check!
           render_api_error!('Version 2 flags not supported', :unprocessable_entity) if feature_flag.new_version_flag?
 
           result = ::FeatureFlags::DisableService
@@ -162,6 +165,7 @@ module API
         end
         put do
           authorize_update_feature_flag!
+          exclude_legacy_flags_check!
           render_api_error!('PUT operations are not supported for legacy feature flags', :unprocessable_entity) if feature_flag.legacy_flag?
 
           attrs = declared_params(include_missing: false)
@@ -232,6 +236,10 @@ module API
         @feature_flag ||= user_project.operations_feature_flags.find_by_name!(params[:feature_flag_name])
       end
 
+      def project
+        @project ||= feature_flag.project
+      end
+
       def new_version_flag_present?
         user_project.operations_feature_flags.new_version_flag.find_by_name(params[:name]).present?
       end
@@ -245,6 +253,14 @@ module API
         hash[key] = yield(hash[key]) if hash.key?(key)
         hash
       end
+
+      def exclude_legacy_flags_check!
+        if Feature.enabled?(:remove_legacy_flags, project, default_enabled: :yaml) &&
+          Feature.disabled?(:remove_legacy_flags_override, project, default_enabled: :yaml) &&
+          feature_flag.legacy_flag?
+          not_found!
+        end
+      end
     end
   end
 end

spec/controllers/projects/feature_flags_controller_spec.rb

@@ -371,6 +371,58 @@ RSpec.describe Projects::FeatureFlagsController do
     end
   end
 
+  describe 'GET edit' do
+    subject { get(:edit, params: params) }
+
+    context 'with legacy flags' do
+      let!(:feature_flag) { create(:operations_feature_flag, project: project) }
+
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project,
+          iid: feature_flag.iid
+        }
+      end
+
+      context 'removed' do
+        before do
+          stub_feature_flags(remove_legacy_flags: true, remove_legacy_flags_override: false)
+        end
+
+        it 'returns not found' do
+          is_expected.to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'removed' do
+        before do
+          stub_feature_flags(remove_legacy_flags: false)
+        end
+
+        it 'returns ok' do
+          is_expected.to have_gitlab_http_status(:ok)
+        end
+      end
+    end
+
+    context 'with new version flags' do
+      let!(:feature_flag) { create(:operations_feature_flag, :new_version_flag, project: project) }
+
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project,
+          iid: feature_flag.iid
+        }
+      end
+
+      it 'returns successfully' do
+        is_expected.to have_gitlab_http_status(:ok)
+      end
+    end
+  end
+
   describe 'POST create.json' do
     subject { post(:create, params: params, format: :json) }
 

spec/requests/api/feature_flags_spec.rb

@@ -148,6 +148,18 @@ RSpec.describe API::FeatureFlags do
         expect(json_response['version']).to eq('legacy_flag')
       end
 
+      context 'without legacy flags' do
+        before do
+          stub_feature_flags(remove_legacy_flags: true, remove_legacy_flags_override: false)
+        end
+
+        it 'returns not found' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
       it_behaves_like 'check user permission'
     end
 
@@ -492,6 +504,18 @@ RSpec.describe API::FeatureFlags do
       end
 
       it_behaves_like 'check user permission'
+
+      context 'without legacy flags' do
+        before do
+          stub_feature_flags(remove_legacy_flags: true, remove_legacy_flags_override: false)
+        end
+
+        it 'returns not found' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
     end
 
     context 'when feature flag exists already' do
@@ -537,6 +561,18 @@ RSpec.describe API::FeatureFlags do
           end
         end
       end
+
+      context 'without legacy flags' do
+        before do
+          stub_feature_flags(remove_legacy_flags: true, remove_legacy_flags_override: false)
+        end
+
+        it 'returns not found' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
     end
 
     context 'with a version 2 flag' do
@@ -612,6 +648,18 @@ RSpec.describe API::FeatureFlags do
           })
         end
 
+        context 'without legacy flags' do
+          before do
+            stub_feature_flags(remove_legacy_flags: true, remove_legacy_flags_override: false)
+          end
+
+          it 'returns not found' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+
         it_behaves_like 'check user permission'
 
         context 'when strategies become empty array after the removal' do
@@ -976,6 +1024,20 @@ RSpec.describe API::FeatureFlags do
         expect(feature_flag.reload.strategies.first.scopes.count).to eq(0)
       end
     end
+
+    context 'without legacy flags' do
+      before do
+        stub_feature_flags(remove_legacy_flags: true, remove_legacy_flags_override: false)
+      end
+
+      it 'returns not found' do
+        params = { description: 'new description' }
+
+        put api("/projects/#{project.id}/feature_flags/other_flag_name", user), params: params
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
   end
 
   describe 'DELETE /projects/:id/feature_flags/:name' do