From 038b6f49a9288de3e4f1c9e8881cc4b846b81a00 Mon Sep 17 00:00:00 2001 From: Hank Grabowski Date: Thu, 18 Oct 2018 10:32:54 -0400 Subject: [PATCH] Comments API Endpoint complete with full unit tests --- app/controllers/api/v1/comments_controller.rb | 55 +++++- app/presenters/comment_presenter.rb | 2 +- app/services/comment_service.rb | 2 + config/locales/diaspora/en.yml | 7 +- .../api/comments_controller_spec.rb | 164 +++++++++++++++++- spec/services/comment_service_spec.rb | 2 +- 6 files changed, 213 insertions(+), 19 deletions(-) diff --git a/app/controllers/api/v1/comments_controller.rb b/app/controllers/api/v1/comments_controller.rb index 54b5e4edb..33f32ab01 100644 --- a/app/controllers/api/v1/comments_controller.rb +++ b/app/controllers/api/v1/comments_controller.rb @@ -8,12 +8,23 @@ module Api end before_action only: %i[create destroy] do - require_access_token %w[read write] + require_access_token %w[write] + end + + rescue_from ActiveRecord::RecordNotFound do + render json: I18n.t("api.endpoint_errors.posts.post_not_found"), status: :not_found + end + + rescue_from ActiveRecord::RecordInvalid do + render json: I18n.t("api.endpoint_errors.comments.not_allowed"), status: :unprocessable_entity end def create @comment = comment_service.create(params[:post_id], params[:body]) comment = comment_as_json(@comment) + rescue ActiveRecord::RecordNotFound + render json: I18n.t("api.endpoint_errors.posts.post_not_found"), status: :not_found + else render json: comment, status: :created end @@ -23,12 +34,18 @@ module Api end def destroy - comment_service.destroy!(params[:id]) - head :no_content + if comment_and_post_validate(params[:post_id], params[:id]) + comment_service.destroy!(params[:id]) + head :no_content + end + rescue ActiveRecord::RecordInvalid + render json: I18n.t("api.endpoint_errors.comments.no_delete"), status: :forbidden end def report + post_guid = params.require(:post_id) comment_guid = params.require(:comment_id) + return unless comment_and_post_validate(post_guid, comment_guid) reason = params.require(:reason) comment = comment_service.find!(comment_guid) report = current_user.reports.new( @@ -39,13 +56,37 @@ module Api if report.save head :no_content else - render( - json: {error: I18n.t("report.status.failed")}, - status: :internal_server_error - ) + render json: I18n.t("api.endpoint_errors.comments.duplicate_report"), status: :conflict end end + private + + def comment_and_post_validate(post_guid, comment_guid) + if !comment_exists(comment_guid) + render json: I18n.t("api.endpoint_errors.comments.not_found"), status: :not_found + false + elsif !comment_is_for_post(post_guid, comment_guid) + render json: I18n.t("api.endpoint_errors.comments.not_found"), status: :not_found + false + else + true + end + end + + def comment_is_for_post(post_guid, comment_guid) + comments = comment_service.find_for_post(post_guid) + comment = comments.find {|comment| comment[:guid] == comment_guid } + comment ? true : false + end + + def comment_exists(comment_guid) + comment = comment_service.find!(comment_guid) + comment ? true : false + rescue ActiveRecord::RecordNotFound + false + end + def comment_service @comment_service ||= CommentService.new(current_user) end diff --git a/app/presenters/comment_presenter.rb b/app/presenters/comment_presenter.rb index 6eae62ece..161c77718 100644 --- a/app/presenters/comment_presenter.rb +++ b/app/presenters/comment_presenter.rb @@ -20,7 +20,7 @@ class CommentPresenter < BasePresenter { guid: @comment.guid, body: @comment.message.plain_text_for_json, - author: @comment.author.as_api_response(:backbone), + author: PersonPresenter.new(@comment.author).as_api_json, created_at: @comment.created_at } end diff --git a/app/services/comment_service.rb b/app/services/comment_service.rb index 5f5588e1b..2e23033f8 100644 --- a/app/services/comment_service.rb +++ b/app/services/comment_service.rb @@ -34,6 +34,8 @@ class CommentService user.retract(comment) elsif user.owns?(comment.parent) user.retract(comment) + elsif comment + raise ActiveRecord::RecordInvalid else raise ActiveRecord::RecordNotFound end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index b257a9a1c..572569691 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -951,6 +951,11 @@ en: login_required: "You must first login before you can authorize this application" could_not_authorize: "The application could not be authorized" endpoint_errors: + comments: + not_found: "Comment not found for the given post" + not_allowed: "User is not allowed to comment" + no_delete: "User not allowed to delete the comment" + duplicate_report: "This item already has been reported by this user" likes: user_not_allowed_to_like: "User is not allowed to like" like_exists: "Like already exists" @@ -1367,5 +1372,3 @@ en: disabled: "Not available" open: "Open" closed: "Closed" - - diff --git a/spec/integration/api/comments_controller_spec.rb b/spec/integration/api/comments_controller_spec.rb index a3fdf5423..94aac5e60 100644 --- a/spec/integration/api/comments_controller_spec.rb +++ b/spec/integration/api/comments_controller_spec.rb @@ -7,23 +7,33 @@ describe Api::V1::CommentsController do let!(:access_token) { auth.create_access_token.to_s } before do - @status = auth.user.post( + @status = alice.post( "Post", status_message: {text: "This is a status message"}, public: true, to: "all", type: "Post" ) + + @eves_post = eve.post( + "Post", + status_message: {text: "This is a status message"}, + public: true, + to: "all", + type: "Post" + ) + + @comment_on_eves_post = comment_service.create(@eves_post.guid, "Comment on eve's post") end describe "#create" do context "valid post ID" do it "succeeds in adding a comment" do - create_comment(@status.guid, "This is a comment") + comment_text = "This is a comment" + create_comment(@status.guid, comment_text) expect(response.status).to eq(201) comment = response_body(response) - expect(comment["body"]).to eq("This is a comment") - expect(comment_service.find!(comment["guid"])).to_not be_nil + confirm_comment_format(comment, auth.user, comment_text) end end @@ -31,14 +41,26 @@ describe Api::V1::CommentsController do it "fails at adding a comment" do create_comment("999_999_999", "This is a comment") expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.posts.post_not_found")) + end + end + + context "lack of permissions" do + it "fails at adding a comment" do + alice.blocks.create(person: auth.user.person) + create_comment(@status.guid, "That shouldn't be there because I am ignored by this user") + expect(response.status).to eq(422) + expect(response.body).to eq(I18n.t("api.endpoint_errors.comments.not_allowed")) end end end describe "#read" do before do - create_comment(@status.guid, "This is a comment") - create_comment(@status.guid, "This is a comment 2") + @comment_text1 = "This is a comment" + @comment_text2 = "This is a comment 2" + create_comment(@status.guid, @comment_text1) + create_comment(@status.guid, @comment_text2) end context "valid post ID" do @@ -49,6 +71,9 @@ describe Api::V1::CommentsController do ) expect(response.status).to eq(200) expect(response_body(response).length).to eq(2) + comments = response_body(response) + confirm_comment_format(comments[0], auth.user, @comment_text1) + confirm_comment_format(comments[1], auth.user, @comment_text2) end end @@ -59,6 +84,7 @@ describe Api::V1::CommentsController do params: {access_token: access_token} ) expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.posts.post_not_found")) end end end @@ -85,6 +111,20 @@ describe Api::V1::CommentsController do end end + context "invalid Post ID" do + it "fails at deleting comment" do + delete( + api_v1_post_comment_path( + post_id: "999_999_999", + id: @comment_guid + ), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.posts.post_not_found")) + end + end + context "invalid comment ID" do it "fails at deleting comment" do delete( @@ -97,6 +137,35 @@ describe Api::V1::CommentsController do expect(response.status).to eq(404) end end + + context "mismatched post-to-comment ID" do + it "fails at deleting comment" do + delete( + api_v1_post_comment_path( + post_id: @status.guid, + id: @comment_on_eves_post.guid + ), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.comments.not_found")) + end + end + + context "insufficient permissions (not your comment and not owner)" do + it "fails at deleting comment" do + alices_comment = comment_service(alice).create(@status.guid, "Alice's comment") + delete( + api_v1_post_comment_path( + post_id: @status.guid, + id: alices_comment.guid + ), + params: {access_token: access_token} + ) + expect(response.status).to eq(403) + expect(response.body).to eq(I18n.t("api.endpoint_errors.comments.no_delete")) + end + end end describe "#report" do @@ -137,12 +206,76 @@ describe Api::V1::CommentsController do } ) expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.comments.not_found")) + end + end + + context "invalid Post ID" do + it "fails at reporting comment" do + post( + api_v1_post_comment_report_path( + post_id: "999_999_999", + comment_id: @comment_guid + ), + params: { + reason: "bad comment", + access_token: access_token + } + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.posts.post_not_found")) + end + end + + context "mismatched post-to-comment ID" do + it "fails at reporting comment" do + post( + api_v1_post_comment_report_path( + post_id: @status.guid, + comment_id: @comment_on_eves_post.guid + ), + params: { + reason: "bad comment", + access_token: access_token + } + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.comments.not_found")) + end + end + + context "already reported" do + it "fails at reporting comment" do + post( + api_v1_post_comment_report_path( + post_id: @status.guid, + comment_id: @comment_guid + ), + params: { + reason: "bad comment", + access_token: access_token + } + ) + expect(response.status).to eq(204) + + post( + api_v1_post_comment_report_path( + post_id: @status.guid, + comment_id: @comment_guid + ), + params: { + reason: "bad comment", + access_token: access_token + } + ) + expect(response.status).to eq(409) + expect(response.body).to eq(I18n.t("api.endpoint_errors.comments.duplicate_report")) end end end - def comment_service - CommentService.new(auth.user) + def comment_service(user=auth.user) + CommentService.new(user) end def create_comment(post_guid, text) @@ -155,4 +288,19 @@ describe Api::V1::CommentsController do def response_body(response) JSON.parse(response.body) end + + private + + # rubocop:disable Metrics/AbcSize + def confirm_comment_format(comment, user, comment_text) + expect(comment.has_key?("guid")).to be_truthy + expect(comment.has_key?("created_at")).to be_truthy + expect(comment["body"]).to eq(comment_text) + author = comment["author"] + expect(author["guid"]).to eq(user.guid) + expect(author["diaspora_id"]).to eq(user.diaspora_handle) + expect(author["name"]).to eq(user.name) + expect(author["avatar"]).to eq(user.profile.image_url) + end + # rubocop:enable Metrics/AbcSize end diff --git a/spec/services/comment_service_spec.rb b/spec/services/comment_service_spec.rb index e5e6a59ca..a0d027632 100644 --- a/spec/services/comment_service_spec.rb +++ b/spec/services/comment_service_spec.rb @@ -89,7 +89,7 @@ describe CommentService do it "does not let someone destroy others comment" do expect { CommentService.new(eve).destroy!(comment.guid) - }.to raise_error ActiveRecord::RecordNotFound + }.to raise_error ActiveRecord::RecordInvalid end it "raises exception the comment does not exist" do