Commit 359d362b authored by Jon Jensen's avatar Jon Jensen Committed by Zach Wily
Browse files

ensure conversations with deleted users load correctly

fixes regression from #7471

test plan:
1. start a conversation with another one or more users
2. delete one of the users
3. ensure the conversation still loads (you should see all participants
   and messages)

Change-Id: Ic0e567f22010864054f7396553b43173110be317
Reviewed-on: https://gerrit.instructure.com/9211

Tested-by: default avatarHudson <hudson@instructure.com>
Reviewed-by: default avatarCameron Matheson <cameron@instructure.com>
parent 5cab4520
......@@ -124,7 +124,7 @@ class ConversationParticipant < ActiveRecord::Base
end
return participants unless options[:include_context_info]
# we do this to find out the contexts they share with the user
user.messageable_users(:ids => participants.map(&:id), :no_check_context => true).each { |user|
user.messageable_users(:ids => participants.map(&:id), :skip_visibility_checks => true).each { |user|
context_info[user.id] = user
}
participants.each { |user|
......
......@@ -1999,7 +1999,8 @@ class User < ActiveRecord::Base
# bother doing a query that's guaranteed to return no results.
return [] if options[:ids] && options[:ids].empty?
user_conditions = [messageable_user_clause]
user_conditions = []
user_conditions << [messageable_user_clause] unless options[:skip_visibility_checks] && options[:ids]
user_conditions << "users.id IN (#{options[:ids].map(&:to_i).join(', ')})" if options[:ids].present?
user_conditions << "users.id NOT IN (#{options[:exclude_ids].map(&:to_i).join(', ')})" if options[:exclude_ids].present?
if options[:search] && (parts = options[:search].strip.split(/\s+/)).present?
......@@ -2072,7 +2073,7 @@ class User < ActiveRecord::Base
AND conversation_participants.conversation_id = #{options[:conversation_id].to_i}
#{user_condition_sql}
SQL
elsif options[:no_check_context]
elsif options[:skip_visibility_checks] # we don't care about the contexts, we've passed in ids
user_sql << <<-SQL
SELECT #{MESSAGEABLE_USER_COLUMN_SQL}, NULL AS course_id, NULL AS group_id, NULL AS roles
FROM users
......
......@@ -153,6 +153,53 @@ describe ConversationParticipant do
end
end
context "participants" do
before do
@me = course_with_student(:active_all => true).user
@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.add_message "ohai"
@u3.destroy
@u4 = student_in_course(:active_all => true).user
other_convo = @u4.initiate_conversation([@me.id])
message = other_convo.add_message "just between you and me"
@convo.add_message("haha i forwarded it", :forwarded_message_ids => [message.id])
end
it "should include shared contexts by default" do
users = @convo.reload.participants
users.each do |user|
user.common_groups.should == {}
if [@me.id, @u3.id].include? user.id
user.common_courses.should == {}
else
user.common_courses.should == {@course.id => ["StudentEnrollment"]}
end
end
end
it "should not include forwarded participants by default" do
users = @convo.reload.participants
users.map(&:id).sort.should eql [@me.id, @u1.id, @u2.id, @u3.id]
end
it "should not include shared contexts if asked not to" do
users = @convo.reload.participants(:include_context_info => false)
users.each do |user|
user.common_groups.should be_nil
user.common_courses.should be_nil
end
end
it "should include include forwarded participants if requested" do
users = @convo.reload.participants(:include_indirect_participants => true)
users.map(&:id).sort.should eql [@me.id, @u1.id, @u2.id, @u3.id, @u4.id]
end
end
context "move_to_user" do
before do
@user1 = user_model
......
......@@ -708,6 +708,14 @@ describe User do
set_up_course_with_users
@student.messageable_users.map(&:id).should_not include(@deleted_user.id)
@student.messageable_users(:search => @deleted_user.name).map(&:id).should be_empty
@student.messageable_users(:ids => [@deleted_user.id]).map(&:id).should be_empty
@student.messageable_users(:skip_visibility_checks => true).map(&:id).should_not include(@deleted_user.id)
@student.messageable_users(:skip_visibility_checks => true, :search => @deleted_user.name).map(&:id).should be_empty
end
it "should include deleted iff skip_visibility_checks=true && ids are given" do
set_up_course_with_users
@student.messageable_users(:skip_visibility_checks => true, :ids => [@deleted_user.id]).map(&:id).should == [@deleted_user.id]
end
it "should only include users from the specified section" do
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment