user co-enrollment backend work, closes #8588
also fixed user merging so that observer enrollments get merged when merging observed users test plan: 1. test observer enrollment creation/updating 1. enroll a student in some courses 2. create another user 3. add that user as an observer of the student (via console) 4. confirm that the user gets enrolled as an observer in the student's courses 5. enroll the student in another course 6. confirm that the observer is also enrolled 7. conclude the student's enrollment in a course 8. confirm that the observer's enrollment is also concluded 2. test observer enrollment consolidation (via user merge) 1. observe two users in a course 2. merge the users 3. confirm that you just have one observer enrollment after the merge 3. test observer enrollment creation (via user merge) 1. enroll user 1 in course A 2. enroll user 2 in course B 3. observe user 1 4. merge the users (either direction) 5. confirm that you are now observing the user in both courses Change-Id: Ic5ab29157ab8fc8dc9e7b32cafebbb1290bd4f6b Reviewed-on: https://gerrit.instructure.com/10811 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Cameron Matheson <cameron@instructure.com>
This commit is contained in:
parent
4eeb2cca04
commit
a1844e4414
|
@ -36,8 +36,10 @@ class Enrollment < ActiveRecord::Base
|
|||
before_save :assert_section
|
||||
before_save :update_user_account_associations_if_necessary
|
||||
before_save :audit_groups_for_deleted_enrollments
|
||||
after_create :create_linked_enrollments
|
||||
after_save :clear_email_caches
|
||||
after_save :cancel_future_appointments
|
||||
after_save :update_linked_enrollments
|
||||
|
||||
attr_accessible :user, :course, :workflow_state, :course_section, :limit_priveleges_to_course_section, :limit_privileges_to_course_section
|
||||
|
||||
|
@ -236,6 +238,55 @@ class Enrollment < ActiveRecord::Base
|
|||
end
|
||||
protected :audit_groups_for_deleted_enrollments
|
||||
|
||||
def observers
|
||||
student? ? user.observers : []
|
||||
end
|
||||
|
||||
def create_linked_enrollments
|
||||
observers.each do |observer|
|
||||
create_linked_enrollment_for(observer)
|
||||
end
|
||||
end
|
||||
|
||||
def update_linked_enrollments
|
||||
observers.each do |observer|
|
||||
if enrollment = linked_enrollment_for(observer)
|
||||
enrollment.update_from(self)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def create_linked_enrollment_for(observer)
|
||||
enrollment = linked_enrollment_for(observer) || observer.observer_enrollments.build
|
||||
enrollment.associated_user_id = user_id
|
||||
enrollment.update_from(self)
|
||||
end
|
||||
|
||||
def linked_enrollment_for(observer)
|
||||
# there should really only ever be one, but due to SIS or legacy data there
|
||||
# could be multiple. we'll use the best match (based on workflow_state)
|
||||
enrollment = observer.observer_enrollments.find :first, :conditions => {
|
||||
:associated_user_id => user_id,
|
||||
:course_id => course_id,
|
||||
:course_section_id => course_section_id_was
|
||||
}, :order => self.class.state_rank_sql
|
||||
# we don't want to "undelete" observer enrollments that have been
|
||||
# explicitly deleted
|
||||
return nil if enrollment && enrollment.deleted? && workflow_state_was != 'deleted'
|
||||
enrollment
|
||||
end
|
||||
|
||||
def update_from(other)
|
||||
self.course_id = other.course_id
|
||||
self.workflow_state = other.workflow_state
|
||||
self.start_at = other.start_at
|
||||
self.end_at = other.end_at
|
||||
self.course_section_id = other.course_section_id
|
||||
self.root_account_id = other.root_account_id
|
||||
self.user.touch if workflow_state_changed?
|
||||
save!
|
||||
end
|
||||
|
||||
def clear_email_caches
|
||||
if self.workflow_state_changed? && (self.workflow_state_was == 'invited' || self.workflow_state == 'invited')
|
||||
self.user.communication_channels.email.unretired.each { |cc| Rails.cache.delete([cc.path, 'invited_enrollments'].cache_key)}
|
||||
|
@ -826,6 +877,23 @@ class Enrollment < ActiveRecord::Base
|
|||
return deleted
|
||||
end
|
||||
|
||||
# similar to above, but used on a scope or association (e.g. User#enrollments)
|
||||
def self.remove_duplicates!
|
||||
scope = current_scoped_methods && current_scoped_methods[:find]
|
||||
raise "remove_duplicates! needs to be scoped" unless scope && scope[:conditions]
|
||||
|
||||
where(["workflow_state NOT IN (?)", ['deleted', 'inactive', 'rejected']]).
|
||||
group_by{ |e| [e.user_id, e.course_id, e.course_section_id, e.associated_user_id] }.
|
||||
each do |key, enrollments|
|
||||
next if enrollments.size == 1
|
||||
enrollments.
|
||||
sort_by{ |e| [e.sis_batch_id || ''] + [-e.state_sortable] }.
|
||||
reverse.
|
||||
slice(1, enrollments.size - 1).
|
||||
each(&:destroy)
|
||||
end
|
||||
end
|
||||
|
||||
def effective_start_at
|
||||
# try and use the enrollment dates logic first, since it knows about
|
||||
# overrides, etc. but if it doesn't find anything, start guessing by
|
||||
|
|
|
@ -44,6 +44,12 @@ class User < ActiveRecord::Base
|
|||
( enrollments.workflow_state = 'invited' and ((courses.workflow_state = 'available' and (enrollments.type = 'StudentEnrollment' or enrollments.type = 'ObserverEnrollment')) or (courses.workflow_state != 'deleted' and (enrollments.type IN ('TeacherEnrollment', 'TaEnrollment', 'DesignerEnrollment', 'StudentViewEnrollment')))) )"
|
||||
has_many :not_ended_enrollments, :class_name => 'Enrollment', :conditions => ["enrollments.workflow_state NOT IN (?)", ['rejected', 'completed', 'deleted']]
|
||||
has_many :concluded_enrollments, :class_name => 'Enrollment', :include => [:course, :course_section], :conditions => "enrollments.workflow_state = 'completed'", :order => 'enrollments.created_at'
|
||||
has_many :observer_enrollments
|
||||
has_many :observee_enrollments, :foreign_key => :associated_user_id, :class_name => 'ObserverEnrollment'
|
||||
has_many :user_observers, :dependent => :delete_all
|
||||
has_many :observers, :through => :user_observers, :class_name => 'User'
|
||||
has_many :user_observees, :class_name => 'UserObserver', :foreign_key => :observer_id, :dependent => :delete_all
|
||||
has_many :observed_users, :through => :user_observees, :source => :user
|
||||
has_many :courses, :through => :current_enrollments, :uniq => true
|
||||
has_many :current_and_invited_courses, :source => :course, :through => :current_and_invited_enrollments
|
||||
has_many :concluded_courses, :source => :course, :through => :concluded_enrollments, :uniq => true
|
||||
|
@ -817,6 +823,19 @@ class User < ActiveRecord::Base
|
|||
logger.error "migrating #{table} column #{column} failed: #{e.to_s}"
|
||||
end
|
||||
end
|
||||
# delete duplicate enrollments where this user is the observee
|
||||
new_user.observee_enrollments.remove_duplicates!
|
||||
|
||||
# delete duplicate observers/observees, move the rest
|
||||
user_observees.where(:user_id => new_user.user_observees.map(&:user_id)).delete_all
|
||||
user_observees.update_all(:observer_id => new_user.id)
|
||||
xor_observer_ids = (Set.new(user_observers.map(&:observer_id)) ^ new_user.user_observers.map(&:observer_id)).to_a
|
||||
user_observers.where(:observer_id => new_user.user_observers.map(&:observer_id)).delete_all
|
||||
user_observers.update_all(:user_id => new_user.id)
|
||||
# for any observers not already watching both users, make sure they have
|
||||
# any missing observer enrollments added
|
||||
new_user.user_observers.where(:observer_id => xor_observer_ids).each(&:create_linked_enrollments)
|
||||
|
||||
self.reload
|
||||
Enrollment.send_later(:recompute_final_scores, new_user.id)
|
||||
new_user.update_account_associations
|
||||
|
|
|
@ -0,0 +1,31 @@
|
|||
#
|
||||
# Copyright (C) 2012 Instructure, Inc.
|
||||
#
|
||||
# This file is part of Canvas.
|
||||
#
|
||||
# Canvas is free software: you can redistribute it and/or modify it under
|
||||
# the terms of the GNU Affero General Public License as published by the Free
|
||||
# Software Foundation, version 3 of the License.
|
||||
#
|
||||
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
|
||||
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
|
||||
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
|
||||
# details.
|
||||
#
|
||||
# You should have received a copy of the GNU Affero General Public License along
|
||||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
|
||||
class UserObserver < ActiveRecord::Base
|
||||
belongs_to :user
|
||||
belongs_to :observer, :class_name => 'User'
|
||||
attr_accessible
|
||||
|
||||
after_create :create_linked_enrollments
|
||||
|
||||
def create_linked_enrollments
|
||||
user.student_enrollments.active_or_pending.each do |enrollment|
|
||||
enrollment.create_linked_enrollment_for(observer)
|
||||
end
|
||||
end
|
||||
end
|
|
@ -379,6 +379,10 @@ class ActiveRecord::Base
|
|||
{:order => order_by}
|
||||
}
|
||||
|
||||
named_scope :where, lambda { |conditions|
|
||||
{:conditions => conditions}
|
||||
}
|
||||
|
||||
# set up class-specific getters/setters for a polymorphic association, e.g.
|
||||
# belongs_to :context, :polymorphic => true, :types => [:course, :account]
|
||||
def self.belongs_to(name, options={})
|
||||
|
|
|
@ -0,0 +1,25 @@
|
|||
class AddUserObservers < ActiveRecord::Migration
|
||||
self.transactional = false
|
||||
tag :predeploy
|
||||
|
||||
def self.up
|
||||
create_table :user_observers do |t|
|
||||
t.integer :user_id, :limit => 8, :null => false
|
||||
t.integer :observer_id, :limit => 8, :null => false
|
||||
end
|
||||
add_index :user_observers, [:user_id, :observer_id], :unique => true
|
||||
add_index :user_observers, :observer_id
|
||||
|
||||
# User#move_to_user already needed this, and now we do a second query there
|
||||
if connection.adapter_name =~ /\Apostgresql/i
|
||||
execute('CREATE INDEX CONCURRENTLY "index_enrollments_on_associated_user_id" ON enrollments(associated_user_id) WHERE associated_user_id IS NOT NULL')
|
||||
else
|
||||
add_index :enrollments, [:associated_user_id], :name => "index_enrollments_on_associated_user_id"
|
||||
end
|
||||
end
|
||||
|
||||
def self.down
|
||||
drop_table :user_observers
|
||||
remove_index :enrollments, :name => "index_enrollments_on_associated_user_id"
|
||||
end
|
||||
end
|
|
@ -1291,4 +1291,74 @@ describe Enrollment do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'observing users' do
|
||||
before do
|
||||
@student = user(:active_all => true)
|
||||
@parent = user(:active_all => true)
|
||||
@student.observers << @parent
|
||||
end
|
||||
|
||||
it 'should get new observer enrollments when an observed user gets a new enrollment' do
|
||||
se = course_with_student(:active_all => true, :user => @student)
|
||||
pe = @parent.observer_enrollments.first
|
||||
|
||||
pe.should_not be_nil
|
||||
pe.course_id.should eql se.course_id
|
||||
pe.course_section_id.should eql se.course_section_id
|
||||
pe.workflow_state.should eql se.workflow_state
|
||||
pe.associated_user_id.should eql se.user_id
|
||||
end
|
||||
|
||||
it 'should have their observer enrollments updated when an observed user\'s enrollment is updated' do
|
||||
se = course_with_student(:user => @student)
|
||||
pe = @parent.observer_enrollments.first
|
||||
pe.should_not be_nil
|
||||
|
||||
se.invite
|
||||
se.accept
|
||||
pe.reload.should be_active
|
||||
|
||||
se.complete
|
||||
pe.reload.should be_completed
|
||||
end
|
||||
|
||||
it 'should update the best observer enrollment if there are duplicates' do
|
||||
se = course_with_student(:user => @student)
|
||||
pe = @parent.observer_enrollments.first
|
||||
pe.should_not be_nil
|
||||
|
||||
pe.destroy
|
||||
pe2 = @parent.observer_enrollments.build
|
||||
pe2.course_id = pe.course_id
|
||||
pe2.course_section_id = pe.course_section_id
|
||||
pe2.associated_user_id = pe.associated_user_id
|
||||
pe2.save!
|
||||
|
||||
se.invite
|
||||
pe.reload.should be_deleted
|
||||
pe2.reload.should be_invited
|
||||
|
||||
se.accept
|
||||
pe.reload.should be_deleted
|
||||
pe2.reload.should be_active
|
||||
|
||||
se.complete
|
||||
pe.reload.should be_deleted
|
||||
pe2.reload.should be_completed
|
||||
end
|
||||
|
||||
it 'should not undelete observer enrollments if the student enrollment wasn\'t already deleted' do
|
||||
se = course_with_student(:user => @student)
|
||||
pe = @parent.observer_enrollments.first
|
||||
pe.should_not be_nil
|
||||
pe.destroy
|
||||
|
||||
se.invite
|
||||
pe.reload.should be_deleted
|
||||
|
||||
se.accept
|
||||
pe.reload.should be_deleted
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,40 @@
|
|||
#
|
||||
# Copyright (C) 2012 Instructure, Inc.
|
||||
#
|
||||
# This file is part of Canvas.
|
||||
#
|
||||
# Canvas is free software: you can redistribute it and/or modify it under
|
||||
# the terms of the GNU Affero General Public License as published by the Free
|
||||
# Software Foundation, version 3 of the License.
|
||||
#
|
||||
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
|
||||
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
|
||||
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
|
||||
# details.
|
||||
#
|
||||
# You should have received a copy of the GNU Affero General Public License along
|
||||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
|
||||
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
|
||||
|
||||
describe UserObserver do
|
||||
it "should enroll the observer in all pending/active courses" do
|
||||
student = user
|
||||
c1 = course(:active_all => true)
|
||||
e1 = student_in_course(:course => c1, :user => student)
|
||||
c2 = course(:active_all => true)
|
||||
e2 = student_in_course(:active_all => true, :course => c2, :user => student)
|
||||
c3 = course(:active_all => true)
|
||||
e3 = student_in_course(:active_all => true, :course => c3, :user => student)
|
||||
e3.complete!
|
||||
|
||||
observer = user
|
||||
student.observers << observer
|
||||
|
||||
enrollments = observer.observer_enrollments.sort_by(&:course_id)
|
||||
enrollments.size.should eql 2
|
||||
enrollments.map(&:course_id).should eql [c1.id, c2.id]
|
||||
enrollments.map(&:workflow_state).should eql ["invited", "active"]
|
||||
end
|
||||
end
|
|
@ -548,6 +548,61 @@ describe User do
|
|||
@user1.enrollments.should be_empty
|
||||
end
|
||||
|
||||
it "should move and uniquify observee enrollments" do
|
||||
@user1 = user_model
|
||||
@course1 = course(:active_all => 1)
|
||||
@enrollment1 = @course1.enroll_user(@user1)
|
||||
@user2 = user_model
|
||||
@course2 = course(:active_all => 1)
|
||||
@enrollment2 = @course1.enroll_user(@user2)
|
||||
|
||||
@observer1 = user_model
|
||||
@observer2 = user_model
|
||||
@user1.observers << @observer1 << @observer2
|
||||
@user2.observers << @observer2
|
||||
ObserverEnrollment.count.should eql 3
|
||||
|
||||
@user1.move_to_user(@user2)
|
||||
|
||||
@user1.observee_enrollments.should be_empty
|
||||
@user2.observee_enrollments.size.should eql 3 # 1 deleted
|
||||
@user2.observee_enrollments.active_or_pending.size.should eql 2
|
||||
@observer1.observer_enrollments.active_or_pending.size.should eql 1
|
||||
@observer2.observer_enrollments.active_or_pending.size.should eql 1
|
||||
end
|
||||
|
||||
it "should move and uniquify observers" do
|
||||
@user1 = user_model
|
||||
@user2 = user_model
|
||||
@observer1 = user_model
|
||||
@observer2 = user_model
|
||||
@user1.observers << @observer1 << @observer2
|
||||
@user2.observers << @observer2
|
||||
|
||||
@user1.move_to_user(@user2)
|
||||
|
||||
@user1.reload
|
||||
@user1.observers.should be_empty
|
||||
@user2.reload
|
||||
@user2.observers.sort_by(&:id).should eql [@observer1, @observer2]
|
||||
end
|
||||
|
||||
it "should move and uniquify observed users" do
|
||||
@user1 = user_model
|
||||
@user2 = user_model
|
||||
@student1 = user_model
|
||||
@student2 = user_model
|
||||
@user1.observed_users << @student1 << @student2
|
||||
@user2.observed_users << @student2
|
||||
|
||||
@user1.move_to_user(@user2)
|
||||
|
||||
@user1.reload
|
||||
@user1.observed_users.should be_empty
|
||||
@user2.reload
|
||||
@user2.observed_users.sort_by(&:id).should eql [@student1, @student2]
|
||||
end
|
||||
|
||||
it "should update account associations" do
|
||||
@account1 = account_model
|
||||
@account2 = account_model
|
||||
|
|
Loading…
Reference in New Issue