Merge branch '290116-enable-webhook-test-reorder-feature' into 'master'

Enable selecting newest record for webhook tests [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54030

app/services/concerns/integrations/project_test_data.rb

@@ -8,10 +8,6 @@ module Integrations
       Gitlab::DataBuilder::Push.build_sample(project, current_user)
     end
 
-    def use_newest_record?
-      Feature.enabled?(:integrations_test_webhook_reorder, project)
-    end
-
     def note_events_data
       note = NotesFinder.new(current_user, project: project, target: project).execute.reorder(nil).last # rubocop: disable CodeReuse/ActiveRecord
 
@@ -37,11 +33,7 @@ module Integrations
     end
 
     def job_events_data
-      build = if use_newest_record?
-                Ci::JobsFinder.new(current_user: current_user, project: project).execute.first
-              else
-                project.builds.first
-              end
+      build = Ci::JobsFinder.new(current_user: current_user, project: project).execute.first
 
       return { error: s_('TestHooks|Ensure the project has CI jobs.') } unless build.present?
 
@@ -49,11 +41,7 @@ module Integrations
     end
 
     def pipeline_events_data
-      pipeline = if use_newest_record?
-                   Ci::PipelinesFinder.new(project, current_user, order_by: 'id', sort: 'desc').execute.first
-                 else
-                   project.ci_pipelines.newest_first.first
-                 end
+      pipeline = Ci::PipelinesFinder.new(project, current_user, order_by: 'id', sort: 'desc').execute.first
 
       return { error: s_('TestHooks|Ensure the project has CI pipelines.') } unless pipeline.present?
 
@@ -71,11 +59,7 @@ module Integrations
     end
 
     def deployment_events_data
-      deployment = if use_newest_record?
-                     DeploymentsFinder.new(project: project, order_by: 'created_at', sort: 'desc').execute.first
-                   else
-                     project.deployments.first
-                   end
+      deployment = DeploymentsFinder.new(project: project, order_by: 'created_at', sort: 'desc').execute.first
 
       return { error: s_('TestHooks|Ensure the project has deployments.') } unless deployment.present?
 
@@ -83,11 +67,7 @@ module Integrations
     end
 
     def releases_events_data
-      release = if use_newest_record?
-                  ReleasesFinder.new(project, current_user, order_by: :created_at, sort: :desc).execute.first
-                else
-                  project.releases.first
-                end
+      release = ReleasesFinder.new(project, current_user, order_by: :created_at, sort: :desc).execute.first
 
       return { error: s_('TestHooks|Ensure the project has releases.') } unless release.present?
 

app/services/test_hooks/system_service.rb

@@ -6,10 +6,6 @@ module TestHooks
 
     private
 
-    def use_newest_record?
-      Feature.enabled?(:integrations_test_webhook_reorder)
-    end
-
     def data
       strong_memoize(:data) do
         case trigger
@@ -24,11 +20,7 @@ module TestHooks
     end
 
     def merge_requests_events_data
-      merge_request = if use_newest_record?
-                        MergeRequest.of_projects(current_user.projects.select(:id)).last
-                      else
-                        MergeRequest.of_projects(current_user.projects.select(:id)).first
-                      end
+      merge_request = MergeRequest.of_projects(current_user.projects.select(:id)).last
 
       return { error: s_('TestHooks|Ensure one of your projects has merge requests.') } unless merge_request.present?
 

changelogs/unreleased/290116-enable-webhook-test-reorder-feature.yml

+---
+title: Select the newest records for webhook test payloads
+merge_request: 54030
+author:
+type: changed

config/feature_flags/development/integrations_test_webhook_reorder.yml

----
-name: integrations_test_webhook_reorder
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53433
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320861
-milestone: '13.9'
-type: development
-group: group::ecosystem
-default_enabled: false

spec/services/integrations/test/project_service_spec.rb

@@ -146,21 +146,6 @@ RSpec.describe Integrations::Test::ProjectService do
           expect(integration).to receive(:test).with(sample_data).and_return(success_result)
           expect(subject).to eq(success_result)
         end
-
-        context 'when the reorder feature flag is disabled' do
-          before do
-            stub_feature_flags(integrations_test_webhook_reorder: false)
-          end
-
-          it 'executes the old query' do
-            allow(Gitlab::DataBuilder::Deployment).to receive(:build).and_return(sample_data)
-
-            expect(DeploymentsFinder).not_to receive(:new)
-            expect(project).to receive(:deployments).and_return([deployment])
-            expect(integration).to receive(:test).with(sample_data).and_return(success_result)
-            expect(subject).to eq(success_result)
-          end
-        end
       end
 
       context 'pipeline' do
@@ -179,21 +164,6 @@ RSpec.describe Integrations::Test::ProjectService do
           expect(integration).to receive(:test).with(sample_data).and_return(success_result)
           expect(subject).to eq(success_result)
         end
-
-        context 'when the reorder feature flag is disabled' do
-          before do
-            stub_feature_flags(integrations_test_webhook_reorder: false)
-          end
-
-          it 'executes the old query' do
-            create(:ci_empty_pipeline, project: project)
-            allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data)
-
-            expect(Ci::PipelinesFinder).not_to receive(:new)
-            expect(integration).to receive(:test).with(sample_data).and_return(success_result)
-            expect(subject).to eq(success_result)
-          end
-        end
       end
 
       context 'wiki_page' do

spec/services/test_hooks/project_service_spec.rb

@@ -140,21 +140,6 @@ RSpec.describe TestHooks::ProjectService do
         expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
-
-      context 'when the reorder feature flag is disabled' do
-        before do
-          stub_feature_flags(integrations_test_webhook_reorder: false)
-        end
-
-        it 'executes the old query' do
-          allow(Gitlab::DataBuilder::Build).to receive(:build).and_return(sample_data)
-
-          expect(Ci::JobsFinder).not_to receive(:new)
-          expect(project).to receive(:builds).and_return([ci_job])
-          expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
-          expect(service.execute).to include(success_result)
-        end
-      end
     end
 
     context 'pipeline_events' do
@@ -174,21 +159,6 @@ RSpec.describe TestHooks::ProjectService do
         expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
-
-      context 'when the reorder feature flag is disabled' do
-        before do
-          stub_feature_flags(integrations_test_webhook_reorder: false)
-        end
-
-        it 'executes the old query' do
-          create(:ci_empty_pipeline, project: project)
-          allow(Gitlab::DataBuilder::Pipeline).to receive(:build).and_return(sample_data)
-
-          expect(Ci::PipelinesFinder).not_to receive(:new)
-          expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
-          expect(service.execute).to include(success_result)
-        end
-      end
     end
 
     context 'wiki_page_events' do
@@ -234,21 +204,6 @@ RSpec.describe TestHooks::ProjectService do
         expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
-
-      context 'when the reorder feature flag is disabled' do
-        before do
-          stub_feature_flags(integrations_test_webhook_reorder: false)
-        end
-
-        it 'executes the old query' do
-          allow(release).to receive(:to_hook_data).and_return(sample_data)
-
-          expect(ReleasesFinder).not_to receive(:new)
-          expect(project).to receive(:releases).and_return([release])
-          expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
-          expect(service.execute).to include(success_result)
-        end
-      end
     end
   end
 end

spec/services/test_hooks/system_service_spec.rb

@@ -64,7 +64,7 @@ RSpec.describe TestHooks::SystemService do
     context 'merge_requests_events' do
       let(:trigger) { 'merge_requests_events' }
       let(:trigger_key) { :merge_request_hooks }
-      let(:merge_requests) { build_list(:merge_request, 2) }
+      let(:merge_request) { build(:merge_request) }
       let(:sample_data) { { data: 'sample' } }
 
       it 'returns error message if the user does not have any repository with a merge request' do
@@ -73,24 +73,11 @@ RSpec.describe TestHooks::SystemService do
       end
 
       it 'executes hook' do
-        expect(MergeRequest).to receive(:of_projects).and_return(merge_requests)
-        expect(merge_requests.last).to receive(:to_hook_data).and_return(sample_data)
+        expect(MergeRequest).to receive(:of_projects).and_return([merge_request])
+        expect(merge_request).to receive(:to_hook_data).and_return(sample_data)
         expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
         expect(service.execute).to include(success_result)
       end
-
-      context 'when the reorder feature flag is disabled' do
-        before do
-          stub_feature_flags(integrations_test_webhook_reorder: false)
-        end
-
-        it 'executes the old query' do
-          expect(MergeRequest).to receive(:of_projects).and_return(merge_requests)
-          expect(merge_requests.first).to receive(:to_hook_data).and_return(sample_data)
-          expect(hook).to receive(:execute).with(sample_data, trigger_key).and_return(success_result)
-          expect(service.execute).to include(success_result)
-        end
-      end
     end
   end
 end