ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Cross-shard conversations 

fixes CNVS-1171

test plan:
 * full conversations regression test
 * initiate a conversation with a user from another shard
 * reply to that conversation from both the sender and the
   receiver
 * repeat for a group conversation involving two or more
   shards
 * repeat for huge batch conversations with hundreds of
   users and two or more shards
 * known NOT working yet:
   * re-using the correct cross-shard private conversation
   * probably the tagging of messages with Course x,
     Group y, etc.

Change-Id: I52549039875941cd518077cea4e28bfd2bc10dbf
Reviewed-on: https://gerrit.instructure.com/16523
Reviewed-by: Cody Cutrer <cody@instructure.com>
QA-Review: Clare Hetherington <clare@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
This commit is contained in:
Cody Cutrer 2012-11-28 15:53:00 -07:00
parent dd574808c2
commit e8e81deb5b
49 changed files with 935 additions and 538 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
app/controllers/conversations_controller.rb
View File

@ -115,7 +115,7 @@ class ConversationsController < ApplicationController
if request.format == :json
conversations = Api.paginate(@conversations_scope, self, api_v1_conversations_url)
# optimize loading the most recent messages for each conversation into a single query
ConversationParticipant.preload_latest_messages(conversations, @current_user.id)
ConversationParticipant.preload_latest_messages(conversations, @current_user)
@conversations_json = conversations.map{ |c| conversation_json(c, @current_user, session, :include_participant_avatars => false, :include_participant_contexts => false, :visible => true) }
if params[:include_all_conversation_ids]
@ -123,10 +123,6 @@ class ConversationsController < ApplicationController
end
render :json => @conversations_json
else
if @current_user.shard != Shard.current
flash[:notice] = 'Conversations are not yet cross-shard enabled'
return redirect_to dashboard_url
end
return redirect_to conversations_path(:scope => params[:redirect_scope]) if params[:redirect_scope]
@current_user.reset_unread_conversations_counter
load_all_contexts :permissions => [:manage_user_notes]
@ -182,12 +178,11 @@ class ConversationsController < ApplicationController
return render_error('body', 'blank') if params[:body].blank?
batch_private_messages = !value_to_boolean(params[:group_conversation]) && @recipients.size > 1
recipient_ids = @recipients.keys
message = build_message
if batch_private_messages
mode = params[:mode] == 'async' ? :async : :sync
batch = ConversationBatch.generate(message, recipient_ids, mode, :user_map => @recipients, :tags => @tags)
batch = ConversationBatch.generate(message, @recipients, mode, :tags => @tags)
if mode == :async
headers['X-Conversation-Batch-Id'] = batch.id.to_s
return render :json => [], :status => :accepted
@ -196,11 +191,11 @@ class ConversationsController < ApplicationController
# reload and preload stuff
conversations = ConversationParticipant.find(:all, :conditions => {:id => batch.conversations.map(&:id)}, :include => [:conversation], :order => "visible_last_authored_at DESC, last_message_at DESC, id DESC")
Conversation.preload_participants(conversations.map(&:conversation))
ConversationParticipant.preload_latest_messages(conversations, @current_user.id)
ConversationParticipant.preload_latest_messages(conversations, @current_user)
visibility_map = infer_visibility(*conversations)
render :json => conversations.map{ |c| conversation_json(c, @current_user, session, :include_participant_avatars => false, :include_participant_contexts => false, :visible => visibility_map[c.conversation_id]) }, :status => :created
else
@conversation = @current_user.initiate_conversation(recipient_ids)
@conversation = @current_user.initiate_conversation(@recipients)
@conversation.add_message(message, :tags => @tags)
render :json => [conversation_json(@conversation.reload, @current_user, session, :include_indirect_participants => true, :messages => [message])], :status => :created
end
@ -472,7 +467,7 @@ class ConversationsController < ApplicationController
#
def add_recipients
if @recipients.present?
@conversation.add_participants(@recipients.keys, :tags => @tags, :root_account_id => @domain_root_account.id)
@conversation.add_participants(@recipients, :tags => @tags, :root_account_id => @domain_root_account.id)
render :json => conversation_json(@conversation.reload, @current_user, session, :messages => [@conversation.messages.first])
else
render :json => {}, :status => :bad_request
@ -554,11 +549,7 @@ class ConversationsController < ApplicationController
# }
def remove_messages
if params[:remove]
to_delete = []
@conversation.messages.each do |message|
to_delete << message if params[:remove].include?(message.id.to_s)
end
@conversation.remove_messages(*to_delete)
@conversation.remove_messages(*@conversation.messages.find_all_by_id(*params[:remove]))
render :json => conversation_json(@conversation, @current_user, session)
end
end
@ -686,14 +677,11 @@ class ConversationsController < ApplicationController
if recipient_ids.is_a?(String)
params[:recipients] = recipient_ids = recipient_ids.split(/,/)
end
recipients = @current_user.messageable_users(:ids => recipient_ids.grep(/\A\d+\z/), :conversation_id => params[:from_conversation_id])
@recipients = @current_user.messageable_users(:ids => recipient_ids.grep(/\A\d+\z/), :conversation_id => params[:from_conversation_id])
recipient_ids.grep(User::MESSAGEABLE_USER_CONTEXT_REGEX).map do |context|
recipients.concat @current_user.messageable_users(:context => context)
@recipients.concat @current_user.messageable_users(:context => context)
end
@recipients = recipients.inject({}){ |hash, user|
hash[user.id] ||= user
hash
}
@recipients.uniq!
end
end

408
app/models/conversation.rb
View File

@ -26,11 +26,12 @@ class Conversation < ActiveRecord::Base
has_one :stream_item, :as => :asset
# see also User#messageable_users
has_many :participants,
:through => :conversation_participants,
:source => :user,
:select => User::MESSAGEABLE_USER_COLUMN_SQL + ", NULL AS common_courses, NULL AS common_groups",
:order => 'last_authored_at IS NULL, last_authored_at DESC, LOWER(COALESCE(short_name, name))'
def participants(reload = false)
if !@participants || reload
Conversation.preload_participants([self])
end
@participants
end
attr_accessible
@ -46,11 +47,20 @@ class Conversation < ActiveRecord::Base
Digest::SHA1.hexdigest(user_ids.uniq.sort.join(','))
end
def self.initiate(user_ids, private)
if user_ids.first.is_a?(User)
user_ids = user_ids.map(&:id)
end
user_ids = user_ids.map(&:to_i).uniq
def bulk_insert_participants(user_ids, options = {})
options = {
:conversation_id => self.id,
:workflow_state => 'read',
:has_attachments => has_attachments?,
:has_media_objects => has_media_objects?
}.merge(options)
connection.bulk_insert('conversation_participants', user_ids.map{ |user_id|
options.merge({:user_id => user_id})
})
end
def self.initiate(users, private)
user_ids = users.uniq.map(&:id)
private_hash = private ? private_hash_for(user_ids) : nil
transaction do
unless private_hash && conversation = find_by_private_hash(private_hash)
@ -60,16 +70,14 @@ class Conversation < ActiveRecord::Base
conversation.has_media_objects = false
conversation.tags = []
conversation.save!
connection.bulk_insert('conversation_participants', user_ids.map{ |user_id|
{
:conversation_id => conversation.id,
:user_id => user_id,
:workflow_state => 'read',
:has_attachments => false,
:has_media_objects => false,
:tags => ''
}
})
# TODO: transaction on these shards as well?
Shard.partition_by_shard(user_ids) do |shard_user_ids|
next if Shard.current == conversation.shard
conversation.bulk_insert_participants(shard_user_ids, :tags => '')
end
# the conversation's shard gets a full copy
conversation.bulk_insert_participants(user_ids, :tags => '')
end
conversation
end
@ -85,7 +93,7 @@ class Conversation < ActiveRecord::Base
# * <tt>:only_existing</tt> - Boolean option. If +true+, only existing ones are updated. No new ones are created.
# Additional options are passed on further but not directly used here.
# * <tt>:update_participants</tt> - Boolean option.
# * <tt>:skip_ids</tt> - Array of IDs to skip.
# * <tt>:skip_users</tt> - Array of users to skip.
# * <tt>:recalculate_count</tt> - Boolean
# * <tt>:recalculate_last_authored_at</tt> - Boolean
def self.update_all_for_asset(asset, options)
@ -101,7 +109,7 @@ class Conversation < ActiveRecord::Base
conversations = if groups.empty?
[]
elsif options[:only_existing]
find_all_by_private_hash(groups.map{ |g| private_hash_for(g) }, :lock => true)
find_all_by_private_hash(groups.map{ |g| private_hash_for(g.map(&:id)) }, :lock => true)
else
groups.map{ |g| initiate(g, true) }.each(&:lock!)
end
@ -131,7 +139,7 @@ class Conversation < ActiveRecord::Base
end
if (data = asset.conversation_message_data).present?
message.created_at = data[:created_at]
message.author_id = data[:author_id]
message.author = data[:author]
message.body = data[:body]
message.save!
end
@ -144,48 +152,49 @@ class Conversation < ActiveRecord::Base
message
end
def add_participants(current_user, user_ids, options={})
if user_ids.first.is_a?(User)
user_ids = users_ids.map(&:id)
end
user_ids = user_ids.map(&:to_i).uniq
raise "can't add participants to a private conversation" if private?
transaction do
lock!
user_ids -= conversation_participants.map(&:user_id)
next if user_ids.empty?
def add_participants(current_user, users, options={})
self.shard.activate do
user_ids = users.uniq.map(&:id)
raise "can't add participants to a private conversation" if private?
transaction do
lock!
user_ids -= conversation_participants.map(&:user_id)
next if user_ids.empty?
last_message_at = conversation_messages.human.first.created_at
raise "can't add participants if there are no messages" unless last_message_at
num_messages = conversation_messages.human.size
last_message_at = conversation_messages.human.first.try(:created_at)
raise "can't add participants if there are no messages" unless last_message_at
num_messages = conversation_messages.human.size
User.update_all(["unread_conversations_count = unread_conversations_count + 1, updated_at = ?", Time.now.utc], :id => user_ids)
bulk_insert_options = {
:workflow_state => 'unread',
:last_message_at => last_message_at,
:message_count => num_messages
}
connection.bulk_insert('conversation_participants', user_ids.map{ |user_id|
{
:conversation_id => id,
:user_id => user_id,
:workflow_state => 'unread',
:has_attachments => has_attachments?,
:has_media_objects => has_media_objects?,
:last_message_at => last_message_at,
:message_count => num_messages
}
})
Shard.partition_by_shard(user_ids) do |shard_user_ids|
User.update_all(["unread_conversations_count = unread_conversations_count + 1, updated_at = ?", Time.now.utc], :id => shard_user_ids) unless shard_user_ids.empty?
# give them all messages
# NOTE: individual messages in group conversations don't have tags
connection.execute(sanitize_sql([<<-SQL, self.id, user_ids]))
INSERT INTO conversation_message_participants(conversation_message_id, conversation_participant_id)
SELECT conversation_messages.id, conversation_participants.id
FROM conversation_messages, conversation_participants
WHERE conversation_messages.conversation_id = ?
AND conversation_messages.conversation_id = conversation_participants.conversation_id
AND conversation_participants.user_id IN (?)
SQL
next if Shard.current == self.shard
bulk_insert_participants(shard_user_ids, bulk_insert_options)
end
# the conversation's shard gets a participant for all users
bulk_insert_participants(user_ids, bulk_insert_options)
# announce their arrival
add_event_message(current_user, {:event_type => :users_added, :user_ids => user_ids}, options)
# give them all messages
# NOTE: individual messages in group conversations don't have tags
connection.execute(sanitize_sql([<<-SQL, self.id, user_ids]))
INSERT INTO conversation_message_participants(conversation_message_id, conversation_participant_id, user_id)
SELECT conversation_messages.id, conversation_participants.id, conversation_participants.user_id
FROM conversation_messages, conversation_participants
WHERE conversation_messages.conversation_id = ?
AND conversation_messages.conversation_id = conversation_participants.conversation_id
AND conversation_participants.user_id IN (?)
SQL
# announce their arrival
add_event_message(current_user, {:event_type => :users_added, :user_ids => user_ids}, options)
end
end
end
@ -206,7 +215,7 @@ class Conversation < ActiveRecord::Base
# * <tt>:update_participants</tt> - Boolean. Defaults to true unless message was :generated. Will update all
# participants with the new message.
# * <tt>:update_for_skips</tt> - Boolean. Defaults to true (or :update_for_sender).
# * <tt>:skip_ids</tt> - Array of IDs. Defaults to the current_user only.
# * <tt>:skip_users</tt> - Array of users. Defaults to the current_user only.
# * <tt>:tags</tt> - Array of tags for the message.
# * <tt>:root_account_id</tt> - The root account ID to link to the conversation. When set, the message context
# is the Account.
@ -223,18 +232,22 @@ class Conversation < ActiveRecord::Base
:only_existing => false}.update(options)
options[:update_participants] = !options[:generated] unless options.has_key?(:update_participants)
options[:update_for_skips] = options[:update_for_sender] unless options.has_key?(:update_for_skips)
options[:skip_ids] ||= [current_user.id]
options[:skip_users] ||= [current_user]
message = body_or_obj.is_a?(ConversationMessage) ?
body_or_obj :
Conversation.build_message(current_user, body_or_obj, options)
message.conversation = self
message.shard = self.shard
# all specified (or implicit) tags, regardless of visibility to individual participants
new_tags = options[:tags] ? options[:tags] & current_context_strings(1) : []
new_tags = current_context_strings if new_tags.blank? && tags.empty? # i.e. we're creating the first message and there are no tags yet
self.tags |= new_tags if new_tags.present?
self.root_account_ids |= [message.root_account_id] if message.root_account_id
Shard.default.activate do
self.root_account_ids |= [message.root_account_id] if message.root_account_id
end
options[:root_account_ids] = read_attribute(:root_account_ids) if self.root_account_ids_changed?
save! if new_tags.present? || root_account_ids_changed?
# so we can take advantage of other preloaded associations
@ -286,32 +299,45 @@ class Conversation < ActiveRecord::Base
# the message. Otherwise, the participants receive it.
# * <tt>:tags</tt> - Array of tags for the message data.
def add_message_to_participants(message, options = {})
cps = options[:only_existing] ?
conversation_participants.visible :
conversation_participants
unless options[:new_message]
skip_ids = ConversationMessageParticipant.for_conversation_and_message(id, message.id).map(&:conversation_participant_id)
cps = cps.scoped(:conditions => ["id NOT IN (?)", skip_ids]) if skip_ids.present?
skip_users = message.conversation_message_participants.find(:all, :select => 'user_id')
end
ConversationParticipant.update_all("message_count = message_count + 1", ["id IN (?)", cps.map(&:id)]) unless options[:generated]
self.conversation_participants.with_each_shard do |cps|
cps = cps.visible if options[:only_existing]
all_new_tags = options[:tags] || []
message_data = []
ConversationMessage.preload_latest(cps) if private? && !all_new_tags.present?
cps.each do |cp|
next unless cp.user
new_tags, message_tags = infer_new_tags_for(cp, all_new_tags)
cp.update_attribute :tags, cp.tags | new_tags if new_tags.present?
message_data << {
:conversation_message_id => message.id,
:conversation_participant_id => cp.id,
:tags => message_tags ? serialized_tags(message_tags) : nil
}
unless options[:new_message]
cps = cps.scoped(:conditions => ["user_id NOT IN (?)", skip_users.map(&:user_id)]) if skip_users.present?
end
cps.update_all("message_count = message_count + 1") unless options[:generated]
if self.shard == Shard.current
all_new_tags = options[:tags] || []
message_data = []
ConversationMessage.preload_latest(cps) if private? && !all_new_tags.present?
cps.each do |cp|
next unless cp.user
new_tags, message_tags = infer_new_tags_for(cp, all_new_tags)
if new_tags.present?
cp.update_attribute :tags, cp.tags | new_tags
if cp.user.shard != self.shard
cp.user.shard.activate do
ConversationParticipant.update_all({:tags => cp.tags}, :conversation_id => self.id, :user_id => cp.user_id)
end
end
end
message_data << {
:conversation_message_id => message.id,
:conversation_participant_id => cp.id,
:user_id => cp.user_id,
:tags => message_tags ? serialized_tags(message_tags) : nil
}
end
connection.bulk_insert "conversation_message_participants", message_data
end
end
connection.bulk_insert "conversation_message_participants", message_data
end
def infer_new_tags_for(cp, all_new_tags)
@ -341,53 +367,56 @@ class Conversation < ActiveRecord::Base
end
def update_participants(message, options = {})
skip_ids = options[:skip_ids] || [message.author_id]
skip_ids = [0] if skip_ids.empty?
update_for_skips = options[:update_for_skips] != false
# make sure this jumps to the top of the inbox and is marked as unread for anyone who's subscribed
cp_conditions = sanitize_sql([
"cp.conversation_id = ? AND cp.workflow_state <> 'unread' AND (cp.last_message_at IS NULL OR cp.subscribed) AND cp.user_id NOT IN (?)",
self.id,
skip_ids
])
if connection.adapter_name =~ /mysql/i
connection.execute <<-SQL
UPDATE users, conversation_participants cp
SET unread_conversations_count = unread_conversations_count + 1
WHERE users.id = cp.user_id AND #{cp_conditions}
SQL
else
User.update_all 'unread_conversations_count = unread_conversations_count + 1',
"id IN (SELECT user_id FROM conversation_participants cp WHERE #{cp_conditions})"
end
conversation_participants.update_all(
{:last_message_at => message.created_at, :workflow_state => 'unread'},
["(last_message_at IS NULL OR subscribed) AND user_id NOT IN (?)", skip_ids]
)
# for the sender (or override(s)), we just update the timestamps (if
# needed). for last_authored_at, we ignore update_for_skips, since the
# column is only viewed by the other participants and doesn't care about
# what messages the author may have deleted
updates = [
maybe_update_timestamp('last_message_at', message.created_at, update_for_skips ? [] : ["last_message_at IS NOT NULL"]),
maybe_update_timestamp('last_authored_at', message.created_at, ["user_id = ?", message.author_id]),
maybe_update_timestamp('visible_last_authored_at', message.created_at, ["user_id = ?", message.author_id])
]
updates << "workflow_state = CASE WHEN workflow_state = 'archived' THEN 'read' ELSE workflow_state END" if update_for_skips
conversation_participants.update_all(updates.join(", "), ["user_id IN (?)", skip_ids])
updated = false
if message.has_attachments?
self.has_attachments = true
conversation_participants.update_all({:has_attachments => true}, "NOT has_attachments")
updated = true
end
if message.has_media_objects?
self.has_media_objects = true
conversation_participants.update_all({:has_media_objects => true}, "NOT has_media_objects")
updated = true
self.conversation_participants.with_each_shard do |conversation_participants|
skip_ids = options[:skip_users].try(:map, &:id) || [message.author_id]
skip_ids = [0] if skip_ids.empty?
update_for_skips = options[:update_for_skips] != false
# make sure this jumps to the top of the inbox and is marked as unread for anyone who's subscribed
cp_conditions = sanitize_sql([
"cp.conversation_id = ? AND cp.workflow_state <> 'unread' AND (cp.last_message_at IS NULL OR cp.subscribed) AND cp.user_id NOT IN (?)",
self.id,
skip_ids
])
if connection.adapter_name =~ /mysql/i
connection.execute <<-SQL
UPDATE users, conversation_participants cp
SET unread_conversations_count = unread_conversations_count + 1
WHERE users.id = cp.user_id AND #{cp_conditions}
SQL
else
User.update_all 'unread_conversations_count = unread_conversations_count + 1',
"id IN (SELECT user_id FROM conversation_participants cp WHERE #{cp_conditions})"
end
conversation_participants.update_all(
{:last_message_at => message.created_at, :workflow_state => 'unread'},
["(last_message_at IS NULL OR subscribed) AND user_id NOT IN (?)", skip_ids]
)
# for the sender (or override(s)), we just update the timestamps (if
# needed). for last_authored_at, we ignore update_for_skips, since the
# column is only viewed by the other participants and doesn't care about
# what messages the author may have deleted
updates = [
maybe_update_timestamp('last_message_at', message.created_at, update_for_skips ? [] : ["last_message_at IS NOT NULL"]),
maybe_update_timestamp('last_authored_at', message.created_at, ["user_id = ?", message.author_id]),
maybe_update_timestamp('visible_last_authored_at', message.created_at, ["user_id = ?", message.author_id])
]
updates << "workflow_state = CASE WHEN workflow_state = 'archived' THEN 'read' ELSE workflow_state END" if update_for_skips
updates << "root_account_ids='#{options[:root_account_ids]}'" if options[:root_account_ids]
conversation_participants.update_all(updates.join(", "), ["user_id IN (?)", skip_ids])
if message.has_attachments?
self.has_attachments = true
conversation_participants.update_all({:has_attachments => true}, "NOT has_attachments")
updated = true
end
if message.has_media_objects?
self.has_media_objects = true
conversation_participants.update_all({:has_media_objects => true}, "NOT has_media_objects")
updated = true
end
end
self.save if updated
end
@ -400,7 +429,7 @@ class Conversation < ActiveRecord::Base
def reply_from(opts)
user = opts.delete(:user)
message = opts.delete(:text).to_s.strip
user = nil unless user && self.participants.find_by_id(user.id)
user = nil unless user && self.conversation_participants.find_by_user_id(user.id)
if !user
raise "Only message participants may reply to messages"
elsif message.blank?
@ -431,7 +460,7 @@ class Conversation < ActiveRecord::Base
# if the participant list has changed, e.g. we merged user accounts
def regenerate_private_hash!(user_ids = nil)
return unless private?
self.private_hash = Conversation.private_hash_for(user_ids || self.participant_ids)
self.private_hash = Conversation.private_hash_for(user_ids || self.conversation_participants.map(&:user_id))
return unless private_hash_changed?
if existing = Conversation.find_by_private_hash(private_hash)
merge_into(existing)
@ -451,17 +480,76 @@ class Conversation < ActiveRecord::Base
def merge_into(other)
transaction do
new_participants = other.conversation_participants.inject({}){ |h,p| h[p.user_id] = p; h }
conversation_participants(true).each do |cp|
if new_cp = new_participants[cp.user_id]
new_cp.update_attribute(:workflow_state, cp.workflow_state) if cp.unread? || new_cp.archived?
cp.conversation_message_participants.update_all(["conversation_participant_id = ?", new_cp.id])
cp.destroy
else
cp.update_attribute(:conversation_id, other.id)
new_participants = other.conversation_participants.index_by(&:user_id)
ConversationParticipant.skip_callback(:destroy_conversation_message_participants) do
conversation_participants(true).each do |cp|
if new_cp = new_participants[cp.user_id]
new_cp.update_attribute(:workflow_state, cp.workflow_state) if cp.unread? || new_cp.archived?
# backcompat
cp.conversation_message_participants.update_all(["conversation_participant_id = ?", new_cp.id])
# remove the duplicate participant
cp.destroy
if cp.user.shard != self.shard
# remove the duplicate secondary CP on the user's shard
cp.user.shard.activate do
ConversationParticipant.delete_all(:user_id => cp.user_id, :conversation_id => self.id)
end
end
else
# keep the cp, with updated conversation, iff the source
# conversation shared a shard with the user OR the target
# conversation
if self.shard == other.shard || self.shard == cp.user.shard
cp.update_attribute(:conversation, other)
else
cp.destroy
end
# update the duplicate cp on the user's shard if it's a different
# shard
if cp.user.shard != self.shard
cp.user.shard.activate do
ConversationParticipant.update_all({:conversation_id => other.id},
:user_id => cp.user_id, :conversation_id => self.id)
end
end
# create a new duplicate cp on the target conversation's shard
# if neither the user nor source conversation were there
# already.
if self.shard != other.shard && cp.user.shard != other.shard
new_cp = cp.clone
new_cp.shard = other.shard
new_cp.conversation = other
new_cp.save!
end
end
end
end
conversation_messages.update_all(["conversation_id = ?", other.id])
if other.shard == self.shard
conversation_messages.update_all(["conversation_id = ?", other.id])
else
# move messages and participants over to new shard
conversation_messages.find_each do |message|
new_message = message.clone
new_message.conversation = other
new_message.shard = other.shard
new_message.save!
message.conversation_message_participants.find_each do |cmp|
new_cmp = cmp.clone
new_cmp.conversation_message = new_message
new_cmp.shard = other.shard
new_cmp.save!
end
end
self.shard.activate do
ConversationMessageParticipant.scoped(:joins => :conversation_message).delete_all(
:conversation_messages => { :conversation_id => self.id }
)
self.conversation_messages.scoped({}).delete_all
end
end
conversation_participants.reload # now empty ... need to make sure callbacks don't double-delete
other.conversation_participants(true).each do |cp|
cp.update_cached_data! :recalculate_count => true, :set_last_message_at => false, :regenerate_tags => false
@ -483,15 +571,33 @@ class Conversation < ActiveRecord::Base
# options, so we roll our own that does (plus we do it in one query so we
# don't load conversation_participants into memory)
def self.preload_participants(conversations)
user_map = User.find_by_sql(sanitize_sql([<<-SQL, conversations.map(&:id)])).group_by(&:conversation_id)
SELECT #{reflections[:participants].options[:select]}, conversation_id
FROM users, conversation_participants
WHERE users.id = conversation_participants.user_id
AND conversation_id IN (?)
ORDER BY #{reflections[:participants].options[:order]}
SQL
# clear the cached participants
conversations.each do |conversation|
send :add_preloaded_records_to_collection, [conversation], :participants, user_map[conversation.id.to_s]
conversation.instance_variable_set(:@participants, [])
end
shards = conversations.map(&:associated_shards).flatten.uniq
Shard.with_each_shard(shards) do
user_map = User.find(:all,
:select => "#{User::MESSAGEABLE_USER_COLUMN_SQL}, last_authored_at, NULL AS common_courses, NULL AS common_groups, conversation_id",
:joins => :all_conversations,
:conditions => ["conversation_id IN (?)", conversations.map(&:id)],
:order => 'last_authored_at IS NULL, last_authored_at DESC, LOWER(COALESCE(short_name, name))').group_by(&:conversation_id)
conversations.each do |conversation|
conversation.participants.concat(user_map[conversation.id.to_s] || [])
end
end
# post-sort in Ruby
if shards.length > 1
conversations.each do |conversation|
conversation.participants.sort! do |user1, user2|
result = (user1.last_authored_at ? 0 : 1) <=> (user2.last_authored_at ? 0 : 1)
result = -(user1.last_authored_at <=> user2.last_authored_at) if result == 0 && user1.last_authored_at
result = (user1.short_name.try(:downcase) || user1.name.downcase) <=> (user2.short_name.try(:downcase) || user2.name.downcase) if result == 0
result
end
end
end
end
@ -516,6 +622,10 @@ class Conversation < ActiveRecord::Base
end
memoize :current_context_strings
def associated_shards
[Shard.default]
end
protected
def maybe_update_timestamp(col, val, additional_conditions=[])

18
app/models/conversation_batch.rb
View File

@ -24,7 +24,7 @@ class ConversationBatch < ActiveRecord::Base
ModelCache.with_cache(:conversations => existing_conversations, :users => {:id => user_map}) do
recipient_ids.each_slice(chunk_size) do |ids|
ids.each do |id|
@conversations << conversation = user.initiate_conversation([id])
@conversations << conversation = user.initiate_conversation([user_map[id]])
message = conversation.add_message(root_conversation_message.clone,
:update_for_sender => false,
:tags => tags)
@ -66,11 +66,7 @@ class ConversationBatch < ActiveRecord::Base
attr_writer :user_map
def user_map
@user_map ||= Hash[
User.find_all_by_id(recipient_ids + [user_id]).map { |u|
[u.id, u]
}
]
@user_map ||= User.find_all_by_id(recipient_ids + [user_id]).index_by(&:id)
end
def recipient_ids
@ -108,16 +104,16 @@ class ConversationBatch < ActiveRecord::Base
state :error
end
def self.generate(root_message, recipient_ids, mode = :async, options = {})
def self.generate(root_message, recipients, mode = :async, options = {})
batch = new
batch.mode = mode
batch.root_conversation_message = root_message
batch.user_id = root_message.author_id
batch.recipient_ids = recipient_ids
batch.recipient_ids = recipients.map(&:id)
batch.tags = options[:tags]
if options[:user_map]
batch.user_map = options[:user_map].update(batch.user_id => batch.user)
end
user_map = recipients.index_by(&:id)
user_map[batch.user_id] = batch.user
batch.user_map = user_map
batch.save!
batch
end

68
app/models/conversation_message.rb
View File

@ -41,42 +41,44 @@ class ConversationMessage < ActiveRecord::Base
user_or_id = user_or_id.id if user_or_id.is_a?(User)
{:conditions => {:author_id => user_or_id}}
}
def self.preload_latest(conversation_participants, author_id=nil)
def self.preload_latest(conversation_participants, author=nil)
return unless conversation_participants.present?
base_conditions = sanitize_sql([
"conversation_id IN (?) AND conversation_participant_id in (?) AND NOT generated",
conversation_participants.map(&:conversation_id),
conversation_participants.map(&:id)
])
base_conditions << sanitize_sql([" AND author_id = ?", author_id]) if author_id
# limit it for non-postgres so we can reduce the amount of extra data we
# crunch in ruby (generally none, unless a conversation has multiple
# most-recent messages, i.e. same created_at)
unless connection.adapter_name == 'PostgreSQL'
base_conditions << <<-SQL
AND conversation_messages.created_at = (
SELECT MAX(created_at)
FROM conversation_messages cm2
JOIN conversation_message_participants cmp2 ON cm2.id = conversation_message_id
WHERE cm2.conversation_id = conversation_messages.conversation_id
AND #{base_conditions}
Shard.partition_by_shard(conversation_participants, lambda { |cp| cp.conversation_id }) do |shard_participants|
base_conditions = "(#{shard_participants.map { |cp|
"(conversation_id=#{cp.conversation_id} AND user_id=#{cp.user_id})" }.join(" OR ")
}) AND NOT generated"
base_conditions << sanitize_sql([" AND author_id = ?", author.id]) if author
# limit it for non-postgres so we can reduce the amount of extra data we
# crunch in ruby (generally none, unless a conversation has multiple
# most-recent messages, i.e. same created_at)
unless connection.adapter_name == 'PostgreSQL'
base_conditions << <<-SQL
AND conversation_messages.created_at = (
SELECT MAX(created_at)
FROM conversation_messages cm2
JOIN conversation_message_participants cmp2 ON cm2.id = conversation_message_id
WHERE cm2.conversation_id = conversation_messages.conversation_id
AND #{base_conditions}
)
SQL
end
ActiveRecord::Base::ConnectionSpecification.with_environment(:slave) do
ret = distinct_on(['conversation_id', 'user_id'],
:select => "conversation_messages.*, conversation_participant_id, conversation_message_participants.user_id, conversation_message_participants.tags",
:joins => 'JOIN conversation_message_participants ON conversation_messages.id = conversation_message_id',
:conditions => base_conditions,
:order => 'conversation_id DESC, user_id DESC, created_at DESC'
)
SQL
end
ActiveRecord::Base::ConnectionSpecification.with_environment(:slave) do
ret = distinct_on('conversation_participant_id',
:select => "conversation_messages.*, conversation_participant_id, conversation_message_participants.tags",
:joins => 'JOIN conversation_message_participants ON conversation_messages.id = conversation_message_id',
:conditions => base_conditions,
:order => 'conversation_participant_id, created_at DESC'
)
map = Hash[ret.map{ |m| [m.conversation_participant_id.to_i, m]}]
if author_id
conversation_participants.each{ |cp| cp.last_authored_message = map[cp.id] }
else
conversation_participants.each{ |cp| cp.last_message = map[cp.id] }
map = Hash[ret.map{ |m| [[m.conversation_id, m.user_id.to_i], m]}]
backmap = Hash[ret.map{ |m| [m.conversation_participant_id.to_i, m]}]
if author
shard_participants.each{ |cp| cp.last_authored_message = map[[cp.conversation_id, cp.user_id]] || backmap[cp.id] }
else
shard_participants.each{ |cp| cp.last_message = map[[cp.conversation_id, cp.user_id]] || backmap[cp.id] }
end
end
end
end

2
app/models/conversation_message_participant.rb
View File

@ -20,6 +20,8 @@ class ConversationMessageParticipant < ActiveRecord::Base
include SimpleTags
belongs_to :conversation_message
belongs_to :user
# deprecated
belongs_to :conversation_participant
delegate :author, :author_id, :generated, :body, :to => :conversation_message

112
app/models/conversation_participant.rb
View File

@ -24,13 +24,9 @@ class ConversationParticipant < ActiveRecord::Base
belongs_to :conversation
belongs_to :user
has_many :conversation_message_participants, :dependent => :delete_all
has_many :messages, :source => :conversation_message,
:through => :conversation_message_participants,
:select => "conversation_messages.*, conversation_message_participants.tags",
:order => "created_at DESC, id DESC",
:conditions => 'conversation_id = #{conversation_id}'
# conditions are redundant, but they let us use the best index
# deprecated
has_many :conversation_message_participants
after_destroy :destroy_conversation_message_participants
named_scope :visible, :conditions => "last_message_at IS NOT NULL"
named_scope :default, :conditions => "workflow_state IN ('read', 'unread')"
@ -56,10 +52,12 @@ class ConversationParticipant < ActiveRecord::Base
#
# we're also counting on conversations being in the join
own_root_account_ids = user.associated_root_accounts.select{ |a| a.grants_right?(user, :become_user) }.map(&:id)
own_root_account_ids = Shard.default.activate do
user.associated_root_accounts.select{ |a| a.grants_right?(user, :become_user) }.map(&:id)
end
id_string = "[" + own_root_account_ids.sort.join("][") + "]"
root_account_id_matcher = "'%[' || REPLACE(root_account_ids, ',', ']%[') || ']%'"
{:conditions => ["conversations.root_account_ids <> '' AND " + like_condition('?', root_account_id_matcher, false), id_string]}
root_account_id_matcher = "'%[' || REPLACE(conversation_participants.root_account_ids, ',', ']%[') || ']%'"
{:conditions => ["conversation_participants.root_account_ids <> '' AND " + like_condition('?', root_account_id_matcher, false), id_string]}
}
tagged_scope_handler(/\Auser_(\d+)\z/) do |tags, options|
@ -94,7 +92,7 @@ class ConversationParticipant < ActiveRecord::Base
before_update :update_unread_count_for_update
before_destroy :update_unread_count_for_destroy
attr_accessible :subscribed, :starred, :workflow_state
attr_accessible :subscribed, :starred, :workflow_state, :user
validates_inclusion_of :label, :in => ['starred'], :allow_nil => true
@ -117,6 +115,26 @@ class ConversationParticipant < ActiveRecord::Base
}.with_indifferent_access
end
def messages
self.conversation.shard.activate do
if self.conversation.shard == self.shard
# use a slightly more forgiving backcompat query (since the migration may not have
# fully filled in user_id yet)
ConversationMessage.scoped(:shard => self.conversation.shard,
:select => "conversation_messages.*, conversation_message_participants.tags",
:joins => :conversation_message_participants,
:conditions => ["conversation_id=? AND (user_id=? OR (conversation_participant_id=? AND user_id IS NULL))", self.conversation_id, self.user_id, self.id],
:order => "created_at DESC, id DESC")
else
ConversationMessage.scoped(:shard => self.conversation.shard,
:select => "conversation_messages.*, conversation_message_participants.tags",
:joins => :conversation_message_participants,
:conditions => ["conversation_id=? AND user_id=?", self.conversation_id, self.user_id],
:order => "created_at DESC, id DESC")
end
end
end
def participants(options = {})
options = {
:include_participant_contexts => false,
@ -158,8 +176,8 @@ class ConversationParticipant < ActiveRecord::Base
latest && latest.author_id == user_id
end
def add_participants(user_ids, options={})
conversation.add_participants(user, user_ids, options)
def add_participants(users, options={})
conversation.add_participants(user, users, options)
end
def add_message(body_or_obj, options={})
@ -167,16 +185,24 @@ class ConversationParticipant < ActiveRecord::Base
end
def remove_messages(*to_delete)
if to_delete == [:all]
messages.clear
else
messages.delete(*to_delete)
# if the only messages left are generated ones, e.g. "added
# bob to the conversation", delete those too
messages.clear if messages.all?(&:generated?)
self.conversation.shard.activate do
scope = ConversationMessageParticipant.scoped(
:joins => :conversation_message,
:conditions => {'conversation_messages.conversation_id' => self.conversation_id,
:user_id => self.user_id})
if to_delete == [:all]
scope.delete_all
else
scope.delete_all(:conversation_message_id => to_delete.map(&:id))
# if the only messages left are generated ones, e.g. "added
# bob to the conversation", delete those too
return remove_messages(:all) if messages.count(:all, :conditions => {:generated => false}) == 0
end
end
unless @destroyed
update_cached_data
save
end
update_cached_data
save
end
def update_attributes(hash)
@ -217,7 +243,7 @@ class ConversationParticipant < ActiveRecord::Base
end
def one_on_one?
conversation.participants.size == 2 && private?
conversation.conversation_participants.size == 2 && private?
end
def other_participants(participants=conversation.participants)
@ -294,12 +320,24 @@ class ConversationParticipant < ActiveRecord::Base
def move_to_user(new_user)
self.class.send :with_exclusive_scope do
conversation.conversation_messages.update_all(["author_id = ?", new_user.id], ["author_id = ?", user_id])
if existing = conversation.conversation_participants.find_by_user_id(new_user.id)
existing.update_attribute(:workflow_state, workflow_state) if unread? || existing.archived?
destroy
else
update_attribute :user_id, new_user.id
conversation.shard.activate do
old_shard = self.user.shard
conversation.conversation_messages.update_all(["author_id = ?", new_user.id], ["author_id = ?", user_id])
if existing = conversation.conversation_participants.find_by_user_id(new_user.id)
existing.update_attribute(:workflow_state, workflow_state) if unread? || existing.archived?
destroy
else
ConversationMessageParticipant.scoped(:joins => :conversation_message).update_all({:user_id => new_user.id},
'conversation_messages.conversation_id' => self.conversation_id, :user_id => self.user_id)
update_attribute :user, new_user
existing = self
end
# replicate ConversationParticipant record to the new user's shard
if old_shard != new_user.shard && new_user.shard != conversation.shard
new_cp = existing.clone
new_cp.shard = new_user.shard
new_cp.save!
end
end
conversation.regenerate_private_hash! if private?
end
@ -315,20 +353,18 @@ class ConversationParticipant < ActiveRecord::Base
@last_authored_message ||= messages.human.by_user(user_id).first if visible_last_authored_at
end
def self.preload_latest_messages(conversations, author_id)
def self.preload_latest_messages(conversations, author)
# preload last_message
ConversationMessage.preload_latest conversations.select(&:last_message_at)
# preload last_authored_message
ConversationMessage.preload_latest conversations.select(&:visible_last_authored_at), author_id
ConversationMessage.preload_latest conversations.select(&:visible_last_authored_at), author
end
def self.conversation_ids
scope = current_scoped_methods && current_scoped_methods[:find]
raise "conversation_ids needs to be scoped to a user" unless scope && scope[:conditions] =~ /user_id = \d+/
scope[:order] ||= "last_message_at DESC"
# need to join on conversations in case we use this w/ scopes like for_masquerading_user
connection.select_all("SELECT conversation_id FROM conversations, conversation_participants WHERE #{scope[:conditions]} AND conversations.id = conversation_participants.conversation_id ORDER BY #{scope[:order]}").
map{ |row| row['conversation_id'].to_i }
order = 'last_message_at DESC' unless scope[:order]
self.find(:all, :select => 'conversation_id', :order => order).map(&:conversation_id)
end
protected
@ -337,6 +373,12 @@ class ConversationParticipant < ActiveRecord::Base
end
private
def destroy_conversation_message_participants
@destroyed = true
remove_messages(:all) if self.conversation_id
end
def update_unread_count(direction=:up, user_id=self.user_id)
User.update_all "unread_conversations_count = unread_conversations_count #{direction == :up ? '+' : '-'} 1, updated_at = '#{Time.now.to_s(:db)}'",
:id => user_id

14
app/models/stream_item.rb
View File

@ -49,11 +49,15 @@ class StreamItem < ActiveRecord::Base
when 'Submission'
data['body'] = nil
end
['users', 'participants'].each do |key|
next unless data.has_key?(key)
users = data.delete(key)
if data.has_key?('users')
users = data.delete('users')
users = users.map { |user| reconstitute_ar_object('User', user) }
res.send(key.to_sym).target = users
res.users.target = users
end
if data.has_key?('participants')
users = data.delete('participants')
users = users.map { |user| reconstitute_ar_object('User', user) }
res.instance_variable_set(:@participants, users)
end
res.instance_variable_set(:@attributes, data)
@ -85,7 +89,7 @@ class StreamItem < ActiveRecord::Base
def prepare_conversation(conversation)
res = conversation.attributes.slice('id', 'has_attachments')
res['private'] = conversation.private?
res['participant_count'] = conversation.participants.size
res['participant_count'] = conversation.conversation_participants.size
# arbitrary limit. would be nice to say "John, Jane, Michael, and 6
# others." if there's too many recipients, where those listed are the N
# most active posters in the conversation, but we'll just leave it at "9

12
app/models/submission.rb
View File

@ -855,14 +855,14 @@ class Submission < ActiveRecord::Base
end
def conversation_groups
participating_instructors.map{ |i| [user_id, i.id] }
participating_instructors.map{ |i| [user, i] }
end
def conversation_message_data
latest = visible_submission_comments.scoped(:conditions => ["author_id IN (?)", possible_participants_ids]).last or return
{
:created_at => latest.created_at,
:author_id => latest.author_id,
:author => latest.author,
:body => latest.comment
}
end
@ -896,7 +896,7 @@ class Submission < ActiveRecord::Base
# updating the conversation.
#
# ==== Overrides
# * <tt>:skip_ids</tt> - Gets passed through to <tt>Conversation</tt>.<tt>update_all_for_asset</tt>.
# * <tt>:skip_users</tt> - Gets passed through to <tt>Conversation</tt>.<tt>update_all_for_asset</tt>.
# nil by default, which means mark-as-unread for
# everyone but the author.
def create_or_update_conversations!(trigger, overrides={})
@ -905,10 +905,10 @@ class Submission < ActiveRecord::Base
when :create
options[:update_participants] = true
options[:update_for_skips] = false
options[:skip_ids] = overrides[:skip_ids] || [conversation_message_data[:author_id]] # don't mark-as-unread for the author
options[:skip_users] = overrides[:skip_users] || [conversation_message_data[:author]] # don't mark-as-unread for the author
participating_instructors.each do |t|
# Check their settings and add to :skip_ids if set to suppress.
options[:skip_ids] << t.id if t.preferences[:no_submission_comments_inbox] == true
# Check their settings and add to :skip_users if set to suppress.
options[:skip_users] << t if t.preferences[:no_submission_comments_inbox] == true
end
when :destroy
options[:delete_all] = visible_submission_comments.empty?

9
app/models/user.rb
View File

@ -1945,11 +1945,10 @@ class User < ActiveRecord::Base
accounts.size == 0 || accounts.any?{ |a| a.settings[:enable_eportfolios] != false }
end
def initiate_conversation(user_ids, private = nil)
this = user_ids.first.is_a?(User) ? self : self.id
user_ids = ([this] + user_ids).uniq
private = user_ids.size <= 2 if private.nil?
Conversation.initiate(user_ids, private).conversation_participants.find_by_user_id(self.id)
def initiate_conversation(users, private = nil)
users = ([self] + users).uniq
private = users.size <= 2 if private.nil?
Conversation.initiate(users, private).conversation_participants.find_by_user_id(self.id)
end
def messageable_user_clause

14
db/migrate/20130103191206_add_user_id_to_conversation_message_participants.rb Normal file
View File

@ -0,0 +1,14 @@
class AddUserIdToConversationMessageParticipants < ActiveRecord::Migration
tag :predeploy
self.transactional = false
def self.up
add_column :conversation_message_participants, :user_id, :integer, :limit => 8
add_index :conversation_message_participants, [:user_id, :conversation_message_id], :name => "index_conversation_message_participants_on_uid_and_message_id", :unique => true, :concurrently => true
end
def self.down
remove_index :conversation_message_participants, [:user_id, :conversation_message_id]
remove_column :conversation_message_participants, :user_id
end
end

7
db/migrate/20130114214157_populate_conversation_message_participant_user_ids.rb Normal file
View File

@ -0,0 +1,7 @@
class PopulateConversationMessageParticipantUserIds < ActiveRecord::Migration
tag :postdeploy
def self.up
DataFixup::PopulateConversationMessageParticipantUserIds.send_later_if_production(:run)
end
end

11
db/migrate/20130114214749_add_root_account_ids_to_conversation_participant.rb Normal file
View File

@ -0,0 +1,11 @@
class AddRootAccountIdsToConversationParticipant < ActiveRecord::Migration
tag :predeploy
def self.up
add_column :conversation_participants, :root_account_ids, :text
end
def self.down
remove_column :conversation_participants, :root_account_ids
end
end

7
db/migrate/20130114215024_populate_conversation_participant_root_account_ids.rb Normal file
View File

@ -0,0 +1,7 @@
class PopulateConversationParticipantRootAccountIds < ActiveRecord::Migration
tag :postdeploy
def self.up
DataFixup::PopulateConversationParticipantRootAccountIds.send_later_if_production(:run)
end
end

14
db/migrate/20130118162201_make_conversation_participants_index_unique.rb Normal file
View File

@ -0,0 +1,14 @@
class MakeConversationParticipantsIndexUnique < ActiveRecord::Migration
self.transactional = false
tag :predeploy
def self.up
add_index :conversation_participants, [:conversation_id, :user_id], :unique => true, :concurrently => true
remove_index :conversation_participants, [:conversation_id]
end
def self.down
add_index :conversation_participants, [:conversation_id]
remove_index :conversation_participants, [:conversation_id, :user_id]
end
end

10
lib/data_fixup/populate_conversation_message_participant_user_ids.rb Normal file
View File

@ -0,0 +1,10 @@
module DataFixup::PopulateConversationMessageParticipantUserIds
def self.run
target = ConversationMessageParticipant.connection.adapter_name == 'MySQL' ? 'conversation_message_participants.user_id' : 'user_id'
ConversationMessageParticipant.scoped(:conditions => {:user_id => nil}).find_ids_in_ranges do |min, max|
scope = ConversationMessageParticipant.scoped(:joins => :conversation_participant)
scope.update_all("#{target}=conversation_participants.user_id",
["conversation_message_participants.id>=? AND conversation_message_participants.id <=?", min, max])
end
end
end

11
lib/data_fixup/populate_conversation_participant_root_account_ids.rb Normal file
View File

@ -0,0 +1,11 @@
module DataFixup::PopulateConversationParticipantRootAccountIds
def self.run
target = ConversationParticipant.connection.adapter_name == 'MySQL' ? 'conversation_participants.root_account_ids' : 'root_account_ids'
scope = ConversationParticipant.scoped(:conditions => {:root_account_ids => nil})
scope = scope.scoped(:joins => :conversation, :conditions => "conversations.root_account_ids IS NOT NULL")
scope.find_ids_in_ranges do |min, max|
ConversationParticipant.update_all("#{target}=conversations.root_account_ids",
["conversation_participants.id>=? AND conversation_participants.id <=?", min, max])
end
end
end

6
lib/mutable.rb
View File

@ -77,12 +77,12 @@ module Mutable
outstanding = submissions.map{ |submission|
comments = submission.hidden_submission_comments.all
next if comments.empty?
[submission, comments.map(&:author_id).uniq.size == 1 ? [comments.last.author_id] : []]
[submission, comments.map(&:author_id).uniq.size == 1 ? [comments.last.author] : []]
}.compact
SubmissionComment.update_all({ :hidden => false }, { :hidden => true, :submission_id => submissions.map(&:id) })
Submission.send(:preload_associations, outstanding.map(&:first), :visible_submission_comments)
outstanding.each do |submission, skip_ids|
submission.create_or_update_conversations!(:create, :skip_ids => skip_ids)
outstanding.each do |submission, skip_users|
submission.create_or_update_conversations!(:create, :skip_users => skip_users)
end
end
end

4
lib/stubs/shard.rb
View File

@ -70,6 +70,10 @@ class Shard
end
ActiveRecord::Base.class_eval do
class << self
VALID_FIND_OPTIONS << :shard
end
def shard
Shard.default
end

8
spec/apis/v1/conversations_api_spec.rb
View File

@ -201,7 +201,7 @@ describe ConversationsController, :type => :integration do
@c1 = conversation(@bob)
@c2 = conversation(@bob, @billy)
@c2.conversation.add_message(@bob, 'ohai')
@c2.remove_messages([@message]) # delete my original message
@c2.remove_messages(@message) # delete my original message
@c3 = conversation(@jane, :workflow_state => 'archived')
json = api_call(:get, "/api/v1/conversations.json?scope=sent",
@ -929,9 +929,9 @@ describe ConversationsController, :type => :integration do
context "batches" do
it "should return all in-progress batches" do
batch1 = ConversationBatch.generate(Conversation.build_message(@me, "hi all"), [@bob.id, @billy.id], :async)
batch2 = ConversationBatch.generate(Conversation.build_message(@me, "ohai"), [@bob.id, @billy.id], :sync)
batch3 = ConversationBatch.generate(Conversation.build_message(@bob, "sup"), [@me.id, @billy.id], :async)
batch1 = ConversationBatch.generate(Conversation.build_message(@me, "hi all"), [@bob, @billy], :async)
batch2 = ConversationBatch.generate(Conversation.build_message(@me, "ohai"), [@bob, @billy], :sync)
batch3 = ConversationBatch.generate(Conversation.build_message(@bob, "sup"), [@me, @billy], :async)
json = api_call(:get, "/api/v1/conversations/batches",
:controller => 'conversations',

2
spec/apis/v1/search_api_spec.rb
View File

@ -108,7 +108,7 @@ describe SearchController, :type => :integration do
{ :controller => 'search', :action => 'recipients', :format => 'json', :user_id => other.id.to_s })
json.should == []
# now they have a conversation in common
c = Conversation.initiate([@user.id, other.id], true)
c = Conversation.initiate([@user, other], true)
json = api_call(:get, "/api/v1/search/recipients?user_id=#{other.id}",
{ :controller => 'search', :action => 'recipients', :format => 'json', :user_id => other.id.to_s })
json.should == []

2
spec/apis/v1/stream_items_api_spec.rb
View File

@ -220,7 +220,7 @@ describe UsersController, :type => :integration do
it "should format Conversation" do
@sender = User.create!(:name => 'sender')
@conversation = Conversation.initiate([@user.id, @sender.id], false)
@conversation = Conversation.initiate([@user, @sender], false)
@conversation.add_message(@sender, "hello")
@message = @conversation.conversation_messages.last
json = api_call(:get, "/api/v1/users/activity_stream.json",

12
spec/controllers/conversations_controller_spec.rb
View File

@ -22,14 +22,14 @@ describe ConversationsController do
def conversation(opts = {})
num_other_users = opts[:num_other_users] || 1
course = opts[:course] || @course
user_ids = num_other_users.times.map{
users = num_other_users.times.map{
u = User.create
enrollment = course.enroll_student(u)
enrollment.workflow_state = 'active'
enrollment.save
u.id
u
}
@conversation = @user.initiate_conversation(user_ids)
@conversation = @user.initiate_conversation(users)
@conversation.add_message(opts[:message] || 'test')
@conversation
end
@ -134,9 +134,9 @@ describe ConversationsController do
a = Account.default
@student = user_with_pseudonym(:active_all => true)
course_with_student(:active_all => true, :account => a, :user => @student)
@student.initiate_conversation([user.id]).add_message('test1', :root_account_id => a.id)
@student.initiate_conversation([user.id]).add_message('test2') # no root account, so teacher can't see it
@student.initiate_conversation([user]).add_message('test1', :root_account_id => a.id)
@student.initiate_conversation([user]).add_message('test2') # no root account, so teacher can't see it
course_with_teacher_logged_in(:active_all => true, :account => a)
a.add_user(@user)
session[:become_user_id] = @student.id

2
spec/integration/users_controller_spec.rb
View File

@ -38,7 +38,7 @@ describe UsersController do
get user_student_teacher_activity_url(@teacher, @e1.user)
Nokogiri::HTML(response.body).at_css('table.report tr:first td:nth(2)').text.should match(/never/)
@conversation = Conversation.initiate([@e1.user_id, @teacher.id], false)
@conversation = Conversation.initiate([@e1.user, @teacher], false)
@conversation.add_message(@teacher, "hello")
get user_student_teacher_activity_url(@teacher, @e1.user)

8
spec/lib/user_merge_spec.rb
View File

@ -185,21 +185,21 @@ describe UserMerge do
end
it "should move conversations to the new user" do
c1 = user1.initiate_conversation([user.id, user.id]) # group conversation
c1 = user1.initiate_conversation([user, user]) # group conversation
c1.add_message("hello")
c1.update_attribute(:workflow_state, 'unread')
c2 = user1.initiate_conversation([user.id]) # private conversation
c2 = user1.initiate_conversation([user]) # private conversation
c2.add_message("hello")
c2.update_attribute(:workflow_state, 'unread')
old_private_hash = c2.conversation.private_hash
UserMerge.from(user1).into(user2)
c1.reload.user_id.should eql user2.id
c1.conversation.participant_ids.should_not include(user1.id)
c1.conversation.participants.should_not include(user1)
user1.reload.unread_conversations_count.should eql 0
c2.reload.user_id.should eql user2.id
c2.conversation.participant_ids.should_not include(user1.id)
c2.conversation.participants.should_not include(user1)
c2.conversation.private_hash.should_not eql old_private_hash
user2.reload.unread_conversations_count.should eql 2
end

4
spec/messages/added_to_conversation.email.erb_spec.rb
View File

@ -25,9 +25,9 @@ describe 'added_to_conversation.email' do
student1 = student_in_course.user
student2 = student_in_course.user
student3 = student_in_course.user
conversation = @teacher.initiate_conversation([student1.id, student2.id])
conversation = @teacher.initiate_conversation([student1, student2])
conversation.add_message("some message")
event = conversation.add_participants([student3.id])
event = conversation.add_participants([student3])
generate_message(:added_to_conversation, :email, event)
end
end

4
spec/messages/added_to_conversation.facebook.erb_spec.rb
View File

@ -25,9 +25,9 @@ describe 'added_to_conversation.facebook' do
student1 = student_in_course.user
student2 = student_in_course.user
student3 = student_in_course.user
conversation = @teacher.initiate_conversation([student1.id, student2.id])
conversation = @teacher.initiate_conversation([student1, student2])
conversation.add_message("some message")
event = conversation.add_participants([student3.id])
event = conversation.add_participants([student3])
generate_message(:added_to_conversation, :facebook, event)
end
end

4
spec/messages/added_to_conversation.sms.erb_spec.rb
View File

@ -25,9 +25,9 @@ describe 'added_to_conversation.sms' do
student1 = student_in_course.user
student2 = student_in_course.user
student3 = student_in_course.user
conversation = @teacher.initiate_conversation([student1.id, student2.id])
conversation = @teacher.initiate_conversation([student1, student2])
conversation.add_message("some message")
event = conversation.add_participants([student3.id])
event = conversation.add_participants([student3])
generate_message(:added_to_conversation, :sms, event)
end
end

4
spec/messages/added_to_conversation.summary.erb_spec.rb
View File

@ -25,9 +25,9 @@ describe 'added_to_conversation.summary' do
student1 = student_in_course.user
student2 = student_in_course.user
student3 = student_in_course.user
conversation = @teacher.initiate_conversation([student1.id, student2.id])
conversation = @teacher.initiate_conversation([student1, student2])
conversation.add_message("some message")
event = conversation.add_participants([student3.id])
event = conversation.add_participants([student3])
generate_message(:added_to_conversation, :summary, event)
end
end

4
spec/messages/added_to_conversation.twitter.erb_spec.rb
View File

@ -25,9 +25,9 @@ describe 'added_to_conversation.twitter' do
student1 = student_in_course.user
student2 = student_in_course.user
student3 = student_in_course.user
conversation = @teacher.initiate_conversation([student1.id, student2.id])
conversation = @teacher.initiate_conversation([student1, student2])
conversation.add_message("some message")
event = conversation.add_participants([student3.id])
event = conversation.add_participants([student3])
generate_message(:added_to_conversation, :twitter, event)
end
end

2
spec/messages/conversation_message.email.erb_spec.rb
View File

@ -23,7 +23,7 @@ describe 'conversation_message.email' do
it "should render" do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
message = conversation.add_message("some message")
generate_message(:conversation_message, :email, message)
end

2
spec/messages/conversation_message.facebook.erb_spec.rb
View File

@ -23,7 +23,7 @@ describe 'conversation_message.facebook' do
it "should render" do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
message = conversation.add_message("some message")
generate_message(:conversation_message, :facebook, message)
end

2
spec/messages/conversation_message.sms.erb_spec.rb
View File

@ -23,7 +23,7 @@ describe 'conversation_message.sms' do
it "should render" do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
message = conversation.add_message("some message")
generate_message(:conversation_message, :sms, message)
end

2
spec/messages/conversation_message.summary.erb_spec.rb
View File

@ -23,7 +23,7 @@ describe 'conversation_message.summary' do
it "should render" do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
message = conversation.add_message("some message")
generate_message(:conversation_message, :summary, message)
end

2
spec/messages/conversation_message.twitter.erb_spec.rb
View File

@ -23,7 +23,7 @@ describe 'conversation_message.twitter' do
it "should render" do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
message = conversation.add_message("some message")
generate_message(:conversation_message, :twitter, message)
end

12
spec/migrations/fix_user_conversations_counts_for_all_spec.rb
View File

@ -25,7 +25,7 @@ describe 'FixUserConversationsCountsForAll' do
# Setup user with correct unread_conversations_count (2 unread convos)
u1 = user
2.times do
c = u1.initiate_conversation([u1.id], false)
c = u1.initiate_conversation([u1], false)
c.add_message('Hello')
c.add_message('Hello again')
c.update_attribute(:workflow_state, 'unread')
@ -35,7 +35,7 @@ describe 'FixUserConversationsCountsForAll' do
# Setup user with wrong unread_conversations_count (negative)
u2 = user
1.times do
c = u2.initiate_conversation([u2.id], false)
c = u2.initiate_conversation([u2], false)
c.add_message('Hello')
c.add_message('Hello again')
c.update_attribute(:workflow_state, 'unread')
@ -46,7 +46,7 @@ describe 'FixUserConversationsCountsForAll' do
# Setup user with wrong unread_conversations_count (too many)
u3 = user
3.times do
c = u3.initiate_conversation([u3.id], false)
c = u3.initiate_conversation([u3], false)
c.add_message('Hello')
c.add_message('Hello again')
c.update_attribute(:workflow_state, 'unread')
@ -65,13 +65,13 @@ describe 'FixUserConversationsCountsForAll' do
# Setup user with some deleted conversations
u1 = user
2.times do
c = u1.initiate_conversation([u1.id], false)
c = u1.initiate_conversation([u1], false)
c.add_message('Hello')
c.add_message('Hello again')
c.update_attribute(:workflow_state, 'unread')
end
1.times do
c = u1.initiate_conversation([u1.id], false)
c = u1.initiate_conversation([u1], false)
c.add_message('Deleted myself')
c.add_message('Empty yo')
c.update_attribute(:workflow_state, 'unread')
@ -82,7 +82,7 @@ describe 'FixUserConversationsCountsForAll' do
# Setup user with only deleted conversations (should have count 0)
u2 = user
3.times do
c = u2.initiate_conversation([u2.id], false)
c = u2.initiate_conversation([u2], false)
c.add_message('Hello')
c.add_message('Hello again')
c.update_attribute(:workflow_state, 'unread')

6
spec/migrations/fix_user_merge_conversations2_spec.rb
View File

@ -30,20 +30,20 @@ describe 'FixUserMergeConversations2' do
# set up borked conversation that is partially merged...
# conversation deleted, cp's and cmps orphaned,
# and cm on the target conversation
borked = Conversation.initiate([u1.id, u2.id], true)
borked = Conversation.initiate([u1, u2], true)
borked_cps = borked.conversation_participants.all
borked_cmps = borked_cps.map(&:conversation_message_participants).flatten
m1 = borked.add_message(u1, "test")
Conversation.delete_all(:id => borked.id) # bypass callbacks
correct = Conversation.initiate([u1.id, u2.id], true)
correct = Conversation.initiate([u1, u2], true)
m2 = correct.add_message(u1, "test2")
correct.conversation_participants.each { |cp| cp.update_attribute :workflow_state, 'archived'}
# put it the message on the correct conversation
m1.update_attribute :conversation_id, correct.id
unrelated = Conversation.initiate([u1.id, u3.id], true)
unrelated = Conversation.initiate([u1, u3], true)
unrelated.add_message(u1, "test3")
FixUserMergeConversations2.up

9
spec/migrations/fix_user_merge_conversations_spec.rb
View File

@ -22,19 +22,20 @@ require 'db/migrate/20120216163427_fix_user_merge_conversations.rb'
describe 'FixUserMergeConversations' do
describe "up" do
it "should work" do
pending "no longer possible to create bad data due to db constraint"
u1 = user
u2 = user
u3 = user
c1 = Conversation.initiate([u1.id, u2.id], true)
c1.participants << u1
c1 = Conversation.initiate([u1, u2], true)
c1.conversation_participants.create!(:user => u1)
c1.update_attribute(:private_hash, 'no longer valid')
c1.conversation_participants.size.should eql 3
c2 = Conversation.initiate([u1.id, u3.id], true)
c2 = Conversation.initiate([u1, u3], true)
c2.update_attribute(:private_hash, 'well this is clearly wrong')
c3 = Conversation.initiate([u1.id, u3.id], true)
c3 = Conversation.initiate([u1, u3], true)
FixUserMergeConversations.up

14
spec/migrations/populate_conversation_message_properties.rb
View File

@ -25,20 +25,20 @@ describe 'DataFixup::PopulateConversationMessageProperties' do
student_in_course
u = @user
c1 = u.initiate_conversation([User.create.id])
c1 = u.initiate_conversation([User.create])
m1 = c1.add_message("no attachment")
c2 = u.initiate_conversation([User.create.id])
c2 = u.initiate_conversation([User.create])
a = attachment_model(:context => u, :folder => u.conversation_attachments_folder)
m2 = c2.add_message("attachment!", :attachment_ids => [a.id])
c3 = u.initiate_conversation([User.create.id])
c3 = u.initiate_conversation([User.create])
m3 = c3.add_message("forwarded attachment!", :forwarded_message_ids => [m2.id])
c4 = u.initiate_conversation([User.create.id])
c4 = u.initiate_conversation([User.create])
m4 = c4.add_message("doubly forwarded attachment!", :forwarded_message_ids => [m3.id])
c5 = u.initiate_conversation([User.create.id])
c5 = u.initiate_conversation([User.create])
mc = MediaObject.new
mc.media_type = 'audio'
mc.media_id = 'asdf'
@ -46,10 +46,10 @@ describe 'DataFixup::PopulateConversationMessageProperties' do
mc.save
m5 = c5.add_message("media_comment!", :media_comment => mc)
c6 = u.initiate_conversation([User.create.id])
c6 = u.initiate_conversation([User.create])
m6 = c6.add_message("forwarded media_comment!", :forwarded_message_ids => [m5.id])
c7 = u.initiate_conversation([User.create.id])
c7 = u.initiate_conversation([User.create])
m7 = c7.add_message("doubly forwarded media_comment!", :forwarded_message_ids => [m6.id])
ConversationParticipant.update_all("has_attachments = (id = #{c2.id}), has_media_objects = (id = #{c5.id})")

10
spec/migrations/populate_conversation_root_account_ids_spec.rb
View File

@ -29,7 +29,7 @@ describe 'PopulateConversationRootAccountIds' do
u1 = user
a1a = Account.default
a1b = Account.create
cn1 = Conversation.initiate([u.id, u1.id], true)
cn1 = Conversation.initiate([u, u1], true)
cn1.add_message(u, "test1").update_attribute(:context, a1a)
cn1.add_message(u, "test2").update_attribute(:context, a1a)
cn1.add_message(u, "test3").update_attribute(:context, a1b)
@ -39,7 +39,7 @@ describe 'PopulateConversationRootAccountIds' do
u2 = user
a2 = Account.create
c2 = course(:account => a2)
cn2 = Conversation.initiate([u.id, u2.id], true)
cn2 = Conversation.initiate([u, u2], true)
cn2.add_message(u, "test")
Conversation.connection.execute "INSERT INTO context_messages(context_id, context_type) VALUES(#{c2.id}, 'Course')"
cn2.conversation_messages.update_all("context_message_id = (SELECT id FROM context_messages ORDER BY id DESC LIMIT 1)")
@ -50,7 +50,7 @@ describe 'PopulateConversationRootAccountIds' do
a3 = Account.create
g3 = group(:group_context => a3)
cn3 = Conversation.initiate([u.id, u3.id], true)
cn3 = Conversation.initiate([u, u3], true)
cn3.add_message(u, "test")
Conversation.connection.execute "INSERT INTO context_messages(context_id, context_type) VALUES(#{g3.id}, 'Group')"
cn3.conversation_messages.update_all("context_message_id = (SELECT id FROM context_messages ORDER BY id DESC LIMIT 1)")
@ -63,13 +63,13 @@ describe 'PopulateConversationRootAccountIds' do
student_in_course(:user => u4, :course => c4, :active_all => true)
as4 = c4.assignments.create
s4 = as4.submit_homework(u4, :submission_type => "online_text_entry", :body => "")
cn4 = Conversation.initiate([u.id, u4.id], true)
cn4 = Conversation.initiate([u, u4], true)
cn4.add_message(u, '').update_attribute(:asset, s4)
cn4.root_account_ids.should eql []
# no root account info available
u5 = user
cn5 = Conversation.initiate([u.id, u5.id], true)
cn5 = Conversation.initiate([u, u5], true)
cn5.add_message(u, "test")
PopulateConversationRootAccountIds.up

2
spec/migrations/remove_extraneous_conversation_tags_spec.rb
View File

@ -26,7 +26,7 @@ describe 'DataFixup::RemoveExtraneousConversationTags' do
@course1 = @course
@course2 = course(:active_all => true)
@course2.enroll_student(@u1).update_attribute(:workflow_state, 'active')
@conversation = Conversation.initiate([@u1.id, @u2.id], true)
@conversation = Conversation.initiate([@u1, @u2], true)
@conversation.add_message(@u1, 'test', :tags => [@course1.asset_string])
@message = @conversation.add_message(@u1, 'test')
@cp1 = @u1.conversations.first

6
spec/models/alert_spec.rb
View File

@ -235,7 +235,7 @@ describe Alert do
@teacher = @user
@user = nil
student_in_course(:active_all => 1)
@conversation = @teacher.initiate_conversation([@user.id])
@conversation = @teacher.initiate_conversation([@user])
@conversation.add_message("hello")
alert = @course.alerts.build(:recipients => [:student])
@ -251,7 +251,7 @@ describe Alert do
@teacher = @user
@user = nil
student_in_course(:active_all => 1)
@conversation = @teacher.initiate_conversation([@student.id, user.id])
@conversation = @teacher.initiate_conversation([@student, user])
message = @conversation.add_message("hello")
message.created_at = Time.now - 30.days
message.save!
@ -264,7 +264,7 @@ describe Alert do
Alert.sent_alerts.should == [ @student.id ]
# create a generated message
@conversation.add_participants([user.id])
@conversation.add_participants([user])
@conversation.messages.length.should == 2
# it should still alert, ignoring the new message

10
spec/models/conversation_batch_spec.rb
View File

@ -31,13 +31,13 @@ describe ConversationBatch do
context "generate" do
it "should create an async batch" do
batch = ConversationBatch.generate(@message, [@user2.id, @user3.id], :async)
batch = ConversationBatch.generate(@message, [@user2, @user3], :async)
batch.should be_created
batch.completion.should < 1
end
it "should create a sync batch and run it" do
batch = ConversationBatch.generate(@message, [@user2.id, @user3.id], :sync)
batch = ConversationBatch.generate(@message, [@user2, @user3], :sync)
batch.should be_sent
batch.completion.should eql 1
batch.root_conversation_message.reload.conversation.should be_nil
@ -51,7 +51,7 @@ describe ConversationBatch do
context "deliver" do
it "should be sent to all recipients" do
batch = ConversationBatch.generate(@message, [@user2.id, @user3.id], :async)
batch = ConversationBatch.generate(@message, [@user2, @user3], :async)
batch.deliver
batch.should be_sent
@ -67,7 +67,7 @@ describe ConversationBatch do
it "should apply the tags to each conversation" do
g = @course.groups.create
g.users << @user1 << @user2
batch = ConversationBatch.generate(@message, [@user2.id, @user3.id], :async, :tags => [g.asset_string])
batch = ConversationBatch.generate(@message, [@user2, @user3], :async, :tags => [g.asset_string])
batch.deliver
ConversationMessage.count.should eql 3 # the root message, plus the ones to each recipient
@ -83,7 +83,7 @@ describe ConversationBatch do
attachment = attachment_model(:context => @user1, :folder => @user1.conversation_attachments_folder)
@message = Conversation.build_message @user1, "hi all", :attachment_ids => [attachment.id]
batch = ConversationBatch.generate(@message, [@user2.id, @user3.id], :async)
batch = ConversationBatch.generate(@message, [@user2, @user3], :async)
batch.deliver
ConversationMessage.count.should eql 3

30
spec/models/conversation_message_spec.rb
View File

@ -36,7 +36,7 @@ describe ConversationMessage do
channel.confirm
end
@conversation = @teacher.initiate_conversation(@initial_students.map(&:id))
@conversation = @teacher.initiate_conversation(@initial_students)
add_message # need initial message for add_participants to not barf
end
@ -45,7 +45,7 @@ describe ConversationMessage do
end
def add_last_student
@conversation.add_participants([@last_student.id])
@conversation.add_participants([@last_student])
end
it "should create appropriate notifications on new message" do
@ -112,7 +112,7 @@ describe ConversationMessage do
Account.default.update_attribute :enable_user_notes, true
course_with_teacher
student = student_in_course.user
conversation = @teacher.initiate_conversation([student.id])
conversation = @teacher.initiate_conversation([student])
ConversationMessage.any_instance.stubs(:current_time_from_proper_timezone).returns(Time.at(0))
conversation.add_message("reprimanded!", :generate_user_note => true)
student.user_notes.size.should be(1)
@ -126,7 +126,7 @@ describe ConversationMessage do
Account.default.update_attribute :enable_user_notes, false
course_with_teacher
student = student_in_course.user
conversation = @teacher.initiate_conversation([student.id])
conversation = @teacher.initiate_conversation([student])
conversation.add_message("reprimanded!", :generate_user_note => true)
student.user_notes.size.should be(0)
end
@ -136,7 +136,7 @@ describe ConversationMessage do
course_with_teacher
student1 = student_in_course.user
student2 = student_in_course.user
conversation = @teacher.initiate_conversation([student1.id, student2.id])
conversation = @teacher.initiate_conversation([student1, student2])
conversation.add_message("reprimanded!", :generate_user_note => true)
student1.user_notes.size.should be(0)
student2.user_notes.size.should be(0)
@ -149,7 +149,7 @@ describe ConversationMessage do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
message = conversation.add_message("initial message")
StreamItem.count.should eql(old_count + 1)
@ -176,7 +176,7 @@ describe ConversationMessage do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
conversation.add_message("first message")
stream_item = StreamItem.last
conversation.add_message("second message")
@ -191,7 +191,7 @@ describe ConversationMessage do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
conversation.add_message("initial message")
message = conversation.add_message("second message")
@ -212,7 +212,7 @@ describe ConversationMessage do
it "should set has_attachments if there are attachments" do
a = attachment_model(:context => @teacher, :folder => @teacher.conversation_attachments_folder)
m = @teacher.initiate_conversation([@student.id]).add_message("ohai", :attachment_ids => [a.id])
m = @teacher.initiate_conversation([@student]).add_message("ohai", :attachment_ids => [a.id])
m.read_attribute(:has_attachments).should be_true
m.conversation.reload.has_attachments.should be_true
m.conversation.conversation_participants.all?(&:has_attachments?).should be_true
@ -220,8 +220,8 @@ describe ConversationMessage do
it "should set has_attachments if there are forwareded attachments" do
a = attachment_model(:context => @teacher, :folder => @teacher.conversation_attachments_folder)
m1 = @teacher.initiate_conversation([user.id]).add_message("ohai", :attachment_ids => [a.id])
m2 = @teacher.initiate_conversation([@student.id]).add_message("lulz", :forwarded_message_ids => [m1.id])
m1 = @teacher.initiate_conversation([user]).add_message("ohai", :attachment_ids => [a.id])
m2 = @teacher.initiate_conversation([@student]).add_message("lulz", :forwarded_message_ids => [m1.id])
m2.read_attribute(:has_attachments).should be_true
m2.conversation.reload.has_attachments.should be_true
m2.conversation.conversation_participants.all?(&:has_attachments?).should be_true
@ -233,7 +233,7 @@ describe ConversationMessage do
mc.media_id = 'asdf'
mc.context = mc.user = @teacher
mc.save
m = @teacher.initiate_conversation([@student.id]).add_message("ohai", :media_comment => mc)
m = @teacher.initiate_conversation([@student]).add_message("ohai", :media_comment => mc)
m.read_attribute(:has_media_objects).should be_true
m.conversation.reload.has_media_objects.should be_true
m.conversation.conversation_participants.all?(&:has_media_objects?).should be_true
@ -245,8 +245,8 @@ describe ConversationMessage do
mc.media_id = 'asdf'
mc.context = mc.user = @teacher
mc.save
m1 = @teacher.initiate_conversation([user.id]).add_message("ohai", :media_comment => mc)
m2 = @teacher.initiate_conversation([@student.id]).add_message("lulz", :forwarded_message_ids => [m1.id])
m1 = @teacher.initiate_conversation([user]).add_message("ohai", :media_comment => mc)
m2 = @teacher.initiate_conversation([@student]).add_message("lulz", :forwarded_message_ids => [m1.id])
m2.read_attribute(:has_media_objects).should be_true
m2.conversation.reload.has_media_objects.should be_true
m2.conversation.conversation_participants.all?(&:has_media_objects?).should be_true
@ -257,7 +257,7 @@ describe ConversationMessage do
it "should ignore replies on deleted accounts" do
course_with_teacher
student_in_course
conversation = @teacher.initiate_conversation([@user.id])
conversation = @teacher.initiate_conversation([@user])
cm = conversation.add_message("initial message", :root_account_id => Account.default.id)
Account.default.destroy

83
spec/models/conversation_participant_spec.rb
View File

@ -16,13 +16,13 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper.rb')
describe ConversationParticipant do
it "should correctly set up conversations" do
sender = user
recipient = user
convo = sender.initiate_conversation([recipient.id])
convo = sender.initiate_conversation([recipient])
convo.add_message('test')
sender.conversations.should == [convo]
@ -34,20 +34,21 @@ describe ConversationParticipant do
it "should correctly manage messages" do
sender = user
recipient = user
convo = sender.initiate_conversation([recipient.id])
convo = sender.initiate_conversation([recipient])
convo.add_message('test')
convo.add_message('another')
rconvo = recipient.conversations.first
convo.messages.size.should == 2
rconvo.messages.size.should == 2
convo.messages.delete(convo.messages.last)
convo.remove_messages(convo.messages.last)
convo.messages.reload
convo.messages.size.should == 1
# the recipient's messages are unaffected, since it's a has_many :through
# the recipient's messages are unaffected, since removing a message
# only removes it from the join table
rconvo.messages.size.should == 2
convo.messages.clear
convo.remove_messages(:all)
rconvo.reload
rconvo.messages.size.should == 2
end
@ -56,7 +57,7 @@ describe ConversationParticipant do
sender = user
recipient = user
updated_at = sender.updated_at
conversation = sender.initiate_conversation([recipient.id])
conversation = sender.initiate_conversation([recipient])
conversation.update_attribute(:workflow_state, 'unread')
sender.reload.updated_at.should_not eql updated_at
end
@ -64,7 +65,7 @@ describe ConversationParticipant do
it "should support starred/starred=" do
sender = user
recipient = user
conversation = sender.initiate_conversation([recipient.id])
conversation = sender.initiate_conversation([recipient])
conversation.starred = true
conversation.save
@ -80,7 +81,7 @@ describe ConversationParticipant do
it "should support :starred in update_attributes" do
sender = user
recipient = user
conversation = sender.initiate_conversation([recipient.id])
conversation = sender.initiate_conversation([recipient])
conversation.update_attributes(:starred => true)
conversation.save
@ -97,7 +98,7 @@ describe ConversationParticipant do
def conversation_for(*tags_or_users)
users, tags = tags_or_users.partition{ |u| u.is_a?(User) }
users << user if users.empty?
c = @me.initiate_conversation(users.map(&:id))
c = @me.initiate_conversation(users)
c.add_message("test")
c.tags = tags
c.save!
@ -165,15 +166,15 @@ describe ConversationParticipant do
@target_user = user
# visible to @user
@c1 = @target_user.initiate_conversation([user.id])
@c1 = @target_user.initiate_conversation([user])
@c1.add_message("hey man", :root_account_id => @a1.id)
@c2 = @target_user.initiate_conversation([user.id])
@c2 = @target_user.initiate_conversation([user])
@c2.add_message("foo", :root_account_id => @a1.id)
@c2.add_message("bar", :root_account_id => @a2.id)
# invisible to @user, unless @user is a site admin
@c3 = @target_user.initiate_conversation([user.id])
@c3 = @target_user.initiate_conversation([user])
@c3.add_message("secret", :root_account_id => @a3.id)
@c4 = @target_user.initiate_conversation([user.id])
@c4 = @target_user.initiate_conversation([user])
@c4.add_message("super", :root_account_id => @a1.id)
@c4.add_message("sekrit", :root_account_id => @a3.id)
end
@ -198,12 +199,12 @@ describe ConversationParticipant do
@u1 = student_in_course(:active_all => true).user
@u2 = student_in_course(:active_all => true).user
@u3 = student_in_course(:active_all => true).user
@convo = @me.initiate_conversation([@u1.id, @u2.id, @u3.id])
@convo = @me.initiate_conversation([@u1, @u2, @u3])
@convo.add_message "ohai"
@u3.destroy
@u4 = student_in_course(:active_all => true).user
other_convo = @u4.initiate_conversation([@me.id])
other_convo = @u4.initiate_conversation([@me])
message = other_convo.add_message "just between you and me"
@convo.add_message("haha i forwarded it", :forwarded_message_ids => [message.id])
end
@ -246,24 +247,24 @@ describe ConversationParticipant do
end
it "should move a group conversation to the new user" do
c = @user1.initiate_conversation([user.id, user.id])
c = @user1.initiate_conversation([user, user])
c.add_message("hello")
c.update_attribute(:workflow_state, 'unread')
c.move_to_user @user2
c.reload.user_id.should eql @user2.id
c.conversation.participant_ids.should_not include(@user1.id)
c.conversation.participants.should_not include(@user1)
@user1.reload.unread_conversations_count.should eql 0
@user2.reload.unread_conversations_count.should eql 1
end
it "should clean up group conversations having both users" do
c = @user1.initiate_conversation([@user2.id, user.id, user.id])
c = @user1.initiate_conversation([@user2, user, user])
c.add_message("hello")
c.update_attribute(:workflow_state, 'unread')
rconvo = c.conversation
rconvo.participant_ids.size.should eql 4
rconvo.participants.size.should eql 4
c.move_to_user @user2
@ -271,14 +272,14 @@ describe ConversationParticipant do
rconvo.reload
rconvo.participants.size.should eql 3
rconvo.participant_ids.should_not include(@user1.id)
rconvo.participant_ids.should include(@user2.id)
rconvo.participants.should_not include(@user1)
rconvo.participants.should include(@user2)
@user1.reload.unread_conversations_count.should eql 0
@user2.reload.unread_conversations_count.should eql 1
end
it "should move a private conversation to the new user" do
c = @user1.initiate_conversation([user.id])
c = @user1.initiate_conversation([user])
c.add_message("hello")
c.update_attribute(:workflow_state, 'unread')
rconvo = c.conversation
@ -296,10 +297,10 @@ describe ConversationParticipant do
it "should merge a private conversation into the existing private conversation" do
other_guy = user
c = @user1.initiate_conversation([other_guy.id])
c = @user1.initiate_conversation([other_guy])
c.add_message("hello")
c.update_attribute(:workflow_state, 'unread')
c2 = @user2.initiate_conversation([other_guy.id])
c2 = @user2.initiate_conversation([other_guy])
c2.add_message("hola")
c.reload.move_to_user @user2
@ -318,7 +319,7 @@ describe ConversationParticipant do
end
it "should change a private conversation between the two users into a monologue" do
c = @user1.initiate_conversation([@user2.id])
c = @user1.initiate_conversation([@user2])
c.add_message("hello self")
c.update_attribute(:workflow_state, 'unread')
@user2.mark_all_conversations_as_read!
@ -336,10 +337,10 @@ describe ConversationParticipant do
end
it "should merge a private conversations between the two users into the existing monologue" do
c = @user1.initiate_conversation([@user2.id])
c = @user1.initiate_conversation([@user2])
c.add_message("hello self")
c.update_attribute(:workflow_state, 'unread')
c2 = @user2.initiate_conversation([@user2.id])
c2 = @user2.initiate_conversation([@user2])
c2.add_message("monologue!")
@user2.mark_all_conversations_as_read!
@ -358,10 +359,10 @@ describe ConversationParticipant do
end
it "should merge a monologue into the existing monologue" do
c = @user1.initiate_conversation([@user1.id])
c = @user1.initiate_conversation([@user1])
c.add_message("monologue 1")
c.update_attribute(:workflow_state, 'unread')
c2 = @user2.initiate_conversation([@user2.id])
c2 = @user2.initiate_conversation([@user2])
c2.add_message("monologue 2")
c.reload.move_to_user @user2
@ -380,10 +381,10 @@ describe ConversationParticipant do
it "should not be adversely affected by an outer scope" do
other_guy = user
c = @user1.initiate_conversation([other_guy.id])
c = @user1.initiate_conversation([other_guy])
c.add_message("hello")
c.update_attribute(:workflow_state, 'unread')
c2 = @user2.initiate_conversation([other_guy.id])
c2 = @user2.initiate_conversation([other_guy])
c2.add_message("hola")
c.reload
@ -403,5 +404,23 @@ describe ConversationParticipant do
@user2.reload.unread_conversations_count.should eql 1
other_guy.reload.unread_conversations_count.should eql 1
end
context "sharding" do
it_should_behave_like "sharding"
it "should be able to move to a user on a different shard" do
u1 = User.create!
cp = u1.initiate_conversation([u1])
@shard1.activate do
u2 = User.create!
cp.move_to_user(u2)
cp.reload
cp.user.should == u2
cp2 = u2.all_conversations.first
cp2.should_not == cp
cp2.shard.should == @shard1
end
end
end
end
end

444
spec/models/conversation_spec.rb
View File

@ -16,48 +16,72 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper.rb')
describe Conversation do
context "initiation" do
it "should set private_hash for private conversations" do
users = 2.times.map{ user }
Conversation.initiate(users.map(&:id), true).private_hash.should_not be_nil
Conversation.initiate(users, true).private_hash.should_not be_nil
end
it "should not set private_hash for group conversations" do
users = 3.times.map{ user }
Conversation.initiate(users.map(&:id), false).private_hash.should be_nil
Conversation.initiate(users, false).private_hash.should be_nil
end
it "should reuse private conversations" do
users = 2.times.map{ user }
Conversation.initiate(users.map(&:id), true).should ==
Conversation.initiate(users.map(&:id), true)
Conversation.initiate(users, true).should ==
Conversation.initiate(users, true)
end
it "should not reuse group conversations" do
users = 2.times.map{ user }
Conversation.initiate(users.map(&:id), false).should_not ==
Conversation.initiate(users.map(&:id), false)
Conversation.initiate(users, false).should_not ==
Conversation.initiate(users, false)
end
context "sharding" do
it_should_behave_like "sharding"
it "should create the conversation on the appropriate shard" do
users = []
users << user(:name => 'a')
@shard1.activate { users << user(:name => 'b') }
@shard2.activate { users << user(:name => 'c') }
Shard.with_each_shard([Shard.default, @shard1, @shard2]) do
conversation = Conversation.initiate(users, false)
conversation.shard.should == Shard.current
conversation.conversation_participants.all? { |cp| cp.shard == Shard.current }.should be_true
conversation.conversation_participants.length.should == 3
conversation.participants.should == users
cp = users[0].all_conversations.last
cp.shard.should == Shard.default
cp = users[1].all_conversations.last
cp.shard.should == @shard1
cp = users[2].all_conversations.last
cp.shard.should == @shard2
end
end
end
end
context "adding participants" do
it "should not add participants to private conversations" do
sender = user
root_convo = Conversation.initiate([sender.id, user.id], true)
lambda{ root_convo.add_participants(sender, [user.id]) }.should raise_error
root_convo = Conversation.initiate([sender, user], true)
lambda{ root_convo.add_participants(sender, [user]) }.should raise_error
end
it "should add new participants to group conversations and give them all messages" do
sender = user
root_convo = Conversation.initiate([sender.id, user.id], false)
root_convo = Conversation.initiate([sender, user], false)
root_convo.add_message(sender, 'test')
new_guy = user
lambda{ root_convo.add_participants(sender, [new_guy.id]) }.should_not raise_error
root_convo.participants.size.should == 3
lambda{ root_convo.add_participants(sender, [new_guy]) }.should_not raise_error
root_convo.participants(true).size.should == 3
convo = new_guy.conversations.first
convo.unread?.should be_true
@ -68,125 +92,171 @@ describe Conversation do
it "should not re-add existing participants to group conversations" do
sender = user
recipient = user
root_convo = Conversation.initiate([sender.id, recipient.id], false)
lambda{ root_convo.add_participants(sender, [recipient.id]) }.should_not raise_error
root_convo = Conversation.initiate([sender, recipient], false)
lambda{ root_convo.add_participants(sender, [recipient]) }.should_not raise_error
root_convo.participants.size.should == 2
end
it "should update the updated_at timestamp and clear the identity header cache of new participants" do
sender = user
root_convo = Conversation.initiate([sender.id, user.id], false)
root_convo = Conversation.initiate([sender, user], false)
root_convo.add_message(sender, 'test')
new_guy = user
old_updated_at = new_guy.updated_at
root_convo.add_participants(sender, [new_guy.id])
root_convo.add_participants(sender, [new_guy])
new_guy.reload.updated_at.should_not eql old_updated_at
end
context "sharding" do
it_should_behave_like "sharding"
it "should add participants to the proper shards" do
users = []
users << user(:name => 'a')
users << user(:name => 'b')
users << user(:name => 'c')
conversation = Conversation.initiate(users, false)
conversation.add_message(users.first, 'test')
conversation.conversation_participants.size.should == 3
@shard1.activate do
users << user(:name => 'd')
conversation.add_participants(users.first, [users.last])
conversation.conversation_participants(:reload).size.should == 4
conversation.conversation_participants.all? { |cp| cp.shard == Shard.default }.should be_true
users.last.all_conversations.last.shard.should == @shard1
conversation.participants(true).should == users
end
@shard2.activate do
users << user(:name => 'e')
conversation.add_participants(users.first, users[-2..-1])
conversation.conversation_participants(:reload).size.should == 5
conversation.conversation_participants.all? { |cp| cp.shard == Shard.default }.should be_true
users.last.all_conversations.last.shard.should == @shard2
conversation.participants(true).should == users
end
end
end
end
context "message counts" do
it "should increment when adding messages" do
sender = user
recipient = user
Conversation.initiate([sender.id, recipient.id], false).add_message(sender, 'test')
sender.conversations.first.message_count.should eql 1
recipient.conversations.first.message_count.should eql 1
shared_examples_for "message counts" do
before do
(@shard1 || Shard.default).activate do
@sender = user
@recipient = user
end
end
it "should increment when adding messages" do
Conversation.initiate([@sender, @recipient], false).add_message(@sender, 'test')
@sender.conversations.first.message_count.should eql 1
@recipient.conversations.first.message_count.should eql 1
end
it "should decrement when removing messages" do
root_convo = Conversation.initiate([@sender, @recipient], false)
root_convo.add_message(@sender, 'test')
msg = root_convo.add_message(@sender, 'test2')
@sender.conversations.first.message_count.should eql 2
@recipient.conversations.first.message_count.should eql 2
@sender.conversations.first.remove_messages(msg)
@sender.conversations.first.reload.message_count.should eql 1
@recipient.conversations.first.reload.message_count.should eql 2
end
end
it "should decrement when removing messages" do
sender = user
recipient = user
root_convo = Conversation.initiate([sender.id, recipient.id], false)
root_convo.add_message(sender, 'test')
msg = root_convo.add_message(sender, 'test2')
sender.conversations.first.message_count.should eql 2
recipient.conversations.first.message_count.should eql 2
it_should_behave_like "message counts"
sender.conversations.first.remove_messages(msg)
sender.conversations.first.reload.message_count.should eql 1
recipient.conversations.first.reload.message_count.should eql 2
context "sharding" do
it_should_behave_like "sharding"
it_should_behave_like "message counts"
end
end
context "unread counts" do
it "should increment for recipients when sending the first message in a conversation" do
sender = user
recipient = user
root_convo = Conversation.initiate([sender.id, recipient.id], false)
ConversationParticipant.unread.size.should eql 0 # only once the first message is added
root_convo.add_message(sender, 'test')
sender.reload.unread_conversations_count.should eql 0
sender.conversations.unread.size.should eql 0
recipient.reload.unread_conversations_count.should eql 1
recipient.conversations.unread.size.should eql 1
shared_examples_for "unread counts" do
before do
(@shard1 || Shard.default).activate do
@sender = user
@unread_guy = @recipient = user
@subscribed_guy = user
@unsubscribed_guy = user
end
end
it "should increment for recipients when sending the first message in a conversation" do
root_convo = Conversation.initiate([@sender, @recipient], false)
ConversationParticipant.unread.size.should eql 0 # only once the first message is added
root_convo.add_message(@sender, 'test')
@sender.reload.unread_conversations_count.should eql 0
@sender.conversations.unread.size.should eql 0
@recipient.reload.unread_conversations_count.should eql 1
@recipient.conversations.unread.size.should eql 1
end
it "should increment for subscribed recipients when adding a message to a read conversation" do
root_convo = Conversation.initiate([@sender, @unread_guy, @subscribed_guy, @unsubscribed_guy], false)
root_convo.add_message(@sender, 'test')
@unread_guy.reload.unread_conversations_count.should eql 1
@unread_guy.conversations.unread.size.should eql 1
@subscribed_guy.conversations.first.update_attribute(:workflow_state, "read")
@subscribed_guy.reload.unread_conversations_count.should eql 0
@subscribed_guy.conversations.unread.size.should eql 0
@unsubscribed_guy.conversations.first.update_attributes(:subscribed => false)
@unsubscribed_guy.reload.unread_conversations_count.should eql 0
@unsubscribed_guy.conversations.unread.size.should eql 0
root_convo.add_message(@sender, 'test2')
@unread_guy.reload.unread_conversations_count.should eql 1
@unread_guy.conversations.unread.size.should eql 1
@subscribed_guy.reload.unread_conversations_count.should eql 1
@subscribed_guy.conversations.unread.size.should eql 1
@unsubscribed_guy.reload.unread_conversations_count.should eql 0
@unsubscribed_guy.conversations.unread.size.should eql 0
end
it "should decrement when deleting an unread conversation" do
root_convo = Conversation.initiate([@sender, @unread_guy], false)
root_convo.add_message(@sender, 'test')
@unread_guy.reload.unread_conversations_count.should eql 1
@unread_guy.conversations.unread.size.should eql 1
@unread_guy.conversations.first.remove_messages(:all)
@unread_guy.reload.unread_conversations_count.should eql 0
@unread_guy.conversations.unread.size.should eql 0
end
it "should decrement when marking as read" do
root_convo = Conversation.initiate([@sender, @unread_guy], false)
root_convo.add_message(@sender, 'test')
@unread_guy.reload.unread_conversations_count.should eql 1
@unread_guy.conversations.unread.size.should eql 1
@unread_guy.conversations.first.update_attribute(:workflow_state, "read")
@unread_guy.reload.unread_conversations_count.should eql 0
@unread_guy.conversations.unread.size.should eql 0
end
it "should indecrement when marking as unread" do
root_convo = Conversation.initiate([@sender, @unread_guy], false)
root_convo.add_message(@sender, 'test')
@unread_guy.conversations.first.update_attribute(:workflow_state, "read")
@unread_guy.reload.unread_conversations_count.should eql 0
@unread_guy.conversations.unread.size.should eql 0
@unread_guy.conversations.first.update_attribute(:workflow_state, "unread")
@unread_guy.reload.unread_conversations_count.should eql 1
@unread_guy.conversations.unread.size.should eql 1
end
end
it "should increment for subscribed recipients when adding a message to a read conversation" do
sender = user
unread_guy = user
subscribed_guy = user
unsubscribed_guy = user
root_convo = Conversation.initiate([sender.id, unread_guy.id, subscribed_guy.id, unsubscribed_guy.id], false)
root_convo.add_message(sender, 'test')
unread_guy.reload.unread_conversations_count.should eql 1
unread_guy.conversations.unread.size.should eql 1
subscribed_guy.conversations.first.update_attribute(:workflow_state, "read")
subscribed_guy.reload.unread_conversations_count.should eql 0
subscribed_guy.conversations.unread.size.should eql 0
unsubscribed_guy.conversations.first.update_attributes(:subscribed => false)
unsubscribed_guy.reload.unread_conversations_count.should eql 0
unsubscribed_guy.conversations.unread.size.should eql 0
root_convo.add_message(sender, 'test2')
unread_guy.reload.unread_conversations_count.should eql 1
unread_guy.conversations.unread.size.should eql 1
subscribed_guy.reload.unread_conversations_count.should eql 1
subscribed_guy.conversations.unread.size.should eql 1
unsubscribed_guy.reload.unread_conversations_count.should eql 0
unsubscribed_guy.conversations.unread.size.should eql 0
end
it "should decrement when deleting an unread conversation" do
sender = user
unread_guy = user
root_convo = Conversation.initiate([sender.id, unread_guy.id], false)
root_convo.add_message(sender, 'test')
unread_guy.reload.unread_conversations_count.should eql 1
unread_guy.conversations.unread.size.should eql 1
unread_guy.conversations.first.remove_messages(:all)
unread_guy.reload.unread_conversations_count.should eql 0
unread_guy.conversations.unread.size.should eql 0
end
it "should decrement when marking as read" do
sender = user
unread_guy = user
root_convo = Conversation.initiate([sender.id, unread_guy.id], false)
root_convo.add_message(sender, 'test')
unread_guy.reload.unread_conversations_count.should eql 1
unread_guy.conversations.unread.size.should eql 1
unread_guy.conversations.first.update_attribute(:workflow_state, "read")
unread_guy.reload.unread_conversations_count.should eql 0
unread_guy.conversations.unread.size.should eql 0
end
it "should indecrement when marking as unread" do
sender = user
unread_guy = user
root_convo = Conversation.initiate([sender.id, unread_guy.id], false)
root_convo.add_message(sender, 'test')
unread_guy.conversations.first.update_attribute(:workflow_state, "read")
unread_guy.reload.unread_conversations_count.should eql 0
unread_guy.conversations.unread.size.should eql 0
unread_guy.conversations.first.update_attribute(:workflow_state, "unread")
unread_guy.reload.unread_conversations_count.should eql 1
unread_guy.conversations.unread.size.should eql 1
it_should_behave_like "unread counts"
context "sharding" do
it_should_behave_like "sharding"
it_should_behave_like "unread counts"
end
end
@ -195,7 +265,7 @@ describe Conversation do
sender = user
subscription_guy = user
archive_guy = user
root_convo = Conversation.initiate([sender.id, archive_guy.id, subscription_guy.id], false)
root_convo = Conversation.initiate([sender, archive_guy, subscription_guy], false)
root_convo.add_message(sender, 'test')
subscription_guy.reload.unread_conversations_count.should eql 1
@ -214,7 +284,7 @@ describe Conversation do
flip_flopper_guy = user
subscription_guy = user
archive_guy = user
root_convo = Conversation.initiate([sender.id, flip_flopper_guy.id, archive_guy.id, subscription_guy.id], false)
root_convo = Conversation.initiate([sender, flip_flopper_guy, archive_guy, subscription_guy], false)
root_convo.add_message(sender, 'test')
flip_flopper_guy.conversations.first.update_attributes(:subscribed => false)
@ -247,7 +317,7 @@ describe Conversation do
it "should not toggle read/unread until the subscription change is saved" do
sender = user
subscription_guy = user
root_convo = Conversation.initiate([sender.id, user.id, subscription_guy.id], false)
root_convo = Conversation.initiate([sender, user, subscription_guy], false)
root_convo.add_message(sender, 'test')
subscription_guy.reload.unread_conversations_count.should eql 1
@ -267,7 +337,7 @@ describe Conversation do
it "should deliver the message to all participants" do
sender = user
recipients = 5.times.map{ user }
Conversation.initiate([sender.id] + recipients.map(&:id), false).add_message(sender, 'test')
Conversation.initiate([sender] + recipients, false).add_message(sender, 'test')
convo = sender.conversations.first
convo.reload.read?.should be_true # only for the sender, and then only on the first message
convo.messages.size.should == 1
@ -282,7 +352,7 @@ describe Conversation do
it "should only ever change the workflow_state for the sender if it's archived and it's a direct message (not bulk)" do
sender = user
Conversation.initiate([sender.id, user.id], true).add_message(sender, 'test')
Conversation.initiate([sender, user], true).add_message(sender, 'test')
convo = sender.conversations.first
convo.update_attribute(:workflow_state, "unread")
convo.add_message('another test', :update_for_sender => false) # as if it were a bulk private message
@ -304,12 +374,12 @@ describe Conversation do
it "should not set last_message_at for the sender if the conversation is deleted and update_for_sender=false" do
sender = user
rconvo = Conversation.initiate([sender.id, user.id], true)
rconvo = Conversation.initiate([sender, user], true)
message = rconvo.add_message(sender, 'test')
convo = sender.conversations.first
convo.last_message_at.should_not be_nil
convo.remove_messages([message])
convo.remove_messages(message)
convo.last_message_at.should be_nil
convo.add_message('bulk message', :update_for_sender => false)
@ -322,13 +392,13 @@ describe Conversation do
ConversationMessage.any_instance.expects(:current_time_from_proper_timezone).twice.returns(*expected_times)
sender = user
rconvo = Conversation.initiate([sender.id, user.id], true)
rconvo = Conversation.initiate([sender, user], true)
message = rconvo.add_message(sender, 'test')
convo = sender.conversations.first
convo.last_authored_at.should eql expected_times.first
convo.visible_last_authored_at.should eql expected_times.first
convo.remove_messages([message])
convo.remove_messages(message)
convo.last_authored_at.should eql expected_times.first
convo.visible_last_authored_at.should be_nil
@ -341,7 +411,7 @@ describe Conversation do
it "should deliver the message to unsubscribed participants but not alert them" do
sender = user
recipients = 5.times.map{ user }
Conversation.initiate([sender.id] + recipients.map(&:id), false).add_message(sender, 'test')
Conversation.initiate([sender] + recipients, false).add_message(sender, 'test')
recipient = recipients.last
rconvo = recipient.conversations.first
@ -372,12 +442,12 @@ describe Conversation do
it "should not create conversations if only_existing is set" do
u1 = user
u2 = user
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
asset = Submission.new(:user => u1)
asset.expects(:conversation_groups).returns([[u1.id, u2.id]])
asset.expects(:conversation_groups).returns([[u1, u2]])
asset.expects(:lock!).returns(true)
asset.expects(:conversation_messages).at_least_once.returns([])
asset.expects(:conversation_message_data).returns({:created_at => Time.now.utc, :author_id => u1.id, :body => "asdf"})
asset.expects(:conversation_message_data).returns({:created_at => Time.now.utc, :author => u1, :body => "asdf"})
Conversation.update_all_for_asset asset, :update_message => true, :only_existing => true
conversation.conversation_messages.size.should eql 1
end
@ -385,12 +455,12 @@ describe Conversation do
it "should create conversations by default" do
u1 = user
u2 = user
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
asset = Submission.new(:user => u1)
asset.expects(:conversation_groups).returns([[u1.id, u2.id]])
asset.expects(:conversation_groups).returns([[u1, u2]])
asset.expects(:lock!).returns(true)
asset.expects(:conversation_messages).at_least_once.returns([])
asset.expects(:conversation_message_data).returns({:created_at => Time.now.utc, :author_id => u1.id, :body => "asdf"})
asset.expects(:conversation_message_data).returns({:created_at => Time.now.utc, :author => u1, :body => "asdf"})
Conversation.expects(:initiate).returns(conversation)
Conversation.update_all_for_asset asset, :update_message => true
conversation.conversation_messages.size.should eql 1
@ -420,7 +490,7 @@ describe Conversation do
course2.enroll_student(u1, :allow_multiple_enrollments => true, :section => other_section)
u1.enrollments.size.should eql 3
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.current_context_strings(1).should eql [course1.asset_string]
u1.conversation_context_codes.sort.should eql [course1.asset_string, course2.asset_string].sort # just once
@ -431,7 +501,7 @@ describe Conversation do
it "should save all valid tags on the conversation" do # NOTE: this will change if/when we allow arbitrary tags
u1 = student_in_course(:active_all => true).user
u2 = student_in_course(:active_all => true, :course => @course).user
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.add_message(u1, 'test', :tags => [@course.asset_string, "asdf", "lol"])
conversation.tags.should eql [@course.asset_string]
end
@ -439,7 +509,7 @@ describe Conversation do
it "should set initial empty tags on the conversation and conversation_participant" do
u1 = student_in_course.user
u2 = student_in_course(:course => @course).user
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.read_attribute(:tags).should_not be_nil
conversation.tags.should eql []
u1.all_conversations.first.read_attribute(:tags).should_not be_nil
@ -455,7 +525,7 @@ describe Conversation do
@course1 = @course
@course2 = course(:active_all => true)
@course2.enroll_student(u1).update_attribute(:workflow_state, 'active')
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string, @course2.asset_string])
conversation.tags.should eql [@course1.asset_string]
end
@ -464,7 +534,7 @@ describe Conversation do
u1 = student_in_course(:active_all => true).user
u2 = student_in_course(:active_all => true, :course => @course).user
u3 = user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course.asset_string])
conversation.tags.should eql [@course.asset_string]
u1.conversations.first.tags.should eql [@course.asset_string]
@ -479,7 +549,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
u3 = student_in_course(:active_all => true, :course => @course2).user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test')
conversation.tags.sort.should eql [@course1.asset_string, @course2.asset_string].sort
u1.conversations.first.tags.should eql [@course1.asset_string]
@ -494,7 +564,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
u3 = student_in_course(:active_all => true, :course => @course2).user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
conversation.tags.should eql [@course1.asset_string]
u1.conversations.first.tags.should eql [@course1.asset_string]
@ -507,7 +577,7 @@ describe Conversation do
it "should remove tags when all messages are deleted" do
u1 = student_in_course(:active_all => true).user
u2 = student_in_course(:active_all => true, :course => @course).user
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.add_message(u1, 'test')
conversation.tags.should eql [@course.asset_string]
cp1 = u1.conversations.first
@ -532,7 +602,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
u3 = student_in_course(:active_all => true, :course => @course2).user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
conversation.tags.should eql [@course1.asset_string]
@ -547,7 +617,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
u3 = student_in_course(:active_all => true, :course => @course2).user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
u1.conversations.first.tags.should eql [@course1.asset_string]
u2.conversations.first.tags.should eql [@course1.asset_string]
@ -566,7 +636,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
u3 = student_in_course(:active_all => true, :course => @course2).user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
u1.conversations.first.tags.should eql [@course1.asset_string]
u2.conversations.first.tags.should eql [@course1.asset_string]
@ -591,7 +661,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u1).update_attribute(:workflow_state, 'active')
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
cp = u2.conversations.first
cp.messages.human.first.tags.should eql [@course1.asset_string]
@ -603,7 +673,7 @@ describe Conversation do
it "should save the previous message tags on the conversation_message_participant if there are no new visible ones" do
u1 = student_in_course(:active_all => true).user
u2 = student_in_course(:active_all => true, :course => @course).user
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.add_message(u1, 'test', :tags => [@course.asset_string])
cp = u2.conversations.first
cp.messages.human.first.tags.should eql [@course.asset_string]
@ -619,7 +689,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u1).update_attribute(:workflow_state, 'active')
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
cp = u2.conversations.first
cp.tags.should eql [@course1.asset_string]
@ -640,7 +710,7 @@ describe Conversation do
u2 = student_in_course(:active_all => true, :course => @course).user
u3 = student_in_course(:active_all => true, :course => @course).user
@course = @course
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course.asset_string])
u1.conversations.first.messages.human.first.tags.should eql []
u2.conversations.first.messages.human.first.tags.should eql []
@ -654,7 +724,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
u3 = student_in_course(:active_all => true, :course => @course2).user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
cp = u2.conversations.first
cp.tags.should eql [@course1.asset_string]
@ -674,14 +744,14 @@ describe Conversation do
@course2.enroll_student(u2).update_attribute(:workflow_state, 'active')
u3 = student_in_course(:active_all => true, :course => @course2).user
u4 = student_in_course(:active_all => true, :course => @course2).user
conversation = Conversation.initiate([u1.id, u2.id, u3.id], false)
conversation = Conversation.initiate([u1, u2, u3], false)
conversation.add_message(u1, 'test', :tags => [@course1.asset_string])
conversation.tags.should eql [@course1.asset_string]
u1.conversations.first.tags.should eql [@course1.asset_string]
u2.conversations.first.tags.should eql [@course1.asset_string]
u3.conversations.first.tags.should eql [@course2.asset_string]
conversation.add_participants(u2, [u4.id], :tags => [@course2.asset_string])
conversation.add_participants(u2, [u4], :tags => [@course2.asset_string])
conversation.reload.tags.sort.should eql [@course1.asset_string, @course2.asset_string].sort
u1.conversations.first.tags.should eql [@course1.asset_string]
u2.conversations.first.tags.sort.should eql [@course1.asset_string, @course2.asset_string].sort
@ -698,7 +768,7 @@ describe Conversation do
@course2 = course(:active_all => true)
@course2.enroll_student(@u2).update_attribute(:workflow_state, 'active')
@u3 = student_in_course(:active_all => true, :course => @course2).user
@conversation = Conversation.initiate([@u1.id, @u2.id, @u3.id], false)
@conversation = Conversation.initiate([@u1, @u2, @u3], false)
@conversation.add_message(@u1, 'test', :tags => [@course1.asset_string])
Conversation.update_all "tags = NULL"
ConversationParticipant.update_all "tags = NULL"
@ -745,10 +815,96 @@ describe Conversation do
it "should be saved on the conversation when adding a message" do
u1 = user
u2 = user
conversation = Conversation.initiate([u1.id, u2.id], true)
conversation = Conversation.initiate([u1, u2], true)
conversation.add_message(u1, 'ohai', :root_account_id => 1)
conversation.add_message(u2, 'ohai yourself', :root_account_id => 2)
conversation.root_account_ids.should eql [1, 2]
end
end
def merge_and_check(sender, source, target, source_user, target_user)
raise "source_user and target_user must be the same" if source_user && target_user && source_user != target_user
source.add_participants(sender, [source_user]) if source_user
target.add_participants(sender, [target_user]) if target_user
target_user = source_user || target_user
message_count = source.shard.activate { ConversationMessageParticipant.count(:all, :joins => :conversation_message, :conditions => {:user_id => target_user.id, :conversation_messages => {:conversation_id => source.id}}) }
message_count += target.shard.activate { ConversationMessageParticipant.count(:all, :joins => :conversation_message, :conditions => {:user_id => target_user.id, :conversation_messages => {:conversation_id => target.id}}) }
source.merge_into(target)
lambda { source.reload }.should raise_error(ActiveRecord::RecordNotFound)
ConversationParticipant.find_all_by_conversation_id(source.id).should == []
ConversationMessage.find_all_by_conversation_id(source.id).should == []
target.reload
target.participants(true).should == [sender, target_user]
target_user.reload.all_conversations.map(&:conversation).should == [target]
cp = target_user.all_conversations.first
cp.messages.length.should == message_count
end
describe "merge_into" do
# non-sharding cases are covered by ConversationParticipant#move_to_user specs
context "sharding" do
it_should_behave_like "sharding"
before do
@sender = User.create!(:name => 'a')
@conversation1 = Conversation.initiate([@sender], false)
@conversation2 = Conversation.initiate([@sender], false)
@conversation3 = @shard1.activate { Conversation.initiate([@sender], false) }
@user1 = User.create!(:name => 'b')
@user2 = @shard1.activate { User.create!(:name => 'c') }
@user3 = @shard2.activate { User.create!(:name => 'd') }
@conversation1.add_message(@sender, 'message1')
@conversation2.add_message(@sender, 'message2')
@conversation3.add_message(@sender, 'message3')
end
context "matching shards" do
it "user from another shard participating in both conversations" do
merge_and_check(@sender, @conversation1, @conversation2, @user2, @user2)
@conversation2.associated_shards.should == [Shard.default, @shard1]
end
it "user from another shard participating in source conversation only" do
merge_and_check(@sender, @conversation1, @conversation2, @user2, nil)
@conversation2.associated_shards.should == [Shard.default, @shard1]
end
end
context "differing shards" do
it "user from source shard participating in both conversations" do
merge_and_check(@sender, @conversation1, @conversation3, @user1, @user1)
@conversation3.associated_shards.should == [@shard1, Shard.default]
end
it "user from destination shard participating in both conversations" do
merge_and_check(@sender, @conversation1, @conversation3, @user2, @user2)
@conversation3.associated_shards.should == [@shard1, Shard.default]
end
it "user from third shard participating in both conversations" do
merge_and_check(@sender, @conversation1, @conversation3, @user3, @user3)
@conversation3.associated_shards.sort_by(&:id).should == [Shard.default, @shard1, @shard2]
end
it "user from source shard participating in source conversation only" do
merge_and_check(@sender, @conversation1, @conversation3, @user1, nil)
@conversation3.associated_shards.should == [@shard1, Shard.default]
end
it "user from destination shard participating in source conversation only" do
merge_and_check(@sender, @conversation1, @conversation3, @user2, nil)
@conversation3.associated_shards.should == [@shard1, Shard.default]
end
it "user from third shard participating in source conversation only" do
merge_and_check(@sender, @conversation1, @conversation3, @user3, nil)
@conversation3.associated_shards.sort_by(&:id).should == [Shard.default, @shard1, @shard2]
end
end
end
end
end

18
spec/models/submission_comment_spec.rb
View File

@ -258,7 +258,7 @@ This text has a http://www.google.com link in it...
end
it "should reuse an existing private conversation, but not change its state for teacher" do
convo = Conversation.initiate([@teacher1.id, @student1.id], true)
convo = Conversation.initiate([@teacher1, @student1], true)
convo.add_message(@teacher1, 'direct message')
@teacher1.conversations.count.should == 1
convo = @teacher1.conversations.first
@ -328,12 +328,12 @@ This text has a http://www.google.com link in it...
@student1.conversations.unread.count.should == 1
end
it "should not block direct message from student" do
convo = Conversation.initiate([@student1.id, @teacher.id], false)
convo = Conversation.initiate([@student1, @teacher], false)
convo.add_message(@student1, 'My direct message')
@teacher.conversations.unread.count.should == 1
end
it "should add submission comments to existing conversations" do
convo = Conversation.initiate([@student1.id, @teacher1.id], true)
convo = Conversation.initiate([@student1, @teacher1], true)
convo.add_message(@student1, 'My direct message')
c = @teacher1.conversations.unread.first
c.should_not be_nil
@ -430,7 +430,7 @@ This text has a http://www.google.com link in it...
end
it "should reuse an existing private conversation, but not change its state for teacher on unmute" do
convo = Conversation.initiate([@teacher1.id, @student1.id], true)
convo = Conversation.initiate([@teacher1, @student1], true)
convo.add_message(@teacher1, 'direct message')
@teacher1.conversations.count.should == 1
convo = @teacher1.conversations.first
@ -537,9 +537,9 @@ This text has a http://www.google.com link in it...
end
it "should only create messages where conversations already exist" do
convo1 = @student1.initiate_conversation([@teacher1.id])
convo1 = @student1.initiate_conversation([@teacher1])
convo1.add_message('ohai')
convo2 = @student1.initiate_conversation([@teacher2.id])
convo2 = @student1.initiate_conversation([@teacher2])
convo2.add_message('hey', :update_for_sender => false) # like if the student did a bulk private message
@student1.conversations.size.should eql 1 # second one is not visible to student
@student1.conversations.first.messages.size.should eql 1
@ -565,7 +565,7 @@ This text has a http://www.google.com link in it...
end
it "should not change any unread count/status" do
convo = @student1.initiate_conversation([@teacher1.id])
convo = @student1.initiate_conversation([@teacher1])
convo.add_message('ohai')
@student1.conversations.size.should eql 1
convo.messages.size.should eql 1
@ -588,7 +588,7 @@ This text has a http://www.google.com link in it...
end
it "should update last_message_at, message_count and last_authored_at" do
convo = @student1.initiate_conversation([@teacher1.id])
convo = @student1.initiate_conversation([@teacher1])
convo.add_message('ohai')
tconvo = @teacher1.conversations.first
raw_comment(@submission1, @student1, "hello", Time.now.utc + 1.day)
@ -608,7 +608,7 @@ This text has a http://www.google.com link in it...
end
it "should skip submissions with no participant comments" do
convo = @student1.initiate_conversation([@teacher1.id])
convo = @student1.initiate_conversation([@teacher1])
message = convo.add_message('ohai').reload
tconvo = @teacher1.conversations.first
raw_comment(@submission1, user, "ohai im in ur group", Time.now.utc + 1.day)

2
spec/selenium/conversations_recipient_finder_spec.rb
View File

@ -236,7 +236,7 @@ describe "conversations recipient finder" do
it "should allow a non-contactable user in the hash if a shared conversation exists" do
other = User.create(:name => "other guy")
# if the users have a conversation in common already, then the recipient can be added
c = Conversation.initiate([@user.id, other.id], true)
c = Conversation.initiate([@user, other], true)
get conversations_path(:user_id => other.id, :from_conversation_id => c.id)
wait_for_ajaximations
tokens.should == ["other guy"]

4
spec/selenium/dashboard_spec.rb
View File

@ -96,9 +96,9 @@ describe "dashboard" do
it "should remove the stream item category if all items are removed"
it "should show conversation stream items on the dashboard" do
c = User.create.initiate_conversation([@user.id, User.create.id])
c = User.create.initiate_conversation([@user, User.create])
c.add_message('test')
c.add_participants([User.create.id])
c.add_participants([User.create])
items = @user.stream_item_instances
items.size.should == 1

2
spec/spec_helper.rb
View File

@ -617,7 +617,7 @@ Spec::Runner.configure do |config|
def conversation(*users)
options = users.last.is_a?(Hash) ? users.pop : {}
@conversation = (options.delete(:sender) || @me || users.shift).initiate_conversation(users.map(&:id))
@conversation = (options.delete(:sender) || @me || users.shift).initiate_conversation(users)
@message = @conversation.add_message('test')
@conversation.update_attributes(options)
@conversation.reload