update conversation tags when new courses exist.
fixes CNVS-620
test plan:
* create a course with at least two enrollees (A and B);
* as enrollee A, send a message to enrollee B;
* conclude the course, create a new course, and enroll
users A and B in it;
* as enrollee A, send a message to enrollee B and verify
that the message's tags update to include the new
course.
Change-Id: I36332c84a9602aee31b2e2ee1e73b12e87462e38
Reviewed-on: https://gerrit.instructure.com/20221
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jon Willesen <jonw@instructure.com>
Product-Review: Marc LeGendre <marc@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
This commit is contained in:
Zach Pendleton
parent
ea7bf11883
commit
8f5f5bbc7d
|
|
@ -349,7 +349,13 @@ class Conversation < ActiveRecord::Base
|
|||
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
|
||||
updated_tags = if (active_tags = cp.user.conversation_context_codes(false)).present?
|
||||
(cp.tags | new_tags) & active_tags
|
||||
else
|
||||
cp.tags | new_tags
|
||||
end
|
||||
|
||||
cp.update_attribute(:tags, updated_tags)
|
||||
if cp.user.shard != self.shard
|
||||
cp.user.shard.activate do
|
||||
ConversationParticipant.where(:conversation_id => self, :user_id => cp.user_id).update_all(:tags => serialized_tags(cp.tags))
|
||||
|
|
@ -369,25 +375,23 @@ class Conversation < ActiveRecord::Base
|
|||
end
|
||||
end
|
||||
|
||||
def infer_new_tags_for(cp, all_new_tags)
|
||||
new_tags = []
|
||||
if all_new_tags.present?
|
||||
# limit it to what they can see
|
||||
new_tags = all_new_tags & cp.user.conversation_context_codes
|
||||
end
|
||||
# if they don't have any tags yet (e.g. this is the first message) and
|
||||
# there are no new tags, just get the best possible match(es)
|
||||
if new_tags.empty? && cp.tags.empty?
|
||||
new_tags = current_context_strings & cp.user.conversation_context_codes
|
||||
def infer_new_tags_for(participant, all_new_tags)
|
||||
active_tags = participant.user.conversation_context_codes(false)
|
||||
context_codes = active_tags.present? ? active_tags : participant.user.conversation_context_codes
|
||||
visible_codes = all_new_tags & context_codes
|
||||
|
||||
new_tags = if visible_codes.present?
|
||||
# limit available codes to codes the user can see
|
||||
visible_codes
|
||||
else
|
||||
# otherwise, use all of the available tags.
|
||||
current_context_strings & context_codes
|
||||
end
|
||||
|
||||
# see ConversationParticipant#update_cached_data ... tags are only
|
||||
# recomputed for private conversations, so for group ones we don't bother
|
||||
# tracking at the message level
|
||||
message_tags = if private?
|
||||
message_tags = if self.private?
|
||||
if new_tags.present?
|
||||
new_tags
|
||||
elsif cp.message_count > 0 and last_message = cp.last_message
|
||||
elsif participant.message_count > 0 and last_message = participant.last_message
|
||||
last_message.tags
|
||||
end
|
||||
end
|
||||
|
|
@ -665,7 +669,6 @@ class Conversation < ActiveRecord::Base
|
|||
value > threshold
|
||||
}.sort_by(&:last).map(&:first).reverse
|
||||
end
|
||||
memoize :current_context_strings
|
||||
|
||||
def associated_shards
|
||||
[Shard.default]
|
||||
|
|
|
|||
|
|
@ -1887,13 +1887,21 @@ class User < ActiveRecord::Base
|
|||
end
|
||||
memoize :manageable_appointment_context_codes
|
||||
|
||||
def conversation_context_codes
|
||||
Rails.cache.fetch([self, 'conversation_context_codes4'].cache_key, :expires_in => 1.day) do
|
||||
# Public: Return an array of context codes this user belongs to.
|
||||
#
|
||||
# include_concluded_codes - If true, include concluded courses (default: true).
|
||||
#
|
||||
# Returns an array of context code strings.
|
||||
def conversation_context_codes(include_concluded_codes = true)
|
||||
Rails.cache.fetch([self, include_concluded_codes, 'conversation_context_codes4'].cache_key, :expires_in => 1.day) do
|
||||
Shard.default.activate do
|
||||
( courses.with_each_shard.map{ |c| "course_#{c.id}" } +
|
||||
concluded_courses.with_each_shard.map{ |c| "course_#{c.id}" } +
|
||||
current_groups.with_each_shard.map{ |g| "group_#{g.id}"}
|
||||
).uniq
|
||||
associations = %w{courses concluded_courses current_groups}
|
||||
associations.slice!(1) unless include_concluded_codes
|
||||
|
||||
associations.inject([]) do |result, association|
|
||||
association_type = association.split('_')[-1].slice(0..-2)
|
||||
result.concat(send(association).with_each_shard.map { |x| "#{association_type}_#{x.id}" })
|
||||
end.uniq
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -682,14 +682,14 @@ describe Conversation do
|
|||
u3 = student_in_course(:active_all => true, :course => @course2).user
|
||||
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]
|
||||
u3.conversations.first.tags.should eql [@course2.asset_string]
|
||||
u1.conversations.first.tags.should == [@course1.asset_string]
|
||||
u2.conversations.first.tags.should == [@course1.asset_string]
|
||||
u3.conversations.first.tags.should == [@course2.asset_string]
|
||||
|
||||
conversation.add_message(u1, 'another', :tags => [@course2.asset_string, "course_0"])
|
||||
u1.conversations.first.tags.should eql [@course1.asset_string]
|
||||
u2.conversations.first.tags.sort.should eql [@course1.asset_string, @course2.asset_string].sort
|
||||
u3.conversations.first.tags.should eql [@course2.asset_string]
|
||||
u1.conversations.first.tags.should == [@course1.asset_string]
|
||||
u2.conversations.first.tags.sort.should == [@course1.asset_string, @course2.asset_string].sort
|
||||
u3.conversations.first.tags.should == [@course2.asset_string]
|
||||
end
|
||||
|
||||
it "should ignore conversation_participants without a valid user" do
|
||||
|
|
@ -701,18 +701,18 @@ describe Conversation do
|
|||
u3 = student_in_course(:active_all => true, :course => @course2).user
|
||||
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]
|
||||
u3.conversations.first.tags.should eql [@course2.asset_string]
|
||||
u1.conversations.first.tags.should == [@course1.asset_string]
|
||||
u2.conversations.first.tags.should == [@course1.asset_string]
|
||||
u3.conversations.first.tags.should == [@course2.asset_string]
|
||||
broken_one = u3.conversations.first
|
||||
broken_one.user_id = nil
|
||||
broken_one.tags = []
|
||||
broken_one.save!
|
||||
|
||||
conversation.add_message(u1, 'another', :tags => [@course2.asset_string, "course_0"])
|
||||
u1.conversations.first.tags.should eql [@course1.asset_string]
|
||||
u2.conversations.first.tags.sort.should eql [@course1.asset_string, @course2.asset_string].sort
|
||||
broken_one.reload.tags.should eql []
|
||||
u1.conversations.first.tags.should == [@course1.asset_string]
|
||||
u2.conversations.first.tags.sort.should == [@course1.asset_string]
|
||||
broken_one.reload.tags.should == []
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -866,6 +866,136 @@ describe Conversation do
|
|||
broken_one.reload.tags.should eql [] # skipped
|
||||
end
|
||||
end
|
||||
|
||||
context 'tag updates' do
|
||||
before(:each) do
|
||||
@teacher = teacher_in_course(:active_all => true).user
|
||||
@student = student_in_course(:active_all => true, :course => @course).user
|
||||
@old_course = @course
|
||||
end
|
||||
|
||||
it "should remove old tags and add new ones" do
|
||||
conversation = Conversation.initiate([@teacher, @student], true)
|
||||
conversation.add_message(@teacher, 'first message')
|
||||
|
||||
new_course = course
|
||||
new_course.offer!
|
||||
new_course.enroll_teacher(@teacher).accept!
|
||||
new_course.enroll_student(@student).accept!
|
||||
|
||||
@old_course.complete!
|
||||
|
||||
third_course = course
|
||||
third_course.offer!
|
||||
third_course.enroll_teacher(@teacher).accept!
|
||||
|
||||
conversation.add_message(@student, 'second message')
|
||||
|
||||
conversation.conversation_participants.each do |participant|
|
||||
participant.reload
|
||||
participant.tags.should == [new_course.asset_string]
|
||||
end
|
||||
end
|
||||
|
||||
it "should continue to use old tags if there are no current shared contexts" do
|
||||
conversation = Conversation.initiate([@teacher, @student], true)
|
||||
conversation.add_message(@teacher, 'first message')
|
||||
|
||||
@old_course.complete!
|
||||
|
||||
teacher_course = course
|
||||
teacher_course.offer!
|
||||
teacher_course.enroll_teacher(@teacher).accept!
|
||||
|
||||
student_course = course
|
||||
student_course.offer!
|
||||
student_course.enroll_student(@student).accept!
|
||||
|
||||
conversation.add_message(@student, 'second message')
|
||||
|
||||
conversation.conversation_participants.each do |participant|
|
||||
participant.reload
|
||||
participant.tags.should == [@old_course.asset_string]
|
||||
end
|
||||
end
|
||||
|
||||
it "should use concluded tags from multiple courses" do
|
||||
old_course2 = course
|
||||
|
||||
old_course2.offer!
|
||||
old_course2.enroll_teacher(@teacher).accept!
|
||||
old_course2.enroll_student(@student).accept!
|
||||
|
||||
conversation = Conversation.initiate([@teacher, @student], true)
|
||||
conversation.add_message(@teacher, 'first message')
|
||||
|
||||
[@old_course, old_course2].each { |c| c.complete! }
|
||||
|
||||
teacher_course = course
|
||||
teacher_course.offer!
|
||||
teacher_course.enroll_teacher(@teacher).accept!
|
||||
|
||||
student_course = course
|
||||
student_course.offer!
|
||||
student_course.enroll_student(@student).accept!
|
||||
|
||||
conversation.add_message(@teacher, 'second message')
|
||||
|
||||
conversation.conversation_participants.each do |participant|
|
||||
participant.reload
|
||||
participant.tags.sort.should == [@old_course, old_course2].map(&:asset_string).sort
|
||||
end
|
||||
end
|
||||
|
||||
it "should include concluded group contexts when no active ones exist" do
|
||||
student1 = student_in_course(:active_all => true, :course => @old_course).user
|
||||
student2 = student_in_course(:active_all => true, :course => @old_course).user
|
||||
|
||||
group = Group.create!(:context => @old_course)
|
||||
[student1, student2].each { |s| group.users << s }
|
||||
|
||||
conversation = Conversation.initiate([student1, student2], true)
|
||||
conversation.add_message(student1, 'first message')
|
||||
|
||||
@old_course.complete!
|
||||
group.complete!
|
||||
|
||||
conversation.add_message(student2, 'second message')
|
||||
|
||||
conversation.conversation_participants.each do |participant|
|
||||
participant.reload
|
||||
participant.tags.should include(group.asset_string)
|
||||
end
|
||||
end
|
||||
|
||||
it "should replace concluded group contexts with active ones" do
|
||||
student1 = student_in_course(:active_all => true, :course => @old_course).user
|
||||
student2 = student_in_course(:active_all => true, :course => @old_course).user
|
||||
|
||||
old_group = Group.create!(:context => @old_course)
|
||||
[student1, student2].each { |s| old_group.users << s }
|
||||
|
||||
conversation = Conversation.initiate([student1, student2], true)
|
||||
conversation.add_message(student1, 'first message')
|
||||
|
||||
@old_course.complete!
|
||||
old_group.destroy
|
||||
|
||||
new_course = course
|
||||
new_course.offer!
|
||||
[student1, student2].each { |s| new_course.enroll_student(s).accept! }
|
||||
new_group = Group.create!(:context => new_course)
|
||||
new_group.users << student1
|
||||
new_group.users << student2
|
||||
|
||||
conversation.add_message(student2, 'second message')
|
||||
|
||||
conversation.conversation_participants.each do |participant|
|
||||
participant.reload
|
||||
participant.tags.sort.should == [new_group, new_course].map(&:asset_string).sort
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "root_account_ids" do
|
||||
|
|
|
|||
|
|
@ -2219,6 +2219,12 @@ describe User do
|
|||
@user.conversation_context_codes.should include(@course.asset_string)
|
||||
end
|
||||
|
||||
it "should optionally not include concluded courses" do
|
||||
course_with_student(:user => @user, :active_all => true)
|
||||
@enrollment.update_attribute(:workflow_state, 'completed')
|
||||
@user.conversation_context_codes(false).should_not include(@course.asset_string)
|
||||
end
|
||||
|
||||
it "should include groups" do
|
||||
group_with_user(:user => @user, :active_all => true)
|
||||
@user.conversation_context_codes.should include(@group.asset_string)
|
||||
|
|
@ -2243,6 +2249,12 @@ describe User do
|
|||
@user.conversation_context_codes.should include(@course.asset_string)
|
||||
end
|
||||
|
||||
it "should optionally not include concluded courses on other shards" do
|
||||
course_with_student(:account => @shard1_account, :user => @user, :active_all => true)
|
||||
@enrollment.update_attribute(:workflow_state, 'completed')
|
||||
@user.conversation_context_codes(false).should_not include(@course.asset_string)
|
||||
end
|
||||
|
||||
it "should include groups on other shards" do
|
||||
# course is just to associate the get shard1 in @user's associated shards
|
||||
course_with_student(:account => @shard1_account, :user => @user, :active_all => true)
|
||||
|
|
|
|||
Loading…
Reference in New Issue