diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-05-01 15:39:45 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-05-01 15:39:45 +0300 |
commit | bdc20840aaf5165e654498d011e6273d5e0fd240 (patch) | |
tree | 0f9240de123b0570be2a5aec9566d0dcf7a65627 /spec | |
parent | 3fcb9c115d776feba3f71fb58359a3935edfda9b (diff) | |
parent | 7a76caa5a8d2c6be9aa4b698a8919e223df973d3 (diff) |
Merge branch 'jprovazn-commit-notes-api' into 'master'
Merge request and commit discussions API
Closes #20901
See merge request gitlab-org/gitlab-ce!18537
Diffstat (limited to 'spec')
5 files changed, 198 insertions, 7 deletions
diff --git a/spec/fixtures/api/schemas/public_api/v4/notes.json b/spec/fixtures/api/schemas/public_api/v4/notes.json index 4c4ca3b582f..0f9c221fd6d 100644 --- a/spec/fixtures/api/schemas/public_api/v4/notes.json +++ b/spec/fixtures/api/schemas/public_api/v4/notes.json @@ -24,7 +24,10 @@ "system": { "type": "boolean" }, "noteable_id": { "type": "integer" }, "noteable_iid": { "type": "integer" }, - "noteable_type": { "type": "string" } + "noteable_type": { "type": "string" }, + "resolved": { "type": "boolean" }, + "resolvable": { "type": "boolean" }, + "resolved_by": { "type": ["string", "null"] } }, "required": [ "id", "body", "attachment", "author", "created_at", "updated_at", diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index 4a44b219a67..ef34192f888 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -2,32 +2,53 @@ require 'spec_helper' describe API::Discussions do let(:user) { create(:user) } - let!(:project) { create(:project, :public, namespace: user.namespace) } + let!(:project) { create(:project, :public, :repository, namespace: user.namespace) } let(:private_user) { create(:user) } before do - project.add_reporter(user) + project.add_developer(user) end - context "when noteable is an Issue" do + context 'when noteable is an Issue' do let!(:issue) { create(:issue, project: project, author: user) } let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) } - it_behaves_like "discussions API", 'projects', 'issues', 'iid' do + it_behaves_like 'discussions API', 'projects', 'issues', 'iid' do let(:parent) { project } let(:noteable) { issue } let(:note) { issue_note } end end - context "when noteable is a Snippet" do + context 'when noteable is a Snippet' do let!(:snippet) { create(:project_snippet, project: project, author: user) } let!(:snippet_note) { create(:discussion_note_on_snippet, noteable: snippet, project: project, author: user) } - it_behaves_like "discussions API", 'projects', 'snippets', 'id' do + it_behaves_like 'discussions API', 'projects', 'snippets', 'id' do let(:parent) { project } let(:noteable) { snippet } let(:note) { snippet_note } end end + + context 'when noteable is a Merge Request' do + let!(:noteable) { create(:merge_request_with_diffs, source_project: project, target_project: project, author: user) } + let!(:note) { create(:discussion_note_on_merge_request, noteable: noteable, project: project, author: user) } + let!(:diff_note) { create(:diff_note_on_merge_request, noteable: noteable, project: project, author: user) } + let(:parent) { project } + + it_behaves_like 'discussions API', 'projects', 'merge_requests', 'iid' + it_behaves_like 'diff discussions API', 'projects', 'merge_requests', 'iid' + it_behaves_like 'resolvable discussions API', 'projects', 'merge_requests', 'iid' + end + + context 'when noteable is a Commit' do + let!(:noteable) { create(:commit, project: project, author: user) } + let!(:note) { create(:discussion_note_on_commit, commit_id: noteable.id, project: project, author: user) } + let!(:diff_note) { create(:diff_note_on_commit, commit_id: noteable.id, project: project, author: user) } + let(:parent) { project } + + it_behaves_like 'discussions API', 'projects', 'repository/commits', 'id' + it_behaves_like 'diff discussions API', 'projects', 'repository/commits', 'id' + end end diff --git a/spec/services/notes/resolve_service_spec.rb b/spec/services/notes/resolve_service_spec.rb new file mode 100644 index 00000000000..b54d40a7a5c --- /dev/null +++ b/spec/services/notes/resolve_service_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Notes::ResolveService do + let(:merge_request) { create(:merge_request) } + let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) } + let(:user) { merge_request.author } + + describe '#execute' do + it "resolves the note" do + described_class.new(merge_request.project, user).execute(note) + note.reload + + expect(note.resolved?).to be true + expect(note.resolved_by).to eq(user) + end + + it "sends notifications if all discussions are resolved" do + expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request) + + described_class.new(merge_request.project, user).execute(note) + end + end +end diff --git a/spec/support/shared_examples/requests/api/diff_discussions.rb b/spec/support/shared_examples/requests/api/diff_discussions.rb new file mode 100644 index 00000000000..85a4bd8ca27 --- /dev/null +++ b/spec/support/shared_examples/requests/api/diff_discussions.rb @@ -0,0 +1,57 @@ +shared_examples 'diff discussions API' do |parent_type, noteable_type, id_name| + describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do + it "includes diff discussions" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user) + + discussion = json_response.find { |record| record['id'] == diff_note.discussion_id } + + expect(response).to have_gitlab_http_status(200) + expect(discussion).not_to be_nil + expect(discussion['individual_note']).to eq(false) + expect(discussion['notes'].first['body']).to eq(diff_note.note) + end + end + + describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do + it "returns a discussion by id" do + get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{diff_note.discussion_id}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(diff_note.discussion_id) + expect(json_response['notes'].first['body']).to eq(diff_note.note) + expect(json_response['notes'].first['position']).to eq(diff_note.position.to_h.stringify_keys) + end + end + + describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do + it "creates a new diff note" do + position = diff_note.position.to_h + + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position + + expect(response).to have_gitlab_http_status(201) + expect(json_response['notes'].first['body']).to eq('hi!') + expect(json_response['notes'].first['type']).to eq('DiffNote') + expect(json_response['notes'].first['position']).to eq(position.stringify_keys) + end + + it "returns a 400 bad request error when position is invalid" do + position = diff_note.position.to_h.merge(new_line: '100000') + + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position + + expect(response).to have_gitlab_http_status(400) + end + end + + describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do + it 'adds a new note to the diff discussion' do + post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{diff_note.discussion_id}/notes", user), body: 'hi!' + + expect(response).to have_gitlab_http_status(201) + expect(json_response['body']).to eq('hi!') + expect(json_response['type']).to eq('DiffNote') + end + end +end diff --git a/spec/support/shared_examples/requests/api/resolvable_discussions.rb b/spec/support/shared_examples/requests/api/resolvable_discussions.rb new file mode 100644 index 00000000000..408ad08cc48 --- /dev/null +++ b/spec/support/shared_examples/requests/api/resolvable_discussions.rb @@ -0,0 +1,87 @@ +shared_examples 'resolvable discussions API' do |parent_type, noteable_type, id_name| + describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do + it "resolves discussion if resolved is true" do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}", user), resolved: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response['notes'].size).to eq(1) + expect(json_response['notes'][0]['resolved']).to eq(true) + end + + it "unresolves discussion if resolved is false" do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}", user), resolved: false + + expect(response).to have_gitlab_http_status(200) + expect(json_response['notes'].size).to eq(1) + expect(json_response['notes'][0]['resolved']).to eq(false) + end + + it "returns a 400 bad request error if resolved parameter is not passed" do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}", user) + + expect(response).to have_gitlab_http_status(400) + end + + it "returns a 401 unauthorized error if user is not authenticated" do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}"), resolved: true + + expect(response).to have_gitlab_http_status(401) + end + + it "returns a 403 error if user resolves discussion of someone else" do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}", private_user), resolved: true + + expect(response).to have_gitlab_http_status(403) + end + + context 'when user does not have access to read the discussion' do + before do + parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'responds with 404' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}", private_user), resolved: true + + expect(response).to have_gitlab_http_status(404) + end + end + end + + describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do + it 'returns resolved note when resolved parameter is true' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", user), resolved: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response['resolved']).to eq(true) + end + + it 'returns a 404 error when note id not found' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/12345", user), + body: 'Hello!' + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns a 400 bad request error if neither body nor resolved parameter is given' do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", user) + + expect(response).to have_gitlab_http_status(400) + end + + it "returns a 403 error if user resolves note of someone else" do + put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\ + "discussions/#{note.discussion_id}/notes/#{note.id}", private_user), resolved: true + + expect(response).to have_gitlab_http_status(403) + end + end +end |