diff --git a/app/models/conversation.rb b/app/models/conversation.rb index edefec62b..873cb7f58 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -2,9 +2,9 @@ class Conversation < ActiveRecord::Base include Diaspora::Federated::Base include Diaspora::Guid - has_many :conversation_visibilities, :dependent => :destroy - has_many :participants, :class_name => 'Person', :through => :conversation_visibilities, :source => :person - has_many :messages, -> { order('created_at ASC') } + has_many :conversation_visibilities, dependent: :destroy + has_many :participants, class_name: "Person", through: :conversation_visibilities, source: :person + has_many :messages, -> { order("created_at ASC") }, inverse_of: :conversation belongs_to :author, class_name: "Person" delegate :diaspora_handle, to: :author @@ -50,10 +50,6 @@ class Conversation < ActiveRecord::Base end end - def public? - false - end - def participant_handles participants.map(&:diaspora_handle).join(";") end diff --git a/app/models/message.rb b/app/models/message.rb index eeb9b401c..8138b433e 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -1,7 +1,6 @@ class Message < ActiveRecord::Base include Diaspora::Federated::Base include Diaspora::Guid - include Diaspora::Relayable belongs_to :author, class_name: "Person" belongs_to :conversation, touch: true @@ -9,9 +8,12 @@ class Message < ActiveRecord::Base delegate :diaspora_handle, to: :author delegate :name, to: :author, prefix: true + # TODO: can be removed when messages are not relayed anymore alias_attribute :parent, :conversation - validates :text, :presence => true + validates :conversation, presence: true + validates :author, presence: true + validates :text, presence: true validate :participant_of_parent_conversation def diaspora_handle=(nh) @@ -19,9 +21,7 @@ class Message < ActiveRecord::Base end def conversation_guid=(guid) - if cnv = Conversation.find_by_guid(guid) - self.conversation_id = cnv.id - end + self.conversation_id = Conversation.where(guid: guid).ids.first end def increase_unread(user) @@ -35,6 +35,15 @@ class Message < ActiveRecord::Base @message ||= Diaspora::MessageRenderer.new text end + # @return [Array] + def subscribers + if author.local? + conversation.participants + else # for relaying, TODO: can be removed when messages are not relayed anymore + conversation.participants.remote + end + end + private def participant_of_parent_conversation diff --git a/spec/factories.rb b/spec/factories.rb index d90b01842..55df861e6 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -294,9 +294,8 @@ FactoryGirl.define do factory(:conversation_with_message, parent: :conversation) do after(:create) do |c| - msg = FactoryGirl.build(:message) + msg = FactoryGirl.build(:message, author: c.author) msg.conversation_id = c.id - c.participants << msg.author msg.save end end diff --git a/spec/lib/diaspora/federation/entities_spec.rb b/spec/lib/diaspora/federation/entities_spec.rb index 409ad3472..5651d486f 100644 --- a/spec/lib/diaspora/federation/entities_spec.rb +++ b/spec/lib/diaspora/federation/entities_spec.rb @@ -32,7 +32,8 @@ describe Diaspora::Federation::Entities do end context "Conversation" do - let(:diaspora_entity) { FactoryGirl.create(:conversation_with_message) } + let(:participant) { FactoryGirl.create(:person) } + let(:diaspora_entity) { FactoryGirl.create(:conversation_with_message, participants: [participant]) } let(:federation_entity) { described_class.build(diaspora_entity) } it "builds a conversation" do @@ -45,7 +46,7 @@ describe Diaspora::Federation::Entities do it "adds the participants" do expect(federation_entity.participants) - .to eq("#{diaspora_entity.author.diaspora_handle};#{diaspora_entity.messages.first.author.diaspora_handle}") + .to eq("#{participant.diaspora_handle};#{diaspora_entity.author.diaspora_handle}") end it "includes the message" do diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index 0081f5736..ec853498c 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -21,7 +21,7 @@ describe Message, type: :model do expect(message).not_to be_valid end - it_behaves_like "it is relayable" do + describe "#subscribers" do let(:cnv_hash) { { participant_ids: [local_luke.person, local_leia.person, remote_raphael].map(&:id), @@ -29,14 +29,23 @@ describe Message, type: :model do messages_attributes: [{author: remote_raphael, text: "hey"}] } } - let(:remote_parent) { Conversation.create(cnv_hash.merge(author: remote_raphael)) } - let(:local_parent) { Conversation.create(cnv_hash.merge(author: local_luke.person)) } - let(:object_on_local_parent) { Message.create(author: local_luke.person, text: "yo", conversation: local_parent) } - let(:object_on_remote_parent) { Message.create(author: local_luke.person, text: "yo", conversation: remote_parent) } - let(:remote_object_on_local_parent) { - Message.create(author: remote_raphael, text: "yo", conversation: local_parent) - } - let(:relayable) { Message.new(author: alice.person, text: "ohai!", conversation: conversation) } + let(:local_conv) { Conversation.create(cnv_hash.merge(author: local_luke.person)) } + let(:remote_conv) { Conversation.create(cnv_hash.merge(author: remote_raphael)) } + + it "returns all participants, if the conversation and the author is local" do + message = Message.create(author: local_luke.person, text: "yo", conversation: local_conv) + expect(message.subscribers).to match_array([local_luke.person, local_leia.person, remote_raphael]) + end + + it "returns all participants, if the author is local and the conversation is remote" do + message = Message.create(author: local_luke.person, text: "yo", conversation: remote_conv) + expect(message.subscribers).to match_array([local_luke.person, local_leia.person, remote_raphael]) + end + + it "returns only remote participants, if the conversation is local, but the author is remote" do + message = Message.create(author: remote_raphael, text: "yo", conversation: local_conv) + expect(message.subscribers).to match_array([remote_raphael]) + end end describe "#increase_unread" do