diff options
32 files changed, 925 insertions, 50 deletions
diff --git a/CHANGELOG b/CHANGELOG index 0bd68762563..7c3d7ba3502 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,7 @@ +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. @@ -54,7 +58,7 @@ v 5.4.0 - 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 diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 235247f3e52..55d2c3f04fc 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -3,6 +3,7 @@ require 'gitlab/satellite/satellite' 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] @@ -135,6 +136,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController @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 diff --git a/app/models/commit.rb b/app/models/commit.rb index da80c2940ff..f8ca6a5fe82 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -2,6 +2,9 @@ class Commit 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 @@ -65,6 +68,29 @@ class Commit 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 diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index f22070f8504..27e39339ae8 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -1,12 +1,47 @@ # == 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? @@ -24,14 +59,39 @@ module Mentionable 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 diff --git a/app/models/issue.rb b/app/models/issue.rb index 03c1c166137..ffe9681fc83 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -32,6 +32,7 @@ class Issue < ActiveRecord::Base 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 @@ -56,4 +57,10 @@ class Issue < ActiveRecord::Base # 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 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0bb4b231f62..514fc79f7c5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -31,7 +31,7 @@ class MergeRequest < ActiveRecord::Base 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 @@ -255,6 +255,20 @@ class MergeRequest < ActiveRecord::Base 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) diff --git a/app/models/note.rb b/app/models/note.rb index 7598978ad4d..e819a5516b5 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -24,6 +24,7 @@ class Note < ActiveRecord::Base 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 @@ -54,15 +55,36 @@ class Note < ActiveRecord::Base 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) || @@ -191,6 +213,16 @@ class Note < ActiveRecord::Base 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 diff --git a/app/models/repository.rb b/app/models/repository.rb index 3d649519d8f..aeec48ee5cc 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -18,6 +18,7 @@ class Repository 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 diff --git a/app/observers/activity_observer.rb b/app/observers/activity_observer.rb index 477ebd78aeb..d754ac8185a 100644 --- a/app/observers/activity_observer.rb +++ b/app/observers/activity_observer.rb @@ -5,8 +5,8 @@ class ActivityObserver < BaseObserver 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? diff --git a/app/observers/base_observer.rb b/app/observers/base_observer.rb index 92b73981d27..f9a0242ce77 100644 --- a/app/observers/base_observer.rb +++ b/app/observers/base_observer.rb @@ -10,4 +10,8 @@ class BaseObserver < ActiveRecord::Observer def current_user Thread.current[:current_user] end + + def current_commit + Thread.current[:current_commit] + end end diff --git a/app/observers/issue_observer.rb b/app/observers/issue_observer.rb index 50538419776..886d8b776fb 100644 --- a/app/observers/issue_observer.rb +++ b/app/observers/issue_observer.rb @@ -1,6 +1,8 @@ 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) @@ -17,12 +19,14 @@ class IssueObserver < BaseObserver 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 diff --git a/app/observers/merge_request_observer.rb b/app/observers/merge_request_observer.rb index 689a25ff4b7..d70da514cd2 100644 --- a/app/observers/merge_request_observer.rb +++ b/app/observers/merge_request_observer.rb @@ -7,11 +7,13 @@ class MergeRequestObserver < ActivityObserver 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 @@ -33,11 +35,13 @@ class MergeRequestObserver < ActivityObserver 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) diff --git a/app/observers/note_observer.rb b/app/observers/note_observer.rb index 7b79161cce4..d31b6e8d7ce 100644 --- a/app/observers/note_observer.rb +++ b/app/observers/note_observer.rb @@ -1,5 +1,17 @@ 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 diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index e8b32f52ce1..e774b2276e8 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -1,22 +1,24 @@ 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 @@ -25,11 +27,27 @@ class GitPushService 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 @@ -45,7 +63,7 @@ class GitPushService protected def create_push_event - Event.create( + Event.create!( project: project, action: Event::PUSHED, data: push_data, @@ -53,6 +71,36 @@ class GitPushService ) 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 = { @@ -72,8 +120,6 @@ class GitPushService # } # def post_receive_data(oldrev, newrev, ref) - push_commits = project.repository.commits_between(oldrev, newrev) - # Total commits count push_commits_count = push_commits.size @@ -116,10 +162,26 @@ class GitPushService 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 diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml index 594f4061c23..c4b614b51da 100644 --- a/app/views/projects/merge_requests/show/_mr_box.html.haml +++ b/app/views/projects/merge_requests/show/_mr_box.html.haml @@ -33,4 +33,10 @@ %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) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index a3661821271..389dba59c9d 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -46,6 +46,11 @@ production: &base ## 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 diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 92d4a29b7c1..b3a19783b14 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -68,6 +68,7 @@ rescue ArgumentError # no user configured 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? diff --git a/db/migrate/20130528184641_add_system_to_notes.rb b/db/migrate/20130528184641_add_system_to_notes.rb new file mode 100644 index 00000000000..1b22a4934f9 --- /dev/null +++ b/db/migrate/20130528184641_add_system_to_notes.rb @@ -0,0 +1,16 @@ +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 diff --git a/db/schema.rb b/db/schema.rb index 429551a2bf4..5020230d831 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -152,6 +152,7 @@ ActiveRecord::Schema.define(:version => 20130821090531) do 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" diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb new file mode 100644 index 00000000000..94b01e808d9 --- /dev/null +++ b/lib/gitlab/reference_extractor.rb @@ -0,0 +1,59 @@ +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 diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb new file mode 100644 index 00000000000..7d805f8c72a --- /dev/null +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -0,0 +1,95 @@ +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 diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 73418efd3f2..fa556f94a1d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -1,7 +1,8 @@ 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 @@ -46,4 +47,23 @@ describe Commit 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 diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index c10891331b1..75155d5dc1d 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -56,4 +56,10 @@ describe Issue do 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 diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 40931513af6..18c32d4fb74 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -43,7 +43,6 @@ describe MergeRequest do it { should include_module(Issuable) } end - describe "#mr_and_commit_notes" do let!(:merge_request) { create(:merge_request) } @@ -104,4 +103,34 @@ describe MergeRequest do 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 diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 646255543a0..ef143debcc1 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -72,7 +72,6 @@ describe Note do end let(:project) { create(:project) } - let(:commit) { project.repository.commit } describe "Commit notes" do let!(:note) { create(:note_on_commit, note: "+1 from me") } @@ -131,7 +130,7 @@ describe Note do 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 @@ -150,7 +149,7 @@ describe Note do 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 @@ -161,6 +160,102 @@ describe Note do 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 @@ -208,4 +303,11 @@ describe Note 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 diff --git a/spec/observers/activity_observer_spec.rb b/spec/observers/activity_observer_spec.rb index 78837552438..dc14ab86b6d 100644 --- a/spec/observers/activity_observer_spec.rb +++ b/spec/observers/activity_observer_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' 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 } @@ -34,4 +36,26 @@ describe ActivityObserver do 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 diff --git a/spec/observers/issue_observer_spec.rb b/spec/observers/issue_observer_spec.rb index c9e88aee1a5..4155ff31193 100644 --- a/spec/observers/issue_observer_spec.rb +++ b/spec/observers/issue_observer_spec.rb @@ -8,8 +8,9 @@ describe IssueObserver do 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 } @@ -19,6 +20,15 @@ describe IssueObserver do 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 @@ -26,7 +36,7 @@ describe IssueObserver 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 @@ -35,13 +45,23 @@ describe IssueObserver do 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 @@ -67,5 +87,13 @@ describe IssueObserver do 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 diff --git a/spec/observers/merge_request_observer_spec.rb b/spec/observers/merge_request_observer_spec.rb index 3d6fff9c24e..3f5250a0040 100644 --- a/spec/observers/merge_request_observer_spec.rb +++ b/spec/observers/merge_request_observer_spec.rb @@ -5,15 +5,20 @@ describe MergeRequestObserver do 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 } @@ -24,11 +29,20 @@ describe MergeRequestObserver do 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 @@ -41,6 +55,12 @@ describe MergeRequestObserver 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) @@ -61,13 +81,13 @@ describe MergeRequestObserver do 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 @@ -77,13 +97,13 @@ describe MergeRequestObserver do 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 diff --git a/spec/observers/note_observer_spec.rb b/spec/observers/note_observer_spec.rb index 9ada92704b6..f9b96c255c1 100644 --- a/spec/observers/note_observer_spec.rb +++ b/spec/observers/note_observer_spec.rb @@ -5,9 +5,9 @@ describe NoteObserver do 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 @@ -22,5 +22,35 @@ describe NoteObserver do 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 diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 286a8cdaa22..ffd80f33fc8 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -6,6 +6,7 @@ describe GitPushService do let (:service) { GitPushService.new } before do + @blankrev = '0000000000000000000000000000000000000000' @oldrev = 'b98a310def241a6fd9c9a9a3e7934c48e498fe81' @newrev = 'b19a04f53caeebf4fe5ec2327cb83e9253dc91bb' @ref = 'refs/heads/master' @@ -98,7 +99,7 @@ describe GitPushService do 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 @@ -107,5 +108,107 @@ describe GitPushService 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 diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index 46794f48384..025534a900d 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -18,6 +18,7 @@ module LoginHelpers 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 diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb new file mode 100644 index 00000000000..a7f189777c8 --- /dev/null +++ b/spec/support/mentionable_shared_examples.rb @@ -0,0 +1,94 @@ +# 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 |