diff options
author | Douwe Maan <douwe@selenight.nl> | 2016-08-12 16:24:09 -0500 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2016-08-12 16:24:09 -0500 |
commit | e25720045988e59f7ddfecebbc02e640dc2f8360 (patch) | |
tree | 1555fa2bd5b07d3495032c1fdd0ecce6cb222254 | |
parent | f48642e260e7cc733b619e34e47d6e67b484fee0 (diff) | |
download | gitlab-ce-e25720045988e59f7ddfecebbc02e640dc2f8360.tar.gz |
Add specs for NotesController and DiscussionsController
-rw-r--r-- | app/controllers/projects/discussions_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/notes_controller.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request.rb | 4 | ||||
-rw-r--r-- | spec/controllers/projects/discussions_controller_spec.rb | 112 | ||||
-rw-r--r-- | spec/controllers/projects/notes_controller_spec.rb | 106 |
5 files changed, 222 insertions, 4 deletions
diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb index 86e3f865c6f..502aa20d7b6 100644 --- a/app/controllers/projects/discussions_controller.rb +++ b/app/controllers/projects/discussions_controller.rb @@ -34,7 +34,7 @@ class Projects::DiscussionsController < Projects::ApplicationController end def discussion - @discussion ||= @merge_request.discussions.find { |d| d.id == params[:id] } || render_404 + @discussion ||= @merge_request.find_discussion(params[:id]) || render_404 end def authorize_resolve_discussion! diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 1849311d26d..934a7961a63 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -5,7 +5,7 @@ class Projects::NotesController < Projects::ApplicationController before_action :authorize_read_note! before_action :authorize_create_note!, only: [:create] before_action :authorize_admin_note!, only: [:update, :destroy] - before_action :authorize_resolve_note!, only: [:resolve] + before_action :authorize_resolve_note!, only: [:resolve, :unresolve] before_action :find_current_user_notes, only: [:index] def index diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1ab644a989a..cb0bb3b64ac 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -424,6 +424,10 @@ class MergeRequest < ActiveRecord::Base discussions end + def find_discussion(discussion_id) + discussions.find { |d| d.id == discussion_id } + end + def discussions_resolvable? discussions.any?(&:resolvable?) end diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb new file mode 100644 index 00000000000..14c8a76fb8e --- /dev/null +++ b/spec/controllers/projects/discussions_controller_spec.rb @@ -0,0 +1,112 @@ +require 'spec_helper' + +describe Projects::DiscussionsController do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + + describe 'POST resolve' do + before do + sign_in user + end + + context "when the user is not authorized to resolve the discussion" do + it "returns status 404" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + + expect(response).to have_http_status(404) + end + end + + context "when the user is authorized to resolve the discussion" do + before do + project.team << [user, :developer] + end + + context "when the discussion is not resolvable" do + before do + note.update(system: true) + end + + it "returns status 404" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + + expect(response).to have_http_status(404) + end + end + + context "when the discussion is resolvable" do + it "resolves the discussion" do + expect_any_instance_of(Discussion).to receive(:resolve!).with(user) + + post :resolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + end + + it "checks whether all notes are resolved" do + expect_any_instance_of(MergeRequests::AllDiscussionsResolvedService).to receive(:execute).with(merge_request) + + post :resolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + end + + it "returns the name of the resolving user" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + + expect(JSON.parse(response.body)["resolved_by"]).to eq(user.name) + end + + it "returns status 200" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + + expect(response).to have_http_status(200) + end + end + end + end + + describe 'DELETE unresolve' do + before do + sign_in user + end + + context "when the user is not authorized to resolve the discussion" do + it "returns status 404" do + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + + expect(response).to have_http_status(404) + end + end + + context "when the user is authorized to resolve the discussion" do + before do + project.team << [user, :developer] + end + + context "when the discussion is not resolvable" do + before do + note.update(system: true) + end + + it "returns status 404" do + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + + expect(response).to have_http_status(404) + end + end + + context "when the discussion is resolvable" do + it "unresolves the discussion" do + expect_any_instance_of(Discussion).to receive(:unresolve!) + + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + end + + it "returns status 200" do + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, merge_request_id: merge_request.iid, id: note.discussion_id + + expect(response).to have_http_status(200) + end + end + end + end +end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 75590c1ed4f..d57d5c0b963 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -1,4 +1,4 @@ -require('spec_helper') +require 'spec_helper' describe Projects::NotesController do let(:user) { create(:user) } @@ -6,7 +6,7 @@ describe Projects::NotesController do let(:issue) { create(:issue, project: project) } let(:note) { create(:note, noteable: issue, project: project) } - describe 'POST #toggle_award_emoji' do + describe 'POST toggle_award_emoji' do before do sign_in(user) project.team << [user, :developer] @@ -33,4 +33,106 @@ describe Projects::NotesController do expect(response).to have_http_status(200) end end + + describe 'POST resolve' do + before do + sign_in user + end + + context "when the user is not authorized to resolve the note" do + it "returns status 404" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + + expect(response).to have_http_status(404) + end + end + + context "when the user is authorized to resolve the note" do + before do + project.team << [user, :developer] + end + + context "when the note is not resolvable" do + it "returns status 404" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + + expect(response).to have_http_status(404) + end + end + + context "when the note is resolvable" do + let(:merge_request) { create(:merge_request, source_project: project) } + let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + + it "resolves the note" do + expect_any_instance_of(DiffNote).to receive(:resolve!).with(user) + + post :resolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + end + + it "checks whether all notes are resolved" do + expect_any_instance_of(MergeRequests::AllDiscussionsResolvedService).to receive(:execute).with(merge_request) + + post :resolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + end + + it "returns the name of the resolving user" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + + expect(JSON.parse(response.body)["resolved_by"]).to eq(user.name) + end + + it "returns status 200" do + post :resolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + + expect(response).to have_http_status(200) + end + end + end + end + + describe 'DELETE unresolve' do + before do + sign_in user + end + + context "when the user is not authorized to resolve the note" do + it "returns status 404" do + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + + expect(response).to have_http_status(404) + end + end + + context "when the user is authorized to resolve the note" do + before do + project.team << [user, :developer] + end + + context "when the note is not resolvable" do + it "returns status 404" do + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + + expect(response).to have_http_status(404) + end + end + + context "when the note is resolvable" do + let(:merge_request) { create(:merge_request, source_project: project) } + let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } + + it "unresolves the note" do + expect_any_instance_of(DiffNote).to receive(:unresolve!) + + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + end + + it "returns status 200" do + delete :unresolve, namespace_id: project.namespace.path, project_id: project.path, id: note.id + + expect(response).to have_http_status(200) + end + end + end + end end |