From 75d04f6a29a5506ffd53c227517411febdc54910 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 27 Jul 2017 15:36:39 +0100 Subject: Fix replying to commit comments on MRs from forks A commit comment shows in the MR, but if the MR is from a fork, it will have a different project ID to the MR's target project. In that case, add an note_project_id param so that we can pick the correct project for the note. --- spec/controllers/projects/notes_controller_spec.rb | 62 +++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) (limited to 'spec/controllers/projects/notes_controller_spec.rb') diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 45f4cf9180d..3b88d5b0d7d 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -131,7 +131,7 @@ describe Projects::NotesController do before do sign_in(user) - project.team << [user, :developer] + project.add_developer(user) end it "returns status 302 for html" do @@ -165,6 +165,66 @@ describe Projects::NotesController do expect(response).to have_http_status(302) end end + + context 'when creating a commit comment from an MR fork' do + let(:project) { create(:project) } + + let(:fork_project) do + create(:project).tap do |fork| + create(:forked_project_link, forked_to_project: fork, forked_from_project: project) + end + end + + let(:merge_request) do + create(:merge_request, source_project: fork_project, target_project: project, source_branch: 'feature', target_branch: 'master') + end + + let(:existing_comment) do + create(:note_on_commit, note: 'a note', project: fork_project, commit_id: merge_request.commit_shas.first) + end + + def post_create(extra_params = {}) + post :create, { + note: { note: 'some other note' }, + namespace_id: project.namespace, + project_id: project, + target_type: 'merge_request', + target_id: merge_request.id, + note_project_id: fork_project.id, + in_reply_to_discussion_id: existing_comment.discussion_id + }.merge(extra_params) + end + + context 'when the note_project_id is not correct' do + it 'returns a 404' do + post_create(note_project_id: Project.maximum(:id).succ) + + expect(response).to have_http_status(404) + end + end + + context 'when the user has no access to the fork' do + it 'returns a 404' do + post_create + + expect(response).to have_http_status(404) + end + end + + context 'when the user has access to the fork' do + let(:discussion) { fork_project.notes.find_discussion(existing_comment.discussion_id) } + + before do + fork_project.add_developer(user) + + existing_comment + end + + it 'creates the note' do + expect { post_create }.to change { fork_project.notes.count }.by(1) + end + end + end end describe 'DELETE destroy' do -- cgit v1.2.1 From 9513bd18c43b749c8856d6ad89635d97c8fb73b5 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 1 Aug 2017 14:51:52 -0400 Subject: Ensure all project factories use `:repository` trait or `:empty_project` --- spec/controllers/projects/notes_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/controllers/projects/notes_controller_spec.rb') diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 3b88d5b0d7d..4a6eb76e71f 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -167,10 +167,10 @@ describe Projects::NotesController do end context 'when creating a commit comment from an MR fork' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:fork_project) do - create(:project).tap do |fork| + create(:project, :repository).tap do |fork| create(:forked_project_link, forked_to_project: fork, forked_from_project: project) end end -- cgit v1.2.1 From 72a7b30c9f363063449b28aa9efc3a26a1752f9f Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 2 Aug 2017 15:55:11 -0400 Subject: Change all `:empty_project` to `:project` --- spec/controllers/projects/notes_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/controllers/projects/notes_controller_spec.rb') diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 4a6eb76e71f..f280c55059c 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::NotesController do let(:user) { create(:user) } - let(:project) { create(:empty_project) } + let(:project) { create(:project) } let(:issue) { create(:issue, project: project) } let(:note) { create(:note, noteable: issue, project: project) } -- cgit v1.2.1