Conversations API Endpoint Feature complete with full tests

app/controllers/api/v1/conversations_controller.rb

@@ -14,40 +14,42 @@ module Api
       end
 
       rescue_from ActiveRecord::RecordNotFound do
-        render(
+        render json: I18n.t("api.endpoint_errors.conversations.not_found"), status: :not_found
-          json:   {error: I18n.t("conversations.not_found")},
-          status: :not_found
-        )
       end
 
       def index
-        params.permit(:only_after, :unread)
+        params.permit(:only_after, :only_unread)
-        conversations = conversation_service.all_for_user(params)
+        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: {
+        render json: conversation_as_json(conversation)
-          conversation: 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: {
+        render json: conversation_as_json(conversation), status: :created
-          conversation: conversation_as_json(conversation)
+      rescue ActiveRecord::RecordInvalid, ActionController::ParameterMissing, ActiveRecord::RecordNotFound
-        }, status: :created
+        render json: I18n.t("api.endpoint_errors.conversations.cant_process"), status: :unprocessable_entity
       end
 
       def destroy

app/controllers/api/v1/messages_controller.rb

@@ -12,19 +12,18 @@ module Api
       end
 
       rescue_from ActiveRecord::RecordNotFound do
-        render(
+        render json:   I18n.t("api.endpoint_errors.conversations.not_found"), status: :not_found
-          json:   {error: I18n.t("conversations.not_found")},
-          status: :not_found
-        )
       end
 
       def create
         conversation = conversation_service.find!(params[:conversation_id])
-        opts = params.require(:body)
+        text = params.require(:body)
-        message = current_user.build_message(conversation, text: opts[: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

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"
+
+    

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
+        conversation = JSON.parse(response.body)
-        expect(conversation["subject"]).to eq @conversation[:subject]
+        confirm_conversation_format(conversation, @conversation, [auth.user, alice])
-        expect(conversation["created_at"]).to_not be_nil
-        expect(conversation["participants"].length).to eq 2
-        conversation_service.find!(@conversation_guid)
       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"]
+        conversation = JSON.parse(response.body)
-        expect(conversation["guid"]).to eq conversation_guid
+        confirm_conversation_format(conversation, @conversation, [auth.user, alice])
-        expect(conversation["subject"]).to eq @conversation[:subject]
-        expect(conversation["participants"].length).to eq 2
-        expect(conversation["read"]).to eq true
       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

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 = {
+    @message_text = "reply to first message"
-      body: "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
+        confirm_message_format(message, @message_text, auth.user)
-        expect(message["author"]).to_not be_nil
-        expect(message["created_at"]).to_not be_nil
-        expect(message["body"]).to_not be_nil
 
         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"]
+        confirm_message_format(messages[1], @message_text, auth.user)
-        expect(text).to eq @message[:body]
       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]
+        confirm_message_format(messages[0], "first message", auth.user)
-        expect(message["guid"]).to_not be_nil
+        conversation = get_conversation(@conversation_guid)
-        expect(message["author"]).to_not be_nil
+        expect(conversation[:read]).to be_truthy
-        expect(message["created_at"]).to_not be_nil
-        expect(message["body"]).to_not be_nil
       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