From 663b3c968f73f8ffebf32059fed86192ecbee5d8 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 6 Mar 2015 17:14:19 +0100 Subject: [PATCH] Condense commits already in target branch when updating merge request source branch. --- CHANGELOG | 1 + app/models/note.rb | 31 ++++++++++++++++--- .../merge_requests/refresh_service.rb | 8 ++++- .../merge_requests/refresh_service_spec.rb | 2 +- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index b927b60140e..2cea709f167 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -26,6 +26,7 @@ v 7.9.0 (unreleased) - Add Bitbucket omniauth provider. - Add Bitbucket importer. - Support referencing issues to a project whose name starts with a digit + - Condense commits already in target branch when updating merge request source branch. v 7.8.2 - Fix service migration issue when upgrading from versions prior to 7.3 diff --git a/app/models/note.rb b/app/models/note.rb index e6c258ffbe9..e79b7a88344 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -151,18 +151,41 @@ class Note < ActiveRecord::Base ) end - def create_new_commits_note(noteable, project, author, commits) - commits_text = ActionController::Base.helpers.pluralize(commits.size, 'new commit') + def create_new_commits_note(merge_request, project, author, new_commits, existing_commits = []) + total_count = new_commits.length + existing_commits.length + commits_text = ActionController::Base.helpers.pluralize(total_count, 'commit') body = "Added #{commits_text}:\n\n" - commits.each do |commit| + if existing_commits.length > 0 + commit_ids = + if existing_commits.length == 1 + existing_commits.first.short_id + else + "#{existing_commits.first.short_id}...#{existing_commits.last.short_id}" + end + + commits_text = ActionController::Base.helpers.pluralize(existing_commits.length, 'commit') + + branch = + if merge_request.for_fork? + "#{merge_request.target_project_namespace}:#{merge_request.target_branch}" + else + merge_request.target_branch + end + + message = "* #{commit_ids} - _#{commits_text} from branch `#{branch}`_" + body << message + body << "\n" + end + + new_commits.each do |commit| message = "* #{commit.short_id} - #{commit.title}" body << message body << "\n" end create( - noteable: noteable, + noteable: merge_request, project: project, author: author, note: body, diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 96761bec99f..ea846472766 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -82,8 +82,14 @@ module MergeRequests merge_requests = filter_merge_requests(merge_requests) merge_requests.each do |merge_request| + mr_commit_ids = Set.new(merge_request.commits.map(&:id)) + + new_commits, existing_commits = @commits.partition do |commit| + mr_commit_ids.include?(commit.id) + end + Note.create_new_commits_note(merge_request, merge_request.project, - @current_user, @commits) + @current_user, new_commits, existing_commits) end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 2830da87814..879df0c9c67 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -61,7 +61,7 @@ describe MergeRequests::RefreshService do it { expect(@merge_request.notes).to be_empty } it { expect(@merge_request).to be_open } - it { expect(@fork_merge_request.notes.last.note).to include('new commit') } + it { expect(@fork_merge_request.notes.last.note).to include('Added 4 commits') } it { expect(@fork_merge_request).to be_open } end