diff --git a/app/controllers/api/v1/conversations_controller.rb b/app/controllers/api/v1/conversations_controller.rb index ed843cbd3..894077a41 100644 --- a/app/controllers/api/v1/conversations_controller.rb +++ b/app/controllers/api/v1/conversations_controller.rb @@ -14,40 +14,42 @@ module Api end rescue_from ActiveRecord::RecordNotFound do - render( - json: {error: I18n.t("conversations.not_found")}, - status: :not_found - ) + render json: I18n.t("api.endpoint_errors.conversations.not_found"), status: :not_found end def index - params.permit(:only_after, :unread) - conversations = conversation_service.all_for_user(params) + params.permit(:only_after, :only_unread) + mapped_params = {} + mapped_params[:only_after] = params[:only_after] if params.has_key?(:only_after) + + mapped_params[:unread] = params[:only_unread] if params.has_key?(:only_unread) + conversations = conversation_service.all_for_user(mapped_params) render json: conversations.map {|x| conversation_as_json(x) } end def show conversation = conversation_service.find!(params[:id]) - render json: { - conversation: conversation_as_json(conversation) - } + render json: conversation_as_json(conversation) end def create + params.require(%i[subject body recipients]) + recipient_ids = JSON.parse(params[:recipients]).map {|p| Person.find_from_guid_or_username(id: p).id } conversation = conversation_service.build( params[:subject], params[:body], - params[:recipients] + recipient_ids ) + raise ActiveRecord::RecordInvalid unless conversation.participants.length == (recipient_ids.length + 1) conversation.save! Diaspora::Federation::Dispatcher.defer_dispatch( current_user, conversation ) - render json: { - conversation: conversation_as_json(conversation) - }, status: :created + render json: conversation_as_json(conversation), status: :created + rescue ActiveRecord::RecordInvalid, ActionController::ParameterMissing, ActiveRecord::RecordNotFound + render json: I18n.t("api.endpoint_errors.conversations.cant_process"), status: :unprocessable_entity end def destroy diff --git a/app/controllers/api/v1/messages_controller.rb b/app/controllers/api/v1/messages_controller.rb index 76da744be..119fe91ab 100644 --- a/app/controllers/api/v1/messages_controller.rb +++ b/app/controllers/api/v1/messages_controller.rb @@ -12,19 +12,18 @@ module Api end rescue_from ActiveRecord::RecordNotFound do - render( - json: {error: I18n.t("conversations.not_found")}, - status: :not_found - ) + render json: I18n.t("api.endpoint_errors.conversations.not_found"), status: :not_found end def create conversation = conversation_service.find!(params[:conversation_id]) - opts = params.require(:body) - message = current_user.build_message(conversation, text: opts[:body]) + text = params.require(:body) + message = current_user.build_message(conversation, text: text) message.save! Diaspora::Federation::Dispatcher.defer_dispatch(current_user, message) render json: message_json(message), status: :created + rescue ActionController::ParameterMissing + render json: I18n.t("api.endpoint_errors.conversations.cant_process"), status: :unprocessable_entity end def index diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 359969940..8ff068b06 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -951,6 +951,9 @@ en: login_required: "You must first login before you can authorize this application" could_not_authorize: "The application could not be authorized" endpoint_errors: + conversations: + not_found: "Conversation with provided guid could not be found" + cant_process: "Couldn’t accept or process the conversation" comments: not_found: "Comment not found for the given post" not_allowed: "User is not allowed to comment" @@ -1374,3 +1377,5 @@ en: disabled: "Not available" open: "Open" closed: "Closed" + + diff --git a/spec/integration/api/conversations_controller_spec.rb b/spec/integration/api/conversations_controller_spec.rb index fdfdfd64f..789e31c09 100644 --- a/spec/integration/api/conversations_controller_spec.rb +++ b/spec/integration/api/conversations_controller_spec.rb @@ -9,15 +9,14 @@ describe Api::V1::ConversationsController do let!(:access_token_participant) { auth_participant.create_access_token.to_s } before do - auth.user.share_with bob.person, auth.user.aspects[0] auth.user.share_with alice.person, auth.user.aspects[0] alice.share_with auth.user.person, alice.aspects[0] + auth.user.disconnected_by(eve) @conversation = { - author_id: auth.user.id, subject: "new conversation", body: "first message", - recipients: [alice.person.id], + recipients: JSON.generate([alice.guid]), access_token: access_token } end @@ -26,22 +25,74 @@ describe Api::V1::ConversationsController do context "with valid data" do it "creates the conversation" do post api_v1_conversations_path, params: @conversation - @conversation_guid = JSON.parse(response.body)["conversation"]["guid"] - conversation = JSON.parse(response.body)["conversation"] - expect(response.status).to eq 201 - expect(conversation["guid"]).to_not be_nil - expect(conversation["subject"]).to eq @conversation[:subject] - expect(conversation["created_at"]).to_not be_nil - expect(conversation["participants"].length).to eq 2 - conversation_service.find!(@conversation_guid) + conversation = JSON.parse(response.body) + confirm_conversation_format(conversation, @conversation, [auth.user, alice]) end end context "without valid data" do - it "fails at creating the conversation" do + it "fails with empty body" do post api_v1_conversations_path, params: {access_token: access_token} expect(response.status).to eq 422 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.cant_process")) + end + + it "fails with missing subject " do + incomplete_convo = { + body: "first message", + recipients: [alice.guid], + access_token: access_token + } + post api_v1_conversations_path, params: incomplete_convo + expect(response.status).to eq 422 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.cant_process")) + end + + it "fails with missing body " do + incomplete_convo = { + subject: "new conversation", + recipients: [alice.guid], + access_token: access_token + } + post api_v1_conversations_path, params: incomplete_convo + expect(response.status).to eq 422 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.cant_process")) + end + + it "fails with missing recipients " do + incomplete_convo = { + subject: "new conversation", + body: "first message", + access_token: access_token + } + post api_v1_conversations_path, params: incomplete_convo + expect(response.status).to eq 422 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.cant_process")) + end + + it "fails with bad recipient ID " do + incomplete_convo = { + subject: "new conversation", + body: "first message", + recipients: JSON.generate(["999_999_999"]), + access_token: access_token + } + post api_v1_conversations_path, params: incomplete_convo + expect(response.status).to eq 422 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.cant_process")) + end + + it "fails with invalid recipient (not allowed to message) " do + incomplete_convo = { + subject: "new conversation", + body: "first message", + recipients: JSON.generate([eve.guid]), + access_token: access_token + } + post api_v1_conversations_path, params: incomplete_convo + expect(response.status).to eq 422 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.cant_process")) end end end @@ -52,7 +103,7 @@ describe Api::V1::ConversationsController do post api_v1_conversations_path, params: @conversation sleep(1) post api_v1_conversations_path, params: @conversation - conversation_guid = JSON.parse(response.body)["conversation"]["guid"] + conversation_guid = JSON.parse(response.body)["guid"] conversation = conversation_service.find!(conversation_guid) conversation.conversation_visibilities[0].unread = 1 conversation.conversation_visibilities[0].save! @@ -64,13 +115,15 @@ describe Api::V1::ConversationsController do it "returns all the user conversations" do get api_v1_conversations_path, params: {access_token: access_token} expect(response.status).to eq 200 - expect(JSON.parse(response.body).length).to eq 3 + returned_convos = JSON.parse(response.body) + expect(returned_convos.length).to eq 3 + confirm_conversation_format(returned_convos[0], @conversation, [auth.user, alice]) end it "returns all the user unread conversations" do get( api_v1_conversations_path, - params: {unread: true, access_token: access_token} + params: {only_unread: true, access_token: access_token} ) expect(response.status).to eq 200 expect(JSON.parse(response.body).length).to eq 2 @@ -93,17 +146,14 @@ describe Api::V1::ConversationsController do end it "returns the corresponding conversation" do - conversation_guid = JSON.parse(response.body)["conversation"]["guid"] + conversation_guid = JSON.parse(response.body)["guid"] get( api_v1_conversation_path(conversation_guid), params: {access_token: access_token} ) expect(response.status).to eq 200 - conversation = JSON.parse(response.body)["conversation"] - expect(conversation["guid"]).to eq conversation_guid - expect(conversation["subject"]).to eq @conversation[:subject] - expect(conversation["participants"].length).to eq 2 - expect(conversation["read"]).to eq true + conversation = JSON.parse(response.body) + confirm_conversation_format(conversation, @conversation, [auth.user, alice]) end end @@ -114,6 +164,7 @@ describe Api::V1::ConversationsController do params: {access_token: access_token} ) expect(response.status).to eq 404 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.not_found")) end end end @@ -127,14 +178,13 @@ describe Api::V1::ConversationsController do ) @conversation = { - author_id: auth.user.id, subject: "new conversation", body: "first message", - recipients: [auth_participant.user.person.id], + recipients: JSON.generate([auth_participant.user.guid]), access_token: access_token } post api_v1_conversations_path, params: @conversation - @conversation_guid = JSON.parse(response.body)["conversation"]["guid"] + @conversation_guid = JSON.parse(response.body)["guid"] end context "destroy" do @@ -149,6 +199,7 @@ describe Api::V1::ConversationsController do params: {access_token: access_token} ) expect(response.status).to eq 404 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.not_found")) get api_v1_conversation_path( @conversation_guid, params: {access_token: access_token_participant} @@ -174,6 +225,7 @@ describe Api::V1::ConversationsController do params: {access_token: access_token_participant} ) expect(response.status).to eq 404 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.not_found")) expect { Conversation.find(guid: @conversation_guid) @@ -188,6 +240,7 @@ describe Api::V1::ConversationsController do params: {access_token: access_token} ) expect(response.status).to eq 404 + expect(response.body).to eq(I18n.t("api.endpoint_errors.conversations.not_found")) end end end @@ -195,4 +248,33 @@ describe Api::V1::ConversationsController do def conversation_service ConversationService.new(alice) end + + private + + # rubocop:disable Metrics/AbcSize + def confirm_conversation_format(conversation, ref_convo, ref_participants) + expect(conversation["guid"]).to_not be_nil + conversation_service.find!(conversation["guid"]) + expect(conversation["subject"]).to eq ref_convo[:subject] + expect(conversation["created_at"]).to_not be_nil + expect(conversation["read"]).to be_truthy + expect(conversation["participants"].length).to eq(ref_participants.length) + participants = conversation["participants"] + + expect(participants.length).to eq(ref_participants.length) + ref_participants.each do |p| + conversation_participant = participants.find {|cp| cp["guid"] == p.guid } + confirm_person_format(conversation_participant, p) + end + end + # rubocop:enable Metrics/AbcSize + + # rubocop:disable Metrics/AbcSize + def confirm_person_format(post_person, user) + expect(post_person["guid"]).to eq(user.guid) + expect(post_person["diaspora_id"]).to eq(user.diaspora_handle) + expect(post_person["name"]).to eq(user.name) + expect(post_person["avatar"]).to eq(user.profile.image_url) + end + # rubocop:enable Metrics/AbcSize end diff --git a/spec/integration/api/messages_controller_spec.rb b/spec/integration/api/messages_controller_spec.rb index ae0c8fd2c..cbedc52b5 100644 --- a/spec/integration/api/messages_controller_spec.rb +++ b/spec/integration/api/messages_controller_spec.rb @@ -13,37 +13,31 @@ describe Api::V1::MessagesController do alice.share_with auth.user.person, alice.aspects[0] @conversation = { - author_id: auth.user.id, subject: "new conversation", body: "first message", - recipients: [alice.person.id], + recipients: JSON.generate([alice.guid]), access_token: access_token } - @message = { - body: "reply to first message" - } + @message_text = "reply to first message" end describe "#create " do before do post api_v1_conversations_path, params: @conversation - @conversation_guid = JSON.parse(response.body)["conversation"]["guid"] + @conversation_guid = JSON.parse(response.body)["guid"] end context "with valid data" do it "creates the message in the conversation scope" do post( api_v1_conversation_messages_path(@conversation_guid), - params: {body: @message, access_token: access_token} + params: {body: @message_text, access_token: access_token} ) expect(response.status).to eq 201 message = JSON.parse(response.body) - expect(message["guid"]).to_not be_nil - expect(message["author"]).to_not be_nil - expect(message["created_at"]).to_not be_nil - expect(message["body"]).to_not be_nil + confirm_message_format(message, @message_text, auth.user) get( api_v1_conversation_messages_path(@conversation_guid), @@ -51,18 +45,27 @@ describe Api::V1::MessagesController do ) messages = JSON.parse(response.body) expect(messages.length).to eq 2 - text = messages[1]["body"] - expect(text).to eq @message[:body] + confirm_message_format(messages[1], @message_text, auth.user) end end context "without valid data" do - it "returns a wrong parameter error (400)" do + it "no data returns a unprocessable entity (422)" do post( api_v1_conversation_messages_path(@conversation_guid), params: {access_token: access_token} ) expect(response.status).to eq 422 + expect(response.body).to eq I18n.t("api.endpoint_errors.conversations.cant_process") + end + + it "empty string returns a unprocessable entity (422)" do + post( + api_v1_conversation_messages_path(@conversation_guid), + params: {body: "", access_token: access_token} + ) + expect(response.status).to eq 422 + expect(response.body).to eq I18n.t("api.endpoint_errors.conversations.cant_process") end end @@ -73,6 +76,7 @@ describe Api::V1::MessagesController do params: {access_token: access_token} ) expect(response.status).to eq 404 + expect(response.body).to eq I18n.t("api.endpoint_errors.conversations.not_found") end end end @@ -80,7 +84,7 @@ describe Api::V1::MessagesController do describe "#index " do before do post api_v1_conversations_path, params: @conversation - @conversation_guid = JSON.parse(response.body)["conversation"]["guid"] + @conversation_guid = JSON.parse(response.body)["guid"] end context "retrieving messages" do @@ -92,12 +96,34 @@ describe Api::V1::MessagesController do messages = JSON.parse(response.body) expect(messages.length).to eq 1 - message = messages[0] - expect(message["guid"]).to_not be_nil - expect(message["author"]).to_not be_nil - expect(message["created_at"]).to_not be_nil - expect(message["body"]).to_not be_nil + confirm_message_format(messages[0], "first message", auth.user) + conversation = get_conversation(@conversation_guid) + expect(conversation[:read]).to be_truthy end end end + + private + + def get_conversation(conversation_id) + conversation_service = ConversationService.new(auth.user) + raw_conversation = conversation_service.find!(conversation_id) + ConversationPresenter.new(raw_conversation).as_api_json + end + + def confirm_message_format(message, ref_message, author) + expect(message["guid"]).to_not be_nil + expect(message["created_at"]).to_not be_nil + expect(message["body"]).to eq ref_message + confirm_person_format(message["author"], author) + end + + # rubocop:disable Metrics/AbcSize + def confirm_person_format(post_person, user) + expect(post_person["guid"]).to eq(user.guid) + expect(post_person["diaspora_id"]).to eq(user.diaspora_handle) + expect(post_person["name"]).to eq(user.name) + expect(post_person["avatar"]).to eq(user.profile.image_url) + end + # rubocop:enable Metrics/AbcSize end