Merge branch 'use-keyword-args-for-databuilder-push' into 'master'

Use keyword args for databuilder push

See merge request gitlab-org/gitlab-ce!24088

7 files changed, 38 insertions, 29 deletions

diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb
index a8478e3a904..9d371e234ee 100644
--- a/app/services/git/base_hooks_service.rb
+++ b/app/services/git/base_hooks_service.rb
@@ -73,13 +73,13 @@ module Git
 
     def push_data
       @push_data ||= Gitlab::DataBuilder::Push.build(
-        project,
-        current_user,
-        params[:oldrev],
-        params[:newrev],
-        params[:ref],
-        limited_commits,
-        event_message,
+        project: project,
+        user: current_user,
+        oldrev: params[:oldrev],
+        newrev: params[:newrev],
+        ref: params[:ref],
+        commits: limited_commits,
+        message: event_message,
         commits_count: commits_count,
         push_options: params[:push_options] || {}
       )
diff --git a/app/services/tags/destroy_service.rb b/app/services/tags/destroy_service.rb
index cab507946b4..4f6ae07be7d 100644
--- a/app/services/tags/destroy_service.rb
+++ b/app/services/tags/destroy_service.rb
@@ -41,12 +41,11 @@ module Tags
 
     def build_push_data(tag)
       Gitlab::DataBuilder::Push.build(
-        project,
-        current_user,
-        tag.dereferenced_target.sha,
-        Gitlab::Git::BLANK_SHA,
-        "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}",
-        [])
+        project: project,
+        user: current_user,
+        oldrev: tag.dereferenced_target.sha,
+        newrev: Gitlab::Git::BLANK_SHA,
+        ref: "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}")
     end
   end
 end
diff --git a/lib/gitlab/data_builder/push.rb b/lib/gitlab/data_builder/push.rb
index af385d7d4ca..40bda3410e1 100644
--- a/lib/gitlab/data_builder/push.rb
+++ b/lib/gitlab/data_builder/push.rb
@@ -58,7 +58,10 @@ module Gitlab
       # }
       #
       # rubocop:disable Metrics/ParameterLists
-      def build(project, user, oldrev, newrev, ref, commits = [], message = nil, commits_count: nil, push_options: {})
+      def build(
+          project:, user:, ref:, oldrev: nil, newrev: nil,
+          commits: [], commits_count: nil, message: nil, push_options: {})
+
         commits = Array(commits)
 
         # Total commits count
@@ -113,7 +116,12 @@ module Gitlab
         ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{project.default_branch}"
         commits = project.repository.commits(project.default_branch.to_s, limit: 3)
 
-        build(project, user, commits.last&.id, commits.first&.id, ref, commits)
+        build(project: project,
+              user: user,
+              oldrev: commits.last&.id,
+              newrev: commits.first&.id,
+              ref: ref,
+              commits: commits)
       end
 
       def sample_data
diff --git a/spec/lib/gitlab/data_builder/push_spec.rb b/spec/lib/gitlab/data_builder/push_spec.rb
index 0c4decc6518..46ad674a1eb 100644
--- a/spec/lib/gitlab/data_builder/push_spec.rb
+++ b/spec/lib/gitlab/data_builder/push_spec.rb
@@ -23,9 +23,12 @@ describe Gitlab::DataBuilder::Push do
 
   describe '.build' do
     let(:data) do
-      described_class.build(project, user, Gitlab::Git::BLANK_SHA,
-                            '8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b',
-                            'refs/tags/v1.1.0')
+      described_class.build(
+        project: project,
+        user: user,
+        oldrev: Gitlab::Git::BLANK_SHA,
+        newrev: '8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b',
+        ref: 'refs/tags/v1.1.0')
     end
 
     it { expect(data).to be_a(Hash) }
@@ -47,7 +50,7 @@ describe Gitlab::DataBuilder::Push do
     include_examples 'deprecated repository hook data'
 
     it 'does not raise an error when given nil commits' do
-      expect { described_class.build(spy, spy, spy, spy, 'refs/tags/v1.1.0', nil) }
+      expect { described_class.build(project: spy, user: spy, ref: 'refs/tags/v1.1.0', commits: nil) }
         .not_to raise_error
     end
   end
diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb
index fd9e33c1781..a04b984c1f6 100644
--- a/spec/models/project_services/hipchat_service_spec.rb
+++ b/spec/models/project_services/hipchat_service_spec.rb
@@ -98,12 +98,11 @@ describe HipchatService do
     context 'tag_push events' do
       let(:push_sample_data) do
         Gitlab::DataBuilder::Push.build(
-          project,
-          user,
-          Gitlab::Git::BLANK_SHA,
-          '1' * 40,
-          'refs/tags/test',
-          [])
+          project: project,
+          user: user,
+          oldrev: Gitlab::Git::BLANK_SHA,
+          newrev: '1' * 40,
+          ref: 'refs/tags/test')
       end
 
       it "calls Hipchat API for tag push events" do
diff --git a/spec/support/shared_examples/models/chat_service_spec.rb b/spec/support/shared_examples/models/chat_service_spec.rb
index 9d3ce5e2be1..0a302e7d030 100644
--- a/spec/support/shared_examples/models/chat_service_spec.rb
+++ b/spec/support/shared_examples/models/chat_service_spec.rb
@@ -70,7 +70,7 @@ shared_examples_for "chat service" do |service_name|
 
       context "with not default branch" do
         let(:sample_data) do
-          Gitlab::DataBuilder::Push.build(project, user, nil, nil, "not-the-default-branch")
+          Gitlab::DataBuilder::Push.build(project: project, user: user, ref: "not-the-default-branch")
         end
 
         context "when notify_only_default_branch enabled" do
diff --git a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb
index c31346374f4..36c486dbdd6 100644
--- a/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb
+++ b/spec/support/shared_examples/slack_mattermost_notifications_shared_examples.rb
@@ -275,7 +275,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do
 
         it 'does not notify push events if they are not for the default branch' do
           ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test"
-          push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, [])
+          push_sample_data = Gitlab::DataBuilder::Push.build(project: project, user: user, ref: ref)
 
           chat_service.execute(push_sample_data)
 
@@ -292,7 +292,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do
 
         it 'still notifies about pushed tags' do
           ref = "#{Gitlab::Git::TAG_REF_PREFIX}test"
-          push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, [])
+          push_sample_data = Gitlab::DataBuilder::Push.build(project: project, user: user, ref: ref)
 
           chat_service.execute(push_sample_data)
 
@@ -307,7 +307,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do
 
         it 'notifies about all push events' do
           ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}test"
-          push_sample_data = Gitlab::DataBuilder::Push.build(project, user, nil, nil, ref, [])
+          push_sample_data = Gitlab::DataBuilder::Push.build(project: project, user: user, ref: ref)
 
           chat_service.execute(push_sample_data)