diff options
author | ash wilson <smashwilson@gmail.com> | 2013-05-30 23:16:49 +0000 |
---|---|---|
committer | Ash Wilson <smashwilson@gmail.com> | 2013-08-25 18:58:41 -0400 |
commit | c8a115c0e3a9c8242c2a422572d47a49e0cb2874 (patch) | |
tree | 5a36c3e0f364fdfb710e01090fc81b9676ea53c4 /spec | |
parent | 2b36dee64485062c69779217d4a202e5ca1b67bd (diff) | |
download | gitlab-ce-c8a115c0e3a9c8242c2a422572d47a49e0cb2874.tar.gz |
Link issues from comments and automatically close them
Any mention of Issues, MergeRequests, or Commits via GitLab-flavored markdown
references in descriptions, titles, or attached Notes creates a back-reference
Note that links to the original referencer. Furthermore, pushing commits with
commit messages that match a (configurable) regexp to a project's default
branch will close any issues mentioned by GFM in the matched closing phrase.
If accepting a merge request would close any Issues in this way, a banner is
appended to the merge request's main panel to indicate this.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/lib/gitlab/reference_extractor_spec.rb | 95 | ||||
-rw-r--r-- | spec/models/commit_spec.rb | 22 | ||||
-rw-r--r-- | spec/models/issue_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 31 | ||||
-rw-r--r-- | spec/models/note_spec.rb | 108 | ||||
-rw-r--r-- | spec/observers/activity_observer_spec.rb | 24 | ||||
-rw-r--r-- | spec/observers/issue_observer_spec.rb | 34 | ||||
-rw-r--r-- | spec/observers/merge_request_observer_spec.rb | 36 | ||||
-rw-r--r-- | spec/observers/note_observer_spec.rb | 32 | ||||
-rw-r--r-- | spec/services/git_push_service_spec.rb | 105 | ||||
-rw-r--r-- | spec/support/login_helpers.rb | 1 | ||||
-rw-r--r-- | spec/support/mentionable_shared_examples.rb | 94 |
12 files changed, 570 insertions, 18 deletions
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 |