+v 6.1.0
+ - Link issues, merge requests, and commits when they reference each other with GFM
+ - Close issues automatically when pushing commits with a special message
+
v 6.0.0
- Feature: Replace teams with group membership
We introduce group membership in 6.0 as a replacement for teams.
- Notify mentioned users with email
v 5.3.0
- - Refactored services
+ - Refactored services
- Campfire service added
- HipChat service added
- Fixed bug with LDAP + git over http
class Projects::MergeRequestsController < Projects::ApplicationController
before_filter :module_enabled
before_filter :merge_request, only: [:edit, :update, :show, :commits, :diffs, :automerge, :automerge_check, :ci_status]
+ before_filter :closes_issues, only: [:edit, :update, :show, :commits, :diffs]
before_filter :validates_merge_request, only: [:show, :diffs]
before_filter :define_show_vars, only: [:show, :diffs]
@merge_request ||= @project.merge_requests.find_by_iid!(params[:id])
end
+ def closes_issues
+ @closes_issues ||= @merge_request.closes_issues
+ end
+
def authorize_modify_merge_request!
return render_404 unless can?(current_user, :modify_merge_request, @merge_request)
end
include ActiveModel::Conversion
include StaticModel
extend ActiveModel::Naming
+ include Mentionable
+
+ attr_mentionable :safe_message
# Safe amount of files with diffs in one commit to render
# Used to prevent 500 error on huge commits by suppressing diff
end
end
+ # Regular expression that identifies commit message clauses that trigger issue closing.
+ def issue_closing_regex
+ @issue_closing_regex ||= Regexp.new(Gitlab.config.gitlab.issue_closing_pattern)
+ end
+
+ # Discover issues should be closed when this commit is pushed to a project's
+ # default branch.
+ def closes_issues project
+ md = issue_closing_regex.match(safe_message)
+ if md
+ extractor = Gitlab::ReferenceExtractor.new
+ extractor.analyze(md[0])
+ extractor.issues_for(project)
+ else
+ []
+ end
+ end
+
+ # Mentionable override.
+ def gfm_reference
+ "commit #{sha[0..5]}"
+ end
+
def method_missing(m, *args, &block)
@raw.send(m, *args, &block)
end
# == Mentionable concern
#
-# Contains common functionality shared between Issues and Notes
+# Contains functionality related to objects that can mention Users, Issues, MergeRequests, or Commits by
+# GFM references.
#
-# Used by Issue, Note
+# Used by Issue, Note, MergeRequest, and Commit.
#
module Mentionable
extend ActiveSupport::Concern
+ module ClassMethods
+ # Indicate which attributes of the Mentionable to search for GFM references.
+ def attr_mentionable *attrs
+ mentionable_attrs.concat(attrs.map(&:to_s))
+ end
+
+ # Accessor for attributes marked mentionable.
+ def mentionable_attrs
+ @mentionable_attrs ||= []
+ end
+ end
+
+ # Generate a GFM back-reference that will construct a link back to this Mentionable when rendered. Must
+ # be overridden if this model object can be referenced directly by GFM notation.
+ def gfm_reference
+ raise NotImplementedError.new("#{self.class} does not implement #gfm_reference")
+ end
+
+ # Construct a String that contains possible GFM references.
+ def mentionable_text
+ self.class.mentionable_attrs.map { |attr| send(attr) || '' }.join
+ end
+
+ # The GFM reference to this Mentionable, which shouldn't be included in its #references.
+ def local_reference
+ self
+ end
+
+ # Determine whether or not a cross-reference Note has already been created between this Mentionable and
+ # the specified target.
+ def has_mentioned? target
+ Note.cross_reference_exists?(target, local_reference)
+ end
+
def mentioned_users
users = []
return users if mentionable_text.blank?
users.uniq
end
- def mentionable_text
- if self.class == Issue
- description
- elsif self.class == Note
- note
- else
- nil
+ # Extract GFM references to other Mentionables from this Mentionable. Always excludes its #local_reference.
+ def references p = project, text = mentionable_text
+ return [] if text.blank?
+ ext = Gitlab::ReferenceExtractor.new
+ ext.analyze(text)
+ (ext.issues_for(p) + ext.merge_requests_for(p) + ext.commits_for(p)).uniq - [local_reference]
+ end
+
+ # Create a cross-reference Note for each GFM reference to another Mentionable found in +mentionable_text+.
+ def create_cross_references! p = project, a = author, without = []
+ refs = references(p) - without
+ refs.each do |ref|
+ Note.create_cross_reference_note(ref, local_reference, a, p)
end
end
+ # If the mentionable_text field is about to change, locate any *added* references and create cross references for
+ # them. Invoke from an observer's #before_save implementation.
+ def notice_added_references p = project, a = author
+ ch = changed_attributes
+ original, mentionable_changed = "", false
+ self.class.mentionable_attrs.each do |attr|
+ if ch[attr]
+ original << ch[attr]
+ mentionable_changed = true
+ end
+ end
+
+ # Only proceed if the saved changes actually include a chance to an attr_mentionable field.
+ return unless mentionable_changed
+
+ preexisting = references(p, original)
+ create_cross_references!(p, a, preexisting)
+ end
+
end
attr_accessible :title, :assignee_id, :position, :description,
:milestone_id, :label_list, :author_id_of_changes,
:state_event
+ attr_mentionable :title, :description
acts_as_taggable_on :labels
# Both open and reopened issues should be listed as opened
scope :opened, -> { with_state(:opened, :reopened) }
+
+ # Mentionable overrides.
+
+ def gfm_reference
+ "issue ##{iid}"
+ end
end
belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"
attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event
-
+ attr_mentionable :title
attr_accessor :should_remove_source_branch
target_project
end
+ # Return the set of issues that will be closed if this merge request is accepted.
+ def closes_issues
+ if target_branch == project.default_branch
+ unmerged_commits.map { |c| c.closes_issues(project) }.flatten.uniq.sort_by(&:id)
+ else
+ []
+ end
+ end
+
+ # Mentionable override.
+ def gfm_reference
+ "merge request !#{iid}"
+ end
+
private
def dump_commits(commits)
attr_accessible :note, :noteable, :noteable_id, :noteable_type, :project_id,
:attachment, :line_code, :commit_id
+ attr_mentionable :note
belongs_to :project
belongs_to :noteable, polymorphic: true
serialize :st_diff
before_create :set_diff, if: ->(n) { n.line_code.present? }
- def self.create_status_change_note(noteable, project, author, status)
+ def self.create_status_change_note(noteable, project, author, status, source)
+ body = "_Status changed to #{status}#{' by ' + source.gfm_reference if source}_"
+
+ create({
+ noteable: noteable,
+ project: project,
+ author: author,
+ note: body,
+ system: true
+ }, without_protection: true)
+ end
+
+ # +noteable+ was referenced from +mentioner+, by including GFM in either +mentioner+'s description or an associated Note.
+ # Create a system Note associated with +noteable+ with a GFM back-reference to +mentioner+.
+ def self.create_cross_reference_note(noteable, mentioner, author, project)
create({
noteable: noteable,
+ commit_id: (noteable.sha if noteable.respond_to? :sha),
project: project,
author: author,
- note: "_Status changed to #{status}_"
+ note: "_mentioned in #{mentioner.gfm_reference}_",
+ system: true
}, without_protection: true)
end
+ # Determine whether or not a cross-reference note already exists.
+ def self.cross_reference_exists?(noteable, mentioner)
+ where(noteable_id: noteable.id, system: true, note: "_mentioned in #{mentioner.gfm_reference}_").any?
+ end
+
def commit_author
@commit_author ||=
project.users.find_by_email(noteable.author_email) ||
for_issue? || (for_merge_request? && !for_diff_line?)
end
+ # Mentionable override.
+ def gfm_reference
+ noteable.gfm_reference
+ end
+
+ # Mentionable override.
+ def local_reference
+ noteable
+ end
+
def noteable_type_name
if noteable_type.present?
noteable_type.downcase
end
def commit(id = nil)
+ return nil unless raw_repository
commit = Gitlab::Git::Commit.find(raw_repository, id)
commit = Commit.new(commit) if commit
commit
event_author_id = record.author_id
if record.kind_of?(Note)
- # Skip system status notes like 'status changed to close'
- return true if record.note.include?("_Status changed to ")
+ # Skip system notes, like status changes and cross-references.
+ return true if record.system?
# Skip wall notes to prevent spamming of dashboard
return true if record.noteable_type.blank?
def current_user
Thread.current[:current_user]
end
+
+ def current_commit
+ Thread.current[:current_commit]
+ end
end
class IssueObserver < BaseObserver
def after_create(issue)
notification.new_issue(issue, current_user)
+
+ issue.create_cross_references!(issue.project, current_user)
end
def after_close(issue, transition)
if issue.is_being_reassigned?
notification.reassigned_issue(issue, current_user)
end
+
+ issue.notice_added_references(issue.project, current_user)
end
protected
# Create issue note with service comment like 'Status changed to closed'
def create_note(issue)
- Note.create_status_change_note(issue, issue.project, current_user, issue.state)
+ Note.create_status_change_note(issue, issue.project, current_user, issue.state, current_commit)
end
end
end
notification.new_merge_request(merge_request, current_user)
+
+ merge_request.create_cross_references!(merge_request.project, current_user)
end
def after_close(merge_request, transition)
create_event(merge_request, Event::CLOSED)
- Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state)
+ Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
notification.close_mr(merge_request, current_user)
end
def after_reopen(merge_request, transition)
create_event(merge_request, Event::REOPENED)
- Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state)
+ Note.create_status_change_note(merge_request, merge_request.target_project, current_user, merge_request.state, nil)
end
def after_update(merge_request)
notification.reassigned_merge_request(merge_request, current_user) if merge_request.is_being_reassigned?
+
+ merge_request.notice_added_references(merge_request.project, current_user)
end
def create_event(record, status)
class NoteObserver < BaseObserver
def after_create(note)
notification.new_note(note)
+
+ unless note.system?
+ # Create a cross-reference note if this Note contains GFM that names an
+ # issue, merge request, or commit.
+ note.references.each do |mentioned|
+ Note.create_cross_reference_note(mentioned, note.noteable, note.author, note.project)
+ end
+ end
+ end
+
+ def after_update(note)
+ note.notice_added_references(note.project, current_user)
end
end
class GitPushService
- attr_accessor :project, :user, :push_data
+ attr_accessor :project, :user, :push_data, :push_commits
# This method will be called after each git update
# and only if the provided user and project is present in GitLab.
#
# All callbacks for post receive action should be placed here.
#
- # Now this method do next:
- # 1. Ensure project satellite exists
- # 2. Update merge requests
- # 3. Execute project web hooks
- # 4. Execute project services
- # 5. Create Push Event
+ # Next, this method:
+ # 1. Creates the push event
+ # 2. Ensures that the project satellite exists
+ # 3. Updates merge requests
+ # 4. Recognizes cross-references from commit messages
+ # 5. Executes the project's web hooks
+ # 6. Executes the project's services
#
def execute(project, user, oldrev, newrev, ref)
@project, @user = project, user
# Collect data for this git push
+ @push_commits = project.repository.commits_between(oldrev, newrev)
@push_data = post_receive_data(oldrev, newrev, ref)
create_push_event
project.discover_default_branch
project.repository.expire_cache
- if push_to_branch?(ref, oldrev)
+ if push_to_existing_branch?(ref, oldrev)
project.update_merge_requests(oldrev, newrev, ref, @user)
+ process_commit_messages(ref)
project.execute_hooks(@push_data.dup)
project.execute_services(@push_data.dup)
end
+
+ if 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)
+ 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)
+ end
+
+ process_commit_messages(ref)
+ end
end
# This method provide a sample data
protected
def create_push_event
- Event.create(
+ Event.create!(
project: project,
action: Event::PUSHED,
data: push_data,
)
end
+ # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched,
+ # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables.
+ def process_commit_messages ref
+ is_default_branch = is_default_branch?(ref)
+
+ @push_commits.each do |commit|
+ # Close issues if these commits were pushed to the project's default branch and the commit message matches the
+ # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to
+ # a different branch.
+ issues_to_close = commit.closes_issues(project)
+ author = commit_user(commit)
+
+ if !issues_to_close.empty? && is_default_branch
+ Thread.current[:current_user] = author
+ Thread.current[:current_commit] = commit
+
+ issues_to_close.each { |i| i.close && i.save }
+ end
+
+ # Create cross-reference notes for any other references. Omit any issues that were referenced in an
+ # issue-closing phrase, or have already been mentioned from this commit (probably from this commit
+ # being pushed to a different branch).
+ refs = commit.references(project) - issues_to_close
+ refs.reject! { |r| commit.has_mentioned?(r) }
+ refs.each do |r|
+ Note.create_cross_reference_note(r, commit, author, project)
+ end
+ end
+ end
+
# Produce a hash of post-receive data
#
# data = {
# }
#
def post_receive_data(oldrev, newrev, ref)
- push_commits = project.repository.commits_between(oldrev, newrev)
-
# Total commits count
push_commits_count = push_commits.size
data
end
- def push_to_branch? ref, oldrev
+ def push_to_existing_branch? ref, oldrev
ref_parts = ref.split('/')
# Return if this is not a push to a branch (e.g. new commits)
- !(ref_parts[1] !~ /heads/ || oldrev == "00000000000000000000000000000000")
+ ref_parts[1] =~ /heads/ && oldrev != "0000000000000000000000000000000000000000"
+ end
+
+ def push_to_new_branch? ref, oldrev
+ ref_parts = ref.split('/')
+
+ ref_parts[1] =~ /heads/ && oldrev == "0000000000000000000000000000000000000000"
+ end
+
+ def is_default_branch? ref
+ ref == "refs/heads/#{project.default_branch}"
+ end
+
+ def commit_user commit
+ User.where(email: commit.author_email).first ||
+ User.where(name: commit.author_name).first ||
+ user
end
end
%i.icon-ok
Merged by #{link_to_member(@project, @merge_request.merge_event.author)}
#{time_ago_in_words(@merge_request.merge_event.created_at)} ago.
-
+ - if !@closes_issues.empty? && @merge_request.opened?
+ .ui-box-bottom.alert-info
+ %span
+ %i.icon-ok
+ Accepting this merge request will close #{@closes_issues.size == 1 ? 'issue' : 'issues'}
+ = succeed '.' do
+ != gfm(@closes_issues.map { |i| "##{i.iid}" }.to_sentence)
## Users management
# signup_enabled: true # default: false - Account passwords are not sent via the email if signup is enabled.
+ ## Automatic issue closing
+ # If a commit message matches this regular express, all issues referenced from the matched text will be closed
+ # if it's pushed to a project's default branch.
+ # issue_closing_pattern: "^([Cc]loses|[Ff]ixes) +#\d+"
+
## Default project features settings
default_projects_features:
issues: true
end
Settings.gitlab['signup_enabled'] ||= false
Settings.gitlab['username_changing_enabled'] = true if Settings.gitlab['username_changing_enabled'].nil?
+Settings.gitlab['issue_closing_pattern'] = '^([Cc]loses|[Ff]ixes) #(\d+)' if Settings.gitlab['issue_closing_pattern'].nil?
Settings.gitlab['default_projects_features'] ||= {}
Settings.gitlab.default_projects_features['issues'] = true if Settings.gitlab.default_projects_features['issues'].nil?
Settings.gitlab.default_projects_features['merge_requests'] = true if Settings.gitlab.default_projects_features['merge_requests'].nil?
--- /dev/null
+class AddSystemToNotes < ActiveRecord::Migration
+ class Note < ActiveRecord::Base
+ end
+
+ def up
+ add_column :notes, :system, :boolean, default: false, null: false
+
+ Note.reset_column_information
+ Note.update_all(system: false)
+ Note.where("note like '_status changed to%'").update_all(system: true)
+ end
+
+ def down
+ remove_column :notes, :system
+ end
+end
t.string "commit_id"
t.integer "noteable_id"
t.text "st_diff"
+ t.boolean "system", :default => false, :null => false
end
add_index "notes", ["author_id"], :name => "index_notes_on_author_id"
--- /dev/null
+module Gitlab
+ # Extract possible GFM references from an arbitrary String for further processing.
+ class ReferenceExtractor
+ attr_accessor :users, :issues, :merge_requests, :snippets, :commits
+
+ include Markdown
+
+ def initialize
+ @users, @issues, @merge_requests, @snippets, @commits = [], [], [], [], []
+ end
+
+ def analyze string
+ parse_references(string.dup)
+ end
+
+ # Given a valid project, resolve the extracted identifiers of the requested type to
+ # model objects.
+
+ def users_for project
+ users.map do |identifier|
+ project.users.where(username: identifier).first
+ end.reject(&:nil?)
+ end
+
+ def issues_for project
+ issues.map do |identifier|
+ project.issues.where(iid: identifier).first
+ end.reject(&:nil?)
+ end
+
+ def merge_requests_for project
+ merge_requests.map do |identifier|
+ project.merge_requests.where(iid: identifier).first
+ end.reject(&:nil?)
+ end
+
+ def snippets_for project
+ snippets.map do |identifier|
+ project.snippets.where(id: identifier).first
+ end.reject(&:nil?)
+ end
+
+ def commits_for project
+ repo = project.repository
+ return [] if repo.nil?
+
+ commits.map do |identifier|
+ repo.commit(identifier)
+ end.reject(&:nil?)
+ end
+
+ private
+
+ def reference_link type, identifier
+ # Append identifier to the appropriate collection.
+ send("#{type}s") << identifier
+ end
+ end
+end
--- /dev/null
+require 'spec_helper'
+
+describe Gitlab::ReferenceExtractor do
+ it 'extracts username references' do
+ subject.analyze "this contains a @user reference"
+ subject.users.should == ["user"]
+ end
+
+ it 'extracts issue references' do
+ subject.analyze "this one talks about issue #1234"
+ subject.issues.should == ["1234"]
+ end
+
+ it 'extracts merge request references' do
+ subject.analyze "and here's !43, a merge request"
+ subject.merge_requests.should == ["43"]
+ end
+
+ it 'extracts snippet ids' do
+ subject.analyze "snippets like $12 get extracted as well"
+ subject.snippets.should == ["12"]
+ end
+
+ it 'extracts commit shas' do
+ subject.analyze "commit shas 98cf0ae3 are pulled out as Strings"
+ subject.commits.should == ["98cf0ae3"]
+ end
+
+ it 'extracts multiple references and preserves their order' do
+ subject.analyze "@me and @you both care about this"
+ subject.users.should == ["me", "you"]
+ end
+
+ it 'leaves the original note unmodified' do
+ text = "issue #123 is just the worst, @user"
+ subject.analyze text
+ text.should == "issue #123 is just the worst, @user"
+ end
+
+ it 'handles all possible kinds of references' do
+ accessors = Gitlab::Markdown::TYPES.map { |t| "#{t}s".to_sym }
+ subject.should respond_to(*accessors)
+ end
+
+ context 'with a project' do
+ let(:project) { create(:project_with_code) }
+
+ it 'accesses valid user objects on the project team' do
+ @u_foo = create(:user, username: 'foo')
+ @u_bar = create(:user, username: 'bar')
+ create(:user, username: 'offteam')
+
+ project.team << [@u_foo, :reporter]
+ project.team << [@u_bar, :guest]
+
+ subject.analyze "@foo, @baduser, @bar, and @offteam"
+ subject.users_for(project).should == [@u_foo, @u_bar]
+ end
+
+ it 'accesses valid issue objects' do
+ @i0 = create(:issue, project: project)
+ @i1 = create(:issue, project: project)
+
+ subject.analyze "##{@i0.iid}, ##{@i1.iid}, and #999."
+ subject.issues_for(project).should == [@i0, @i1]
+ end
+
+ it 'accesses valid merge requests' do
+ @m0 = create(:merge_request, source_project: project, target_project: project, source_branch: 'aaa')
+ @m1 = create(:merge_request, source_project: project, target_project: project, source_branch: 'bbb')
+
+ subject.analyze "!999, !#{@m1.iid}, and !#{@m0.iid}."
+ subject.merge_requests_for(project).should == [@m1, @m0]
+ end
+
+ it 'accesses valid snippets' do
+ @s0 = create(:project_snippet, project: project)
+ @s1 = create(:project_snippet, project: project)
+ @s2 = create(:project_snippet)
+
+ subject.analyze "$#{@s0.id}, $999, $#{@s2.id}, $#{@s1.id}"
+ subject.snippets_for(project).should == [@s0, @s1]
+ end
+
+ it 'accesses valid commits' do
+ commit = project.repository.commit("master")
+
+ subject.analyze "this references commits #{commit.sha[0..6]} and 012345"
+ extracted = subject.commits_for(project)
+ extracted.should have(1).item
+ extracted[0].sha.should == commit.sha
+ extracted[0].message.should == commit.message
+ end
+ end
+end
require 'spec_helper'
describe Commit do
- let(:commit) { create(:project_with_code).repository.commit }
+ let(:project) { create :project_with_code }
+ let(:commit) { project.repository.commit }
describe '#title' do
it "returns no_commit_message when safe_message is blank" do
it { should respond_to(:id) }
it { should respond_to(:to_patch) }
end
+
+ describe '#closes_issues' do
+ let(:issue) { create :issue, project: project }
+
+ it 'detects issues that this commit is marked as closing' do
+ commit.stub(issue_closing_regex: /^([Cc]loses|[Ff]ixes) #\d+/, safe_message: "Fixes ##{issue.iid}")
+ commit.closes_issues(project).should == [issue]
+ end
+ end
+
+ it_behaves_like 'a mentionable' do
+ let(:subject) { commit }
+ let(:mauthor) { create :user, email: commit.author_email }
+ let(:backref_text) { "commit #{subject.sha[0..5]}" }
+ let(:set_mentionable_text) { ->(txt){ subject.stub(safe_message: txt) } }
+
+ # Include the subject in the repository stub.
+ let(:extra_commits) { [subject] }
+ end
end
Issue.open_for(user).count.should eq 2
end
end
+
+ it_behaves_like 'an editable mentionable' do
+ let(:subject) { create :issue, project: mproject }
+ let(:backref_text) { "issue ##{subject.iid}" }
+ let(:set_mentionable_text) { ->(txt){ subject.description = txt } }
+ end
end
it { should include_module(Issuable) }
end
-
describe "#mr_and_commit_notes" do
let!(:merge_request) { create(:merge_request) }
end
end
+ describe 'detection of issues to be closed' do
+ let(:issue0) { create :issue, project: subject.project }
+ let(:issue1) { create :issue, project: subject.project }
+ let(:commit0) { mock('commit0', closes_issues: [issue0]) }
+ let(:commit1) { mock('commit1', closes_issues: [issue0]) }
+ let(:commit2) { mock('commit2', closes_issues: [issue1]) }
+
+ before do
+ subject.stub(unmerged_commits: [commit0, commit1, commit2])
+ end
+
+ it 'accesses the set of issues that will be closed on acceptance' do
+ subject.project.default_branch = subject.target_branch
+
+ subject.closes_issues.should == [issue0, issue1].sort_by(&:id)
+ end
+
+ it 'only lists issues as to be closed if it targets the default branch' do
+ subject.project.default_branch = 'master'
+ subject.target_branch = 'something-else'
+
+ subject.closes_issues.should be_empty
+ end
+ end
+
+ it_behaves_like 'an editable mentionable' do
+ let(:subject) { create :merge_request, source_project: mproject, target_project: mproject }
+ let(:backref_text) { "merge request !#{subject.iid}" }
+ let(:set_mentionable_text) { ->(txt){ subject.title = txt } }
+ end
end
end
let(:project) { create(:project) }
- let(:commit) { project.repository.commit }
describe "Commit notes" do
let!(:note) { create(:note_on_commit, note: "+1 from me") }
describe "Merge request notes" do
let!(:note) { create(:note_on_merge_request, note: "+1 from me") }
- it "should not be votable" do
+ it "should be votable" do
note.should be_votable
end
end
let(:author) { create(:user) }
let(:status) { 'new_status' }
- subject { Note.create_status_change_note(thing, project, author, status) }
+ subject { Note.create_status_change_note(thing, project, author, status, nil) }
it 'creates and saves a Note' do
should be_a Note
its(:project) { should == thing.project }
its(:author) { should == author }
its(:note) { should =~ /Status changed to #{status}/ }
+
+ it 'appends a back-reference if a closing mentionable is supplied' do
+ commit = double('commit', gfm_reference: 'commit 123456')
+ n = Note.create_status_change_note(thing, project, author, status, commit)
+
+ n.note.should =~ /Status changed to #{status} by commit 123456/
+ end
+ end
+
+ describe '#create_cross_reference_note' do
+ let(:project) { create(:project_with_code) }
+ let(:author) { create(:user) }
+ let(:issue) { create(:issue, project: project) }
+ let(:mergereq) { create(:merge_request, target_project: project) }
+ let(:commit) { project.repository.commit }
+
+ # Test all of {issue, merge request, commit} in both the referenced and referencing
+ # roles, to ensure that the correct information can be inferred from any argument.
+
+ context 'issue from a merge request' do
+ subject { Note.create_cross_reference_note(issue, mergereq, author, project) }
+
+ it { should be_valid }
+ its(:noteable) { should == issue }
+ its(:project) { should == issue.project }
+ its(:author) { should == author }
+ its(:note) { should == "_mentioned in merge request !#{mergereq.iid}_" }
+ end
+
+ context 'issue from a commit' do
+ subject { Note.create_cross_reference_note(issue, commit, author, project) }
+
+ it { should be_valid }
+ its(:noteable) { should == issue }
+ its(:note) { should == "_mentioned in commit #{commit.sha[0..5]}_" }
+ end
+
+ context 'merge request from an issue' do
+ subject { Note.create_cross_reference_note(mergereq, issue, author, project) }
+
+ it { should be_valid }
+ its(:noteable) { should == mergereq }
+ its(:project) { should == mergereq.project }
+ its(:note) { should == "_mentioned in issue ##{issue.iid}_" }
+ end
+
+ context 'commit from a merge request' do
+ subject { Note.create_cross_reference_note(commit, mergereq, author, project) }
+
+ it { should be_valid }
+ its(:noteable) { should == commit }
+ its(:project) { should == project }
+ its(:note) { should == "_mentioned in merge request !#{mergereq.iid}_" }
+ end
+ end
+
+ describe '#cross_reference_exists?' do
+ let(:project) { create :project }
+ let(:author) { create :user }
+ let(:issue) { create :issue }
+ let(:commit0) { double 'commit0', gfm_reference: 'commit 123456' }
+ let(:commit1) { double 'commit1', gfm_reference: 'commit 654321' }
+
+ before do
+ Note.create_cross_reference_note(issue, commit0, author, project)
+ end
+
+ it 'detects if a mentionable has already been mentioned' do
+ Note.cross_reference_exists?(issue, commit0).should be_true
+ end
+
+ it 'detects if a mentionable has not already been mentioned' do
+ Note.cross_reference_exists?(issue, commit1).should be_false
+ end
+ end
+
+ describe '#system?' do
+ let(:project) { create(:project) }
+ let(:issue) { create(:issue, project: project) }
+ let(:other) { create(:issue, project: project) }
+ let(:author) { create(:user) }
+
+ it 'should recognize user-supplied notes as non-system' do
+ @note = create(:note_on_issue)
+ @note.should_not be_system
+ end
+
+ it 'should identify status-change notes as system notes' do
+ @note = Note.create_status_change_note(issue, project, author, 'closed', nil)
+ @note.should be_system
+ end
+
+ it 'should identify cross-reference notes as system notes' do
+ @note = Note.create_cross_reference_note(issue, other, author, project)
+ @note.should be_system
+ end
end
describe :authorization do
it { @abilities.allowed?(@u3, :admin_note, @p1).should be_false }
end
end
+
+ it_behaves_like 'an editable mentionable' do
+ let(:issue) { create :issue, project: project }
+ let(:subject) { create :note, noteable: issue, project: project }
+ let(:backref_text) { issue.gfm_reference }
+ let(:set_mentionable_text) { ->(txt) { subject.note = txt } }
+ end
end
describe ActivityObserver do
let(:project) { create(:project) }
+ before { Thread.current[:current_user] = create(:user) }
+
def self.it_should_be_valid_event
it { @event.should_not be_nil }
it { @event.project.should == project }
it { @event.action.should == Event::COMMENTED }
it { @event.target.should == @note }
end
+
+ describe "Ignore system notes" do
+ let(:author) { create(:user) }
+ let!(:issue) { create(:issue, project: project) }
+ let!(:other) { create(:issue) }
+
+ it "should not create events for status change notes" do
+ expect do
+ Note.observers.enable :activity_observer do
+ Note.create_status_change_note(issue, project, author, 'reopened', nil)
+ end
+ end.to_not change { Event.count }
+ end
+
+ it "should not create events for cross-reference notes" do
+ expect do
+ Note.observers.enable :activity_observer do
+ Note.create_cross_reference_note(issue, other, author, issue.project)
+ end
+ end.to_not change { Event.count }
+ end
+ end
end
before { subject.stub(:current_user).and_return(some_user) }
+ before { subject.stub(:current_commit).and_return(nil) }
before { subject.stub(notification: mock('NotificationService').as_null_object) }
-
+ before { mock_issue.project.stub_chain(:repository, :commit).and_return(nil) }
subject { IssueObserver.instance }
subject.after_create(mock_issue)
end
+
+ it 'should create cross-reference notes' do
+ other_issue = create(:issue)
+ mock_issue.stub(references: [other_issue])
+
+ Note.should_receive(:create_cross_reference_note).with(other_issue, mock_issue,
+ some_user, mock_issue.project)
+ subject.after_create(mock_issue)
+ end
end
context '#after_close' do
before { mock_issue.stub(state: 'closed') }
it 'note is created if the issue is being closed' do
- Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'closed')
+ Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'closed', nil)
subject.after_close(mock_issue, nil)
end
subject.notification.should_receive(:close_issue).with(mock_issue, some_user)
subject.after_close(mock_issue, nil)
end
+
+ it 'appends a mention to the closing commit if one is present' do
+ commit = double('commit', gfm_reference: 'commit 123456')
+ subject.stub(current_commit: commit)
+
+ Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'closed', commit)
+
+ subject.after_close(mock_issue, nil)
+ end
end
context 'a status "reopened"' do
before { mock_issue.stub(state: 'reopened') }
it 'note is created if the issue is being reopened' do
- Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'reopened')
+ Note.should_receive(:create_status_change_note).with(mock_issue, mock_issue.project, some_user, 'reopened', nil)
+
subject.after_reopen(mock_issue, nil)
end
end
subject.after_update(mock_issue)
end
end
+
+ context 'cross-references' do
+ it 'notices added references' do
+ mock_issue.should_receive(:notice_added_references)
+
+ subject.after_update(mock_issue)
+ end
+ end
end
end
let(:assignee) { create :user }
let(:author) { create :user }
let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author) }
- let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author) }
- let(:unassigned_mr) { create(:merge_request, author: author) }
- let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author) }
- let(:closed_unassigned_mr) { create(:closed_merge_request, author: author) }
+ let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author, target_project: create(:project)) }
+ let(:unassigned_mr) { create(:merge_request, author: author, target_project: create(:project)) }
+ let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author, target_project: create(:project)) }
+ let(:closed_unassigned_mr) { create(:closed_merge_request, author: author, target_project: create(:project)) }
before { subject.stub(:current_user).and_return(some_user) }
before { subject.stub(notification: mock('NotificationService').as_null_object) }
before { mr_mock.stub(:author_id) }
before { mr_mock.stub(:target_project) }
+ before { mr_mock.stub(:source_project) }
+ before { mr_mock.stub(:project) }
+ before { mr_mock.stub(:create_cross_references!).and_return(true) }
+ before { Repository.any_instance.stub(commit: nil) }
+
before(:each) { enable_observers }
after(:each) { disable_observers }
subject.should_receive(:notification)
subject.after_create(mr_mock)
end
+
+ it 'creates cross-reference notes' do
+ project = create :project
+ mr_mock.stub(title: "this mr references !#{assigned_mr.id}", project: project)
+ mr_mock.should_receive(:create_cross_references!).with(project, some_user)
+
+ subject.after_create(mr_mock)
+ end
end
context '#after_update' do
before(:each) do
mr_mock.stub(:is_being_reassigned?).and_return(false)
+ mr_mock.stub(:notice_added_references)
end
it 'is called when a merge request is changed' do
end
end
+ it 'checks for new references' do
+ mr_mock.should_receive(:notice_added_references)
+
+ subject.after_update(mr_mock)
+ end
+
context 'a notification' do
it 'is sent if the merge request is being reassigned' do
mr_mock.should_receive(:is_being_reassigned?).and_return(true)
context '#after_close' do
context 'a status "closed"' do
it 'note is created if the merge request is being closed' do
- Note.should_receive(:create_status_change_note).with(assigned_mr, assigned_mr.target_project, some_user, 'closed')
+ Note.should_receive(:create_status_change_note).with(assigned_mr, assigned_mr.target_project, some_user, 'closed', nil)
assigned_mr.close
end
it 'notification is delivered only to author if the merge request is being closed' do
- Note.should_receive(:create_status_change_note).with(unassigned_mr, unassigned_mr.target_project, some_user, 'closed')
+ Note.should_receive(:create_status_change_note).with(unassigned_mr, unassigned_mr.target_project, some_user, 'closed', nil)
unassigned_mr.close
end
context '#after_reopen' do
context 'a status "reopened"' do
it 'note is created if the merge request is being reopened' do
- Note.should_receive(:create_status_change_note).with(closed_assigned_mr, closed_assigned_mr.target_project, some_user, 'reopened')
+ Note.should_receive(:create_status_change_note).with(closed_assigned_mr, closed_assigned_mr.target_project, some_user, 'reopened', nil)
closed_assigned_mr.reopen
end
it 'notification is delivered only to author if the merge request is being reopened' do
- Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, closed_unassigned_mr.target_project, some_user, 'reopened')
+ Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, closed_unassigned_mr.target_project, some_user, 'reopened', nil)
closed_unassigned_mr.reopen
end
before { subject.stub(notification: mock('NotificationService').as_null_object) }
let(:team_without_author) { (1..2).map { |n| double :user, id: n } }
+ let(:note) { double(:note).as_null_object }
describe '#after_create' do
- let(:note) { double :note }
it 'is called after a note is created' do
subject.should_receive :after_create
subject.after_create(note)
end
+
+ it 'creates cross-reference notes as appropriate' do
+ @p = create(:project)
+ @referenced = create(:issue, project: @p)
+ @referencer = create(:issue, project: @p)
+ @author = create(:user)
+
+ Note.should_receive(:create_cross_reference_note).with(@referenced, @referencer, @author, @p)
+
+ Note.observers.enable :note_observer do
+ create(:note, project: @p, author: @author, noteable: @referencer,
+ note: "Duplicate of ##{@referenced.iid}")
+ end
+ end
+
+ it "doesn't cross-reference system notes" do
+ Note.should_receive(:create_cross_reference_note).once
+
+ Note.observers.enable :note_observer do
+ Note.create_cross_reference_note(create(:issue), create(:issue))
+ end
+ end
+ end
+
+ describe '#after_update' do
+ it 'checks for new cross-references' do
+ note.should_receive(:notice_added_references)
+
+ subject.after_update(note)
+ end
end
end
let (:service) { GitPushService.new }
before do
+ @blankrev = '0000000000000000000000000000000000000000'
@oldrev = 'b98a310def241a6fd9c9a9a3e7934c48e498fe81'
@newrev = 'b19a04f53caeebf4fe5ec2327cb83e9253dc91bb'
@ref = 'refs/heads/master'
it "when pushing a branch for the first time" do
@project_hook.should_not_receive(:execute)
- service.execute(project, user, '00000000000000000000000000000000', 'newrev', 'refs/heads/master')
+ service.execute(project, user, @blankrev, 'newrev', 'refs/heads/master')
end
it "when pushing tags" do
end
end
end
+
+ describe "cross-reference notes" do
+ let(:issue) { create :issue, project: project }
+ let(:commit_author) { create :user }
+ let(:commit) { project.repository.commit }
+
+ before do
+ commit.stub({
+ safe_message: "this commit \n mentions ##{issue.id}",
+ references: [issue],
+ author_name: commit_author.name,
+ author_email: commit_author.email
+ })
+ project.repository.stub(commits_between: [commit])
+ end
+
+ it "creates a note if a pushed commit mentions an issue" do
+ Note.should_receive(:create_cross_reference_note).with(issue, commit, commit_author, project)
+
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ end
+
+ it "only creates a cross-reference note if one doesn't already exist" do
+ Note.create_cross_reference_note(issue, commit, user, project)
+
+ Note.should_not_receive(:create_cross_reference_note).with(issue, commit, commit_author, project)
+
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ end
+
+ it "defaults to the pushing user if the commit's author is not known" do
+ commit.stub(author_name: 'unknown name', author_email: 'unknown@email.com')
+ Note.should_receive(:create_cross_reference_note).with(issue, commit, user, project)
+
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ end
+
+ it "finds references in the first push to a non-default branch" do
+ project.repository.stub(:commits_between).with(@blankrev, @newrev).and_return([])
+ project.repository.stub(:commits_between).with("master", @newrev).and_return([commit])
+
+ Note.should_receive(:create_cross_reference_note).with(issue, commit, commit_author, project)
+
+ service.execute(project, user, @blankrev, @newrev, 'refs/heads/other')
+ end
+
+ it "finds references in the first push to a default branch" do
+ project.repository.stub(:commits_between).with(@blankrev, @newrev).and_return([])
+ project.repository.stub(:commits).with(@newrev).and_return([commit])
+
+ Note.should_receive(:create_cross_reference_note).with(issue, commit, commit_author, project)
+
+ service.execute(project, user, @blankrev, @newrev, 'refs/heads/master')
+ end
+ end
+
+ describe "closing issues from pushed commits" do
+ let(:issue) { create :issue, project: project }
+ let(:other_issue) { create :issue, project: project }
+ let(:commit_author) { create :user }
+ let(:closing_commit) { project.repository.commit }
+
+ before do
+ closing_commit.stub({
+ issue_closing_regex: /^([Cc]loses|[Ff]ixes) #\d+/,
+ safe_message: "this is some work.\n\ncloses ##{issue.iid}",
+ author_name: commit_author.name,
+ author_email: commit_author.email
+ })
+
+ project.repository.stub(commits_between: [closing_commit])
+ end
+
+ it "closes issues with commit messages" do
+ service.execute(project, user, @oldrev, @newrev, @ref)
+
+ Issue.find(issue.id).should be_closed
+ end
+
+ it "passes the closing commit as a thread-local" do
+ service.execute(project, user, @oldrev, @newrev, @ref)
+
+ Thread.current[:current_commit].should == closing_commit
+ end
+
+ it "doesn't create cross-reference notes for a closing reference" do
+ expect {
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ }.not_to change { Note.where(project_id: project.id, system: true).count }
+ end
+
+ it "doesn't close issues when pushed to non-default branches" do
+ project.stub(default_branch: 'durf')
+
+ # The push still shouldn't create cross-reference notes.
+ expect {
+ service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf')
+ }.not_to change { Note.where(project_id: project.id, system: true).count }
+
+ Issue.find(issue.id).should be_opened
+ end
+ end
end
fill_in "user_login", with: user.email
fill_in "user_password", with: "123456"
click_button "Sign in"
+ Thread.current[:current_user] = user
end
def logout
--- /dev/null
+# Specifications for behavior common to all Mentionable implementations.
+# Requires a shared context containing:
+# - let(:subject) { "the mentionable implementation" }
+# - let(:backref_text) { "the way that +subject+ should refer to itself in backreferences " }
+# - let(:set_mentionable_text) { lambda { |txt| "block that assigns txt to the subject's mentionable_text" } }
+
+def common_mentionable_setup
+ # Avoid name collisions with let(:project) or let(:author) in the surrounding scope.
+ let(:mproject) { create :project }
+ let(:mauthor) { subject.author }
+
+ let(:mentioned_issue) { create :issue, project: mproject }
+ let(:other_issue) { create :issue, project: mproject }
+ let(:mentioned_mr) { create :merge_request, target_project: mproject, source_branch: 'different' }
+ let(:mentioned_commit) { mock('commit', sha: '1234567890abcdef').as_null_object }
+
+ # Override to add known commits to the repository stub.
+ let(:extra_commits) { [] }
+
+ # A string that mentions each of the +mentioned_.*+ objects above. Mentionables should add a self-reference
+ # to this string and place it in their +mentionable_text+.
+ let(:ref_string) do
+ "mentions ##{mentioned_issue.iid} twice ##{mentioned_issue.iid}, !#{mentioned_mr.iid}, " +
+ "#{mentioned_commit.sha[0..5]} and itself as #{backref_text}"
+ end
+
+ before do
+ # Wire the project's repository to return the mentioned commit, and +nil+ for any
+ # unrecognized commits.
+ commitmap = { '123456' => mentioned_commit }
+ extra_commits.each { |c| commitmap[c.sha[0..5]] = c }
+
+ repo = mock('repository')
+ repo.stub(:commit) { |sha| commitmap[sha] }
+ mproject.stub(repository: repo)
+
+ set_mentionable_text.call(ref_string)
+ end
+end
+
+shared_examples 'a mentionable' do
+ common_mentionable_setup
+
+ it 'generates a descriptive back-reference' do
+ subject.gfm_reference.should == backref_text
+ end
+
+ it "extracts references from its reference property" do
+ # De-duplicate and omit itself
+ refs = subject.references(mproject)
+
+ refs.should have(3).items
+ refs.should include(mentioned_issue)
+ refs.should include(mentioned_mr)
+ refs.should include(mentioned_commit)
+ end
+
+ it 'creates cross-reference notes' do
+ [mentioned_issue, mentioned_mr, mentioned_commit].each do |referenced|
+ Note.should_receive(:create_cross_reference_note).with(referenced, subject.local_reference, mauthor, mproject)
+ end
+
+ subject.create_cross_references!(mproject, mauthor)
+ end
+
+ it 'detects existing cross-references' do
+ Note.create_cross_reference_note(mentioned_issue, subject.local_reference, mauthor, mproject)
+
+ subject.has_mentioned?(mentioned_issue).should be_true
+ subject.has_mentioned?(mentioned_mr).should be_false
+ end
+end
+
+shared_examples 'an editable mentionable' do
+ common_mentionable_setup
+
+ it_behaves_like 'a mentionable'
+
+ it 'creates new cross-reference notes when the mentionable text is edited' do
+ new_text = "this text still mentions ##{mentioned_issue.iid} and #{mentioned_commit.sha[0..5]}, " +
+ "but now it mentions ##{other_issue.iid}, too."
+
+ [mentioned_issue, mentioned_commit].each do |oldref|
+ Note.should_not_receive(:create_cross_reference_note).with(oldref, subject.local_reference,
+ mauthor, mproject)
+ end
+
+ Note.should_receive(:create_cross_reference_note).with(other_issue, subject.local_reference, mauthor, mproject)
+
+ subject.save
+ set_mentionable_text.call(new_text)
+ subject.notice_added_references(mproject, mauthor)
+ end
+end
OSDN Git Service
