From 454be1b468cc8c79c6f7747b10599f8d9987f1e5 Mon Sep 17 00:00:00 2001 From: Frank Rousseau Date: Fri, 26 May 2017 23:17:52 +0200 Subject: [PATCH] =?UTF-8?q?Make=20conversation=20API=C2=A0data=20format=20?= =?UTF-8?q?ok=20with=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Base the API requesting on GUID instead of ID * Include read field * Do not include messages in conversation results --- .../api/v0/conversations_controller.rb | 4 +-- app/presenters/conversation_presenter.rb | 22 ++++++------ app/presenters/person_presenter.rb | 9 +++++ app/services/conversation_service.rb | 21 +++++++---- .../api/conversations_controller_spec.rb | 35 ++++++++++--------- spec/services/conversation_service.rb | 14 ++++---- 6 files changed, 61 insertions(+), 44 deletions(-) diff --git a/app/controllers/api/v0/conversations_controller.rb b/app/controllers/api/v0/conversations_controller.rb index 638b1a129..e977297b5 100644 --- a/app/controllers/api/v0/conversations_controller.rb +++ b/app/controllers/api/v0/conversations_controller.rb @@ -30,7 +30,7 @@ module Api def create conversation = conversation_service.build( params[:subject], - params[:text], + params[:body], params[:recipients] ) conversation.save! @@ -54,7 +54,7 @@ module Api end def conversation_as_json(conversation) - ConversationPresenter.new(conversation).as_json + ConversationPresenter.new(conversation).as_api_json end end end diff --git a/app/presenters/conversation_presenter.rb b/app/presenters/conversation_presenter.rb index 65abd1f2b..1d551d8bf 100644 --- a/app/presenters/conversation_presenter.rb +++ b/app/presenters/conversation_presenter.rb @@ -1,17 +1,17 @@ class ConversationPresenter < BasePresenter - def initialize(conversation) - @conversation = conversation - end - def as_json + def as_api_json + read = + @presentable.conversation_visibilities.length > 0 and + @presentable.conversation_visibilities[0].unread == 0 { - id: @conversation.id, - guid: @conversation.guid, - created_at: @conversation.created_at, - subject: @conversation.subject, - messages: @conversation.messages.map {|x| x.as_json["message"] }, - author: @conversation.author, - participants: @conversation.participants + guid: @presentable.guid, + subject: @presentable.subject, + created_at: @presentable.created_at, + read: read, + participants: @presentable.participants.map { + |x| PersonPresenter.new(x).as_api_json + } } end end diff --git a/app/presenters/person_presenter.rb b/app/presenters/person_presenter.rb index bb415cbf7..1532e1791 100644 --- a/app/presenters/person_presenter.rb +++ b/app/presenters/person_presenter.rb @@ -10,6 +10,15 @@ class PersonPresenter < BasePresenter } end + def as_api_json + { + guid: guid, + diaspora_id: diaspora_handle, + name: name, + avatar: AvatarPresenter.new(@presentable).medium, + } + end + def full_hash base_hash_with_contact.merge( relationship: relationship, diff --git a/app/services/conversation_service.rb b/app/services/conversation_service.rb index 345c55c84..7c8d17160 100644 --- a/app/services/conversation_service.rb +++ b/app/services/conversation_service.rb @@ -4,7 +4,12 @@ class ConversationService end def all_for_user - Conversation.where(author_id: @user.person.id).all + Conversation.where(author_id: @user.person.id) + .joins(:conversation_visibilities) + .where(conversation_visibilities: { + person_id: @user.person_id + }) + .all end def build(subject, text, recipients) @@ -22,24 +27,26 @@ class ConversationService @user.build_conversation(opts) end - def find!(conversation_id) + def find!(conversation_guid) + conversation = Conversation.find_by!({guid: conversation_guid}) @user.conversations .joins(:conversation_visibilities) .where(conversation_visibilities: { person_id: @user.person_id, - conversation_id: conversation_id + conversation_id: conversation.id }).first! end - def destroy!(conversation_id) - conversation = find!(conversation_id) + def destroy!(conversation_guid) + conversation = find!(conversation_guid) conversation.destroy! end - def get_visibility(conversation_id) + def get_visibility(conversation_guid) + conversation = find!(conversation_guid) ConversationVisibility.where( person_id: @user.person.id, - conversation_id: conversation_id + conversation_id: conversation.id ).first! end end diff --git a/spec/integration/api/conversations_controller_spec.rb b/spec/integration/api/conversations_controller_spec.rb index ac84f7ced..e6c45adec 100644 --- a/spec/integration/api/conversations_controller_spec.rb +++ b/spec/integration/api/conversations_controller_spec.rb @@ -12,11 +12,7 @@ describe Api::V0::ConversationsController do @conversation = { author_id: auth.user.id, subject: "new conversation", - text: "first message", - messages: [{ - author: auth.user, - text: "first message" - }], + body: "first message", recipients: [alice.person.id], access_token: access_token } @@ -26,12 +22,13 @@ describe Api::V0::ConversationsController do context "with valid data" do it "creates the conversation" do post api_v0_conversations_path, @conversation - response_body = JSON.parse(response.body)["conversation"] + conversation = JSON.parse(response.body)["conversation"] expect(response.status).to eq 201 - expect(response_body["messages"][0]["id"]).to_not be_nil - expect(response_body["id"]).to_not be_nil - expect(response_body["participants"].length).to eq 2 + 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 end end @@ -63,14 +60,18 @@ describe Api::V0::ConversationsController do end it "returns the corresponding conversation" do - conversation_id = JSON.parse(response.body)["conversation"]["id"] + conversation_guid = JSON.parse(response.body)["conversation"]["guid"] get( - api_v0_conversation_path(conversation_id), + api_v0_conversation_path(conversation_guid), access_token: access_token ) expect(response.status).to eq 200 - result_id = JSON.parse(response.body)["conversation"]["id"] - expect(result_id).to eq conversation_id + conversation = JSON.parse(response.body)["conversation"] + expect(conversation["guid"]).to eq conversation_guid + expect(conversation["subject"]).to eq @conversation[:subject] + expect(conversation["created_at"]).to_not be_nil + expect(conversation["participants"].length).to eq 2 + expect(conversation["read"]).to eq true end end @@ -86,20 +87,20 @@ describe Api::V0::ConversationsController do end describe "#delete" do - context "valid conversation ID" do + context "valid conversation GUID" do before do post api_v0_conversations_path, @conversation end it "deletes the conversation" do - conversation_id = JSON.parse(response.body)["conversation"]["id"] + conversation_guid = JSON.parse(response.body)["conversation"]["guid"] delete( - api_v0_conversation_path(conversation_id), + api_v0_conversation_path(conversation_guid), access_token: access_token ) expect(response.status).to eq 204 get( - api_v0_conversation_path(conversation_id), + api_v0_conversation_path(conversation_guid), access_token: access_token ) expect(response.status).to eq 404 diff --git a/spec/services/conversation_service.rb b/spec/services/conversation_service.rb index ea8069993..857e8c08c 100644 --- a/spec/services/conversation_service.rb +++ b/spec/services/conversation_service.rb @@ -9,13 +9,13 @@ describe ConversationService do describe "#find!" do it "returns the conversation, if it is the user's conversation" do - expect(alice_conversation_service.find!(conversation.id)).to eq( + expect(alice_conversation_service.find!(conversation.guid)).to eq( conversation ) end it "returns the conversation, if the user is recipient" do - expect(bob_conversation_service.find!(conversation.id)).to eq( + expect(bob_conversation_service.find!(conversation.guid)).to eq( conversation ) end @@ -28,7 +28,7 @@ describe ConversationService do it "raises RecordNotFound if the user is not recipient" do expect { - eve_conversation_service.find!(conversation.id) + eve_conversation_service.find!(conversation.guid) }.to raise_error ActiveRecord::RecordNotFound end end @@ -61,7 +61,7 @@ describe ConversationService do describe "#get_visibility" do it "returns visibility for current user" do - visibility = alice_conversation_service.get_visibility(conversation.id) + visibility = alice_conversation_service.get_visibility(conversation.guid) expect(visibility).to_not be_nil end @@ -74,9 +74,9 @@ describe ConversationService do describe "#destroy!" do it "deletes the conversation, when it is the user conversation" do - alice_conversation_service.destroy!(conversation.id) + alice_conversation_service.destroy!(conversation.guid) expect { - alice_conversation_service.find!(conversation.id) + alice_conversation_service.find!(conversation.guid) }.to raise_error ActiveRecord::RecordNotFound end @@ -88,7 +88,7 @@ describe ConversationService do it "raises RecordNotFound if the user is not part of the conversation" do expect { - eve_conversation_service.destroy!(conversation.id) + eve_conversation_service.destroy!(conversation.guid) }.to raise_error ActiveRecord::RecordNotFound end end