From 5b432e76710eb70cc41c59af1ea9a294202a49fc Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 5 Dec 2014 22:56:43 +0100 Subject: [PATCH] Extend push_tag event to include tag message and last commit --- CHANGELOG | 1 + app/services/create_tag_service.rb | 3 +- app/services/git_push_service.rb | 60 +++++++++++----------- app/services/git_tag_push_service.rb | 18 ++++++- lib/gitlab/push_data_builder.rb | 3 +- spec/services/git_push_service_spec.rb | 5 -- spec/services/git_tag_push_service_spec.rb | 52 +++++++++++++++++-- 7 files changed, 97 insertions(+), 45 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4c644960088..176a3c833b7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ v 7.10.0 (unreleased) - Improve error message when save profile has error. - Passing the name of pushed ref to CI service (requires GitLab CI 7.9+) - Add location field to user profile + - Add tag message and last commit to tag hook (Kamil Trzciński) v 7.9.0 (unreleased) - Add HipChat integration documentation (Stan Hu) diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb index af4b537cb93..4115d689925 100644 --- a/app/services/create_tag_service.rb +++ b/app/services/create_tag_service.rb @@ -40,7 +40,8 @@ class CreateTagService < BaseService end def create_push_data(project, user, tag) + commits = [project.repository.commit(tag.target)].compact Gitlab::PushDataBuilder. - build(project, user, Gitlab::Git::BLANK_SHA, tag.target, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", []) + build(project, user, Gitlab::Git::BLANK_SHA, tag.target, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", commits, tag.message) end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index a0da07f5c90..1f0b29dff5e 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -23,42 +23,40 @@ class GitPushService project.repository.expire_cache project.update_repository_size - if push_to_branch?(ref) - if push_remove_branch?(ref, newrev) - @push_commits = [] - elsif push_to_new_branch?(ref, oldrev) - # Re-find the pushed commits. - if is_default_branch?(ref) - # Initial push to the default branch. Take the full history of that branch as "newly pushed". - @push_commits = project.repository.commits(newrev) + if push_remove_branch?(ref, newrev) + @push_commits = [] + elsif push_to_new_branch?(ref, oldrev) + # Re-find the pushed commits. + if is_default_branch?(ref) + # Initial push to the default branch. Take the full history of that branch as "newly pushed". + @push_commits = project.repository.commits(newrev) - # Set protection on the default branch if configured - if (current_application_settings.default_branch_protection != PROTECTION_NONE) - developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false - project.protected_branches.create({ name: project.default_branch, developers_can_push: developers_can_push }) - end - else - # Use the pushed commits that aren't reachable by the default branch - # as a heuristic. This may include more commits than are actually pushed, but - # that shouldn't matter because we check for existing cross-references later. - @push_commits = project.repository.commits_between(project.default_branch, newrev) - - # don't process commits for the initial push to the default branch - process_commit_messages(ref) + # Set protection on the default branch if configured + if (current_application_settings.default_branch_protection != PROTECTION_NONE) + developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false + project.protected_branches.create({ name: project.default_branch, developers_can_push: developers_can_push }) end - elsif push_to_existing_branch?(ref, oldrev) - # Collect data for this git push - @push_commits = project.repository.commits_between(oldrev, newrev) - project.update_merge_requests(oldrev, newrev, ref, @user) + else + # Use the pushed commits that aren't reachable by the default branch + # as a heuristic. This may include more commits than are actually pushed, but + # that shouldn't matter because we check for existing cross-references later. + @push_commits = project.repository.commits_between(project.default_branch, newrev) + + # don't process commits for the initial push to the default branch process_commit_messages(ref) end - - @push_data = build_push_data(oldrev, newrev, ref) - - EventCreateService.new.push(project, user, @push_data) - project.execute_hooks(@push_data.dup, :push_hooks) - project.execute_services(@push_data.dup, :push_hooks) + elsif push_to_existing_branch?(ref, oldrev) + # Collect data for this git push + @push_commits = project.repository.commits_between(oldrev, newrev) + project.update_merge_requests(oldrev, newrev, ref, @user) + process_commit_messages(ref) end + + @push_data = build_push_data(oldrev, newrev, ref) + + EventCreateService.new.push(project, user, @push_data) + project.execute_hooks(@push_data.dup, :push_hooks) + project.execute_services(@push_data.dup, :push_hooks) end protected diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 0d8e6e85e47..bf203bbd692 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -3,7 +3,7 @@ class GitTagPushService def execute(project, user, oldrev, newrev, ref) @project, @user = project, user - + @push_data = build_push_data(oldrev, newrev, ref) EventCreateService.new.push(project, user, @push_data) @@ -18,6 +18,20 @@ class GitTagPushService private def build_push_data(oldrev, newrev, ref) - Gitlab::PushDataBuilder.build(project, user, oldrev, newrev, ref, []) + commits = [] + message = nil + + if !Gitlab::Git.blank_ref?(newrev) + tag_name = Gitlab::Git.ref_name(ref) + tag = project.repository.find_tag(tag_name) + if tag && tag.target == newrev + commit = project.repository.commit(tag.target) + commits = [commit].compact + message = tag.message + end + end + + Gitlab::PushDataBuilder. + build(project, user, oldrev, newrev, ref, commits, message) end end diff --git a/lib/gitlab/push_data_builder.rb b/lib/gitlab/push_data_builder.rb index 948cf58fd9a..f8da452e4c0 100644 --- a/lib/gitlab/push_data_builder.rb +++ b/lib/gitlab/push_data_builder.rb @@ -21,7 +21,7 @@ module Gitlab # total_commits_count: Fixnum # } # - def build(project, user, oldrev, newrev, ref, commits = []) + def build(project, user, oldrev, newrev, ref, commits = [], message = nil) # Total commits count commits_count = commits.size @@ -42,6 +42,7 @@ module Gitlab after: newrev, ref: ref, checkout_sha: checkout_sha(project.repository, newrev, ref), + message: message, user_id: user.id, user_name: user.name, user_email: user.email, diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 1b1e3ca5f8b..aa9b15dd9ec 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -145,11 +145,6 @@ describe GitPushService do expect(project).to receive(:execute_hooks) service.execute(project, user, 'oldrev', 'newrev', 'refs/heads/master') end - - it "when pushing tags" do - expect(project).not_to receive(:execute_hooks) - service.execute(project, user, 'newrev', 'newrev', 'refs/tags/v1.0.0') - end end end diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index fcf462edbfc..a050fdf6c0e 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -1,32 +1,39 @@ require 'spec_helper' describe GitTagPushService do + include RepoHelpers + let (:user) { create :user } let (:project) { create :project } let (:service) { GitTagPushService.new } before do - @ref = 'refs/tags/super-tag' - @oldrev = 'b98a310def241a6fd9c9a9a3e7934c48e498fe81' - @newrev = 'b19a04f53caeebf4fe5ec2327cb83e9253dc91bb' + @oldrev = Gitlab::Git::BLANK_SHA + @newrev = "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" # gitlab-test: git rev-parse refs/tags/v1.1.0 + @ref = 'refs/tags/v1.1.0' end - describe 'Git Tag Push Data' do + describe "Git Tag Push Data" do before do service.execute(project, user, @oldrev, @newrev, @ref) @push_data = service.push_data + @tag_name = Gitlab::Git.ref_name(@ref) + @tag = project.repository.find_tag(@tag_name) + @commit = project.repository.commit(@tag.target) end subject { @push_data } + it { is_expected.to include(object_kind: 'tag_push') } it { is_expected.to include(ref: @ref) } it { is_expected.to include(before: @oldrev) } it { is_expected.to include(after: @newrev) } + it { is_expected.to include(message: @tag.message) } it { is_expected.to include(user_id: user.id) } it { is_expected.to include(user_name: user.name) } it { is_expected.to include(project_id: project.id) } - context 'With repository data' do + context "with repository data" do subject { @push_data[:repository] } it { is_expected.to include(name: project.name) } @@ -34,6 +41,41 @@ describe GitTagPushService do it { is_expected.to include(description: project.description) } it { is_expected.to include(homepage: project.web_url) } end + + context "with commits" do + subject { @push_data[:commits] } + + it { is_expected.to be_an(Array) } + it 'has 1 element' do + expect(subject.size).to eq(1) + end + + context "the commit" do + subject { @push_data[:commits].first } + + it { is_expected.to include(id: @commit.id) } + it { is_expected.to include(message: @commit.safe_message) } + it { is_expected.to include(timestamp: @commit.date.xmlschema) } + it do + is_expected.to include( + url: [ + Gitlab.config.gitlab.url, + project.namespace.to_param, + project.to_param, + 'commit', + @commit.id + ].join('/') + ) + end + + context "with a author" do + subject { @push_data[:commits].first[:author] } + + it { is_expected.to include(name: @commit.author_name) } + it { is_expected.to include(email: @commit.author_email) } + end + end + end end describe "Web Hooks" do