From 2b0f57c5602cb7557fc122d8c30fd80de3f4d5cb Mon Sep 17 00:00:00 2001 From: James Williams Date: Tue, 13 Mar 2018 07:29:11 -0600 Subject: [PATCH] rename user_observer/observee classes/associations for my sanity because it's too confusing to actually refactor closes #CORE-1140 Change-Id: I445e0edeb3fde76ffd02a467180ee7a83d916067 Reviewed-on: https://gerrit.instructure.com/143538 Tested-by: Jenkins Reviewed-by: Cody Cutrer Product-Review: James Williams QA-Review: James Williams --- .../communication_channels_controller.rb | 4 +- app/controllers/login/saml_controller.rb | 6 +- app/controllers/user_observees_controller.rb | 8 +- app/controllers/users_controller.rb | 4 +- app/models/enrollment.rb | 2 +- app/models/enrollment/batch_state_updater.rb | 2 +- app/models/split_users.rb | 10 +- app/models/user.rb | 28 ++-- app/models/user_merge_data_record.rb | 2 +- app/models/user_observation_link.rb | 140 ++++++++++++++++++ app/models/user_observer.rb | 134 +---------------- app/models/user_profile.rb | 2 +- ...94659_repair_cross_shard_user_observers.rb | 4 +- .../lib/account_reports/sis_exporter.rb | 2 +- .../sis_provisioning_reports_spec.rb | 8 +- .../link_missing_sis_observer_enrollments.rb | 4 +- lib/data_fixup/remove_invalid_observers.rb | 2 +- lib/enrollments_from_user_list.rb | 2 +- lib/sis/user_observer_importer.rb | 6 +- lib/user_merge.rb | 22 +-- spec/apis/v1/assignments_api_spec.rb | 2 +- spec/apis/v1/calendar_events_api_spec.rb | 2 +- spec/apis/v1/courses_api_spec.rb | 6 +- spec/apis/v1/enrollments_api_spec.rb | 2 +- spec/apis/v1/user_observees_api_spec.rb | 87 ++++++----- spec/apis/v1/users_api_spec.rb | 2 +- .../communication_channels_controller_spec.rb | 4 +- spec/controllers/courses_controller_spec.rb | 6 +- spec/controllers/users_controller_spec.rb | 12 +- spec/lib/sis/csv/enrollment_importer_spec.rb | 2 +- .../sis/csv/user_observer_importer_spec.rb | 10 +- spec/lib/user_merge_spec.rb | 24 +-- spec/lib/user_search_spec.rb | 2 +- ...k_missing_sis_observer_enrollments_spec.rb | 2 +- spec/models/enrollment_spec.rb | 8 +- spec/models/split_user_spec.rb | 48 +++--- spec/models/user_observer_spec.rb | 52 +++---- .../student_grades_page_observer_spec.rb | 2 +- 38 files changed, 337 insertions(+), 328 deletions(-) create mode 100644 app/models/user_observation_link.rb diff --git a/app/controllers/communication_channels_controller.rb b/app/controllers/communication_channels_controller.rb index 01a3d26971e..9b4674324e1 100644 --- a/app/controllers/communication_channels_controller.rb +++ b/app/controllers/communication_channels_controller.rb @@ -256,8 +256,8 @@ class CommunicationChannelsController < ApplicationController # load merge opportunities merge_users = cc.merge_candidates merge_users << @current_user if @current_user && !@user.registered? && !merge_users.include?(@current_user) - user_observers = UserObserver.active.where("user_id = ? OR observer_id = ?", @user.id, @user.id) - merge_users = merge_users.reject { |u| user_observers.any?{|uo| uo.user == u || uo.observer == u} } + observer_links = UserObservationLink.active.where("user_id = ? OR observer_id = ?", @user.id, @user.id) + merge_users = merge_users.reject { |u| observer_links.any?{|uo| uo.user == u || uo.observer == u} } # remove users that don't have a pseudonym for this account, or one can't be created merge_users = merge_users.select { |u| u.find_or_initialize_pseudonym_for_account(@root_account, @domain_root_account) } @merge_opportunities = [] diff --git a/app/controllers/login/saml_controller.rb b/app/controllers/login/saml_controller.rb index 8c15b9995e4..59bf4e2d906 100644 --- a/app/controllers/login/saml_controller.rb +++ b/app/controllers/login/saml_controller.rb @@ -358,8 +358,8 @@ class Login::SamlController < ApplicationController def complete_observee_addition(registration_data) observee_unique_id = registration_data[:observee][:unique_id] observee = @domain_root_account.pseudonyms.by_unique_id(observee_unique_id).first.user - unless @current_user.user_observees.where(user_id: observee).exists? - UserObserver.create_or_restore(observee: observee, observer: @current_user) + unless @current_user.as_observer_observation_links.where(user_id: observee).exists? + UserObservationLink.create_or_restore(student: observee, observer: @current_user) @current_user.touch end end @@ -396,7 +396,7 @@ class Login::SamlController < ApplicationController # set the new user (observer) to observe the target user (observee) observee = @domain_root_account.pseudonyms.active.by_unique_id(observee_unique_id).first.user - UserObserver.create_or_restore(observee: observee, observer: user) + UserObservationLink.create_or_restore(student: observee, observer: user) notify_policy = Users::CreationNotifyPolicy.new(false, unique_id: observer_unique_id) notify_policy.dispatch!(user, pseudonym, cc) diff --git a/app/controllers/user_observees_controller.rb b/app/controllers/user_observees_controller.rb index 6c871451654..4b233fcedb0 100644 --- a/app/controllers/user_observees_controller.rb +++ b/app/controllers/user_observees_controller.rb @@ -43,7 +43,7 @@ class UserObserveesController < ApplicationController # @returns [User] def index includes = params[:include] || [] - observed_users = user.observed_users.active_user_observers.active.order_by_sortable_name + observed_users = user.linked_students.active.order_by_sortable_name observed_users = Api.paginate(observed_users, self, api_v1_user_observees_url) render json: users_json(observed_users, @current_user, session,includes ) end @@ -162,7 +162,7 @@ class UserObserveesController < ApplicationController def destroy raise ActiveRecord::RecordNotFound unless has_observee?(observee) - user.user_observees.active.where(user_id: observee).destroy_all + user.as_observer_observation_links.where(user_id: observee).destroy_all render json: user_json(observee, @current_user, session) end @@ -178,13 +178,13 @@ class UserObserveesController < ApplicationController def add_observee(observee) unless has_observee?(observee) - UserObserver.create_or_restore(observee: observee, observer: user) + UserObservationLink.create_or_restore(student: observee, observer: user) user.touch end end def has_observee?(observee) - user.user_observees.active.where(user_id: observee).exists? + user.as_observer_observation_links.where(user_id: observee).exists? end def self_or_admin_permission_check diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 357dcd92eef..1e912c12ec9 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2551,8 +2551,8 @@ class UsersController < ApplicationController @pseudonym.send(:skip_session_maintenance=, true) end @user.save! - if @observee && !@user.user_observees.where(user_id: @observee).exists? - UserObserver.create_or_restore(observee: @observee, observer: @user) + if @observee && !@user.as_observer_observation_links.where(user_id: @observee).exists? + UserObservationLink.create_or_restore(student: @observee, observer: @user) end if notify_policy.is_self_registration? diff --git a/app/models/enrollment.rb b/app/models/enrollment.rb index 2e4a117acb8..793db6f5cd5 100644 --- a/app/models/enrollment.rb +++ b/app/models/enrollment.rb @@ -401,7 +401,7 @@ class Enrollment < ActiveRecord::Base protected :audit_groups_for_deleted_enrollments def observers - student? ? user.observers.active : [] + student? ? user.linked_observers.active : [] end def create_linked_enrollments diff --git a/app/models/enrollment/batch_state_updater.rb b/app/models/enrollment/batch_state_updater.rb index 511f3c67aa5..8b900776509 100644 --- a/app/models/enrollment/batch_state_updater.rb +++ b/app/models/enrollment/batch_state_updater.rb @@ -44,7 +44,7 @@ class Enrollment::BatchStateUpdater Enrollment.transaction do # cache some data before the destroy that is needed after the destroy invited_user_ids = Enrollment.where(id: batch, workflow_state: 'invited').distinct.pluck(:user_id) - students = Enrollment.of_student_type.where(id: batch).preload(user: :observers).to_a + students = Enrollment.of_student_type.where(id: batch).preload(user: :linked_observers).to_a user_course_tuples = Enrollment.where(id: batch).active.select(%i(user_id course_id)).distinct.to_a user_ids = Enrollment.where(id: batch).distinct.pluck(:user_id) courses = Course.where(id: Enrollment.where(id: batch).select(:course_id).distinct).to_a diff --git a/app/models/split_users.rb b/app/models/split_users.rb index e24a869c3ea..285c1150afd 100644 --- a/app/models/split_users.rb +++ b/app/models/split_users.rb @@ -120,7 +120,7 @@ class SplitUsers # user is the old user that is being restored def move_records_to_old_user(source_user, user, records) fix_communication_channels(source_user, user, records.where(context_type: 'CommunicationChannel')) - move_user_observers(source_user, user, records.where(context_type: 'UserObserver', previous_user_id: user)) + move_user_observers(source_user, user, records.where(context_type: ['UserObserver', 'UserObservationLink'], previous_user_id: user)) move_attachments(source_user, user, records.where(context_type: 'Attachment')) enrollment_ids = records.where(context_type: 'Enrollment', previous_user_id: user).pluck(:context_id) enrollments = Enrollment.where(id: enrollment_ids).where.not(user_id: user) @@ -167,11 +167,11 @@ class SplitUsers def move_user_observers(source_user, user, records) # skip when the user observer is between the two users. Just undlete the record - not_obs = UserObserver.where(user_id: [source_user, user], observer_id: [source_user, user]) - obs = UserObserver.where(id: records.pluck(:context_id)).where.not(id: not_obs) + not_obs = UserObservationLink.where(user_id: [source_user, user], observer_id: [source_user, user]) + obs = UserObservationLink.where(id: records.pluck(:context_id)).where.not(id: not_obs) - source_user.user_observers.where(id: obs).update_all(user_id: user.id) - source_user.user_observees.where(id: obs).update_all(observer_id: user.id) + source_user.as_student_observation_links.where(id: obs).update_all(user_id: user.id) + source_user.as_observer_observation_links.where(id: obs).update_all(observer_id: user.id) end def move_attachments(source_user, user, records) diff --git a/app/models/user.rb b/app/models/user.rb index f5b6b79b9a8..01f811272d6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -59,16 +59,15 @@ class User < ActiveRecord::Base has_many :not_removed_enrollments, -> { where.not(workflow_state: ['rejected', 'deleted', 'inactive']) }, class_name: 'Enrollment', multishard: true has_many :observer_enrollments has_many :observee_enrollments, :foreign_key => :associated_user_id, :class_name => 'ObserverEnrollment' - has_many :user_observers, dependent: :destroy, inverse_of: :user - has_many :active_user_observers, -> { where.not(:workflow_state => 'deleted') }, :class_name => "UserObserver", inverse_of: :user - has_many :observers, :through => :active_user_observers, :class_name => 'User' - has_many :user_observees, - class_name: 'UserObserver', - foreign_key: :observer_id, - dependent: :destroy, - inverse_of: :observer - has_many :observed_users, :through => :user_observees, :source => :user + has_many :as_student_observation_links, -> { where.not(:workflow_state => 'deleted') }, class_name: 'UserObservationLink', + foreign_key: :user_id, dependent: :destroy, inverse_of: :student + has_many :as_observer_observation_links, -> { where.not(:workflow_state => 'deleted') }, class_name: 'UserObservationLink', + foreign_key: :observer_id, dependent: :destroy, inverse_of: :observer + + has_many :linked_observers, :through => :as_student_observation_links, :source => :observer, :class_name => 'User' + has_many :linked_students, :through => :as_observer_observation_links, :source => :student, :class_name => 'User' + has_many :all_courses, :source => :course, :through => :enrollments has_many :all_courses_for_active_enrollments, -> { Enrollment.active }, :source => :course, :through => :enrollments has_many :group_memberships, -> { preload(:group) }, dependent: :destroy @@ -196,7 +195,6 @@ class User < ActiveRecord::Base self.from("(#{scopes.join("\nUNION\n")}) users") } scope :active, -> { where("users.workflow_state<>'deleted'") } - scope :active_user_observers, -> { where.not(user_observers: {workflow_state: 'deleted'}) } scope :has_current_student_enrollments, -> do where("EXISTS (?)", @@ -888,8 +886,8 @@ class User < ActiveRecord::Base if root_account == :all # make sure to hit all shards enrollment_scope = self.enrollments.shard(self) - user_observer_scope = self.user_observers.shard(self) - user_observee_scope = self.user_observees.shard(self) + user_observer_scope = self.as_student_observation_links.shard(self) + user_observee_scope = self.as_observer_observation_links.shard(self) pseudonym_scope = self.pseudonyms.active.shard(self) account_users = self.account_users.active.shard(self) has_other_root_accounts = false @@ -900,8 +898,8 @@ class User < ActiveRecord::Base # student view user won't be cross shard, so that will still be the # right shard enrollment_scope = fake_student? ? self.enrollments : root_account.enrollments.where(user_id: self) - user_observer_scope = self.user_observers.shard(self) - user_observee_scope = self.user_observees.shard(self) + user_observer_scope = self.as_student_observation_links.shard(self) + user_observee_scope = self.as_observer_observation_links.shard(self) pseudonym_scope = root_account.pseudonyms.active.where(user_id: self) account_users = root_account.account_users.where(user_id: self).to_a + @@ -1081,7 +1079,7 @@ class User < ActiveRecord::Base end can :reset_mfa - given { |user| user && user.user_observees.active.where(user_id: self.id).exists? } + given { |user| user && user.as_observer_observation_links.where(user_id: self.id).exists? } can :read and can :read_as_parent end diff --git a/app/models/user_merge_data_record.rb b/app/models/user_merge_data_record.rb index 86957a80620..37640428455 100644 --- a/app/models/user_merge_data_record.rb +++ b/app/models/user_merge_data_record.rb @@ -18,7 +18,7 @@ class UserMergeDataRecord < ActiveRecord::Base belongs_to :previous_user, class_name: 'User' belongs_to :user_merge_data - belongs_to :context, polymorphic: [:account_user, :enrollment, :pseudonym,:user_observer, + belongs_to :context, polymorphic: [:account_user, :enrollment, :pseudonym, :user_observer, :user_observation_link, :attachment, :communication_channel, :user_service, :submission, {quiz_submission: 'Quizzes::QuizSubmission'}] diff --git a/app/models/user_observation_link.rb b/app/models/user_observation_link.rb new file mode 100644 index 00000000000..26165487fe7 --- /dev/null +++ b/app/models/user_observation_link.rb @@ -0,0 +1,140 @@ +# +# Copyright (C) 2011 - present 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 . +# + +# if the observer and observee are on different shards, the "primary" record belongs +# on the same shard as the observee, but a duplicate record is also created on the +# other shard + +class UserObservationLink < ActiveRecord::Base + self.table_name = "user_observers" + + belongs_to :student, :class_name => 'User', inverse_of: :as_student_observation_links, :foreign_key => :user_id + belongs_to :observer, :class_name => 'User', inverse_of: :as_observer_observation_links + + after_create :create_linked_enrollments + + validate :not_same_user, :if => lambda { |uo| uo.changed? } + + scope :active, -> { where.not(workflow_state: 'deleted') } + + # shadow_record param is private + def self.create_or_restore(student: , observer: , shadow_record: false) + raise ArgumentError, 'student and observer are required' unless student && observer + shard = shadow_record ? observer.shard : student.shard + result = shard.activate do + self.unique_constraint_retry do + if (uo = self.where(student: student, observer: observer).take) + if uo.workflow_state == 'deleted' + uo.workflow_state = 'active' + uo.sis_batch_id = nil + uo.save! + end + else + uo = create!(student: student, observer: observer) + end + uo + end + end + + if result.primary_record? + # create the shadow record + create_or_restore(student: student, observer: observer, shadow_record: true) if result.cross_shard? + + result.create_linked_enrollments + result.student.touch + end + + result + end + + def user + student + end + + alias_method :destroy_permanently!, :destroy + def destroy + other_record&.destroy + self.workflow_state = 'deleted' + self.save! + remove_linked_enrollments if primary_record? + end + + def not_same_user + self.errors.add(:observer_id, "Cannot observe yourself") if self.user_id == self.observer_id + end + + def create_linked_enrollments + self.class.connection.after_transaction_commit do + User.skip_updating_account_associations do + student.student_enrollments.shard(student).all_active_or_pending.order("course_id").each do |enrollment| + next unless enrollment.valid? + enrollment.create_linked_enrollment_for(observer) + end + + observer.update_account_associations + end + end + end + + def remove_linked_enrollments + observer.observer_enrollments.shard(observer).where(associated_user_id: student).find_each do |enrollment| + enrollment.workflow_state = 'deleted' + enrollment.save! + end + observer.update_account_associations + observer.touch + end + + def cross_shard? + Shard.shard_for(user_id) != Shard.shard_for(observer_id) + end + + def primary_record? + shard == Shard.shard_for(user_id) + end + + private + + def other_record + if cross_shard? + primary_record? ? shadow_record : self + end + end + + def primary_record + if cross_shard? && !primary_record? + Shard.shard_for(user_id).activate do + self.class.where(user_id: user_id, observer_id: observer_id).take! + end + else + self + end + end + + def shadow_record + if !cross_shard? || !primary_record? + self + else + Shard.shard_for(observer_id).activate do + self.class.where(user_id: user_id, observer_id: observer_id).take! + end + end + end +end + +require_dependency 'user_observer' diff --git a/app/models/user_observer.rb b/app/models/user_observer.rb index 3eb5d5d80d9..c9c8c794fbb 100644 --- a/app/models/user_observer.rb +++ b/app/models/user_observer.rb @@ -1,131 +1,3 @@ -# -# Copyright (C) 2011 - present 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 . -# - -# if the observer and observee are on different shards, the "primary" record belongs -# on the same shard as the observee, but a duplicate record is also created on the -# other shard - -class UserObserver < ActiveRecord::Base - belongs_to :user, inverse_of: :user_observees - belongs_to :observer, :class_name => 'User', inverse_of: :user_observers - after_create :create_linked_enrollments - - validate :not_same_user, :if => lambda { |uo| uo.changed? } - - scope :active, -> { where.not(workflow_state: 'deleted') } - - # shadow_record param is private - def self.create_or_restore(observee: , observer: , shadow_record: false) - raise ArgumentError, 'observee and observer are required' unless observee && observer - shard = shadow_record ? observer.shard : observee.shard - result = shard.activate do - UserObserver.unique_constraint_retry do - if (uo = UserObserver.where(user: observee, observer: observer).take) - if uo.workflow_state == 'deleted' - uo.workflow_state = 'active' - uo.sis_batch_id = nil - uo.save! - end - else - uo = create!(user: observee, observer: observer) - end - uo - end - end - - if result.primary_record? - # create the shadow record - create_or_restore(observee: observee, observer: observer, shadow_record: true) if result.cross_shard? - - result.create_linked_enrollments - result.user.touch - end - - result - end - - alias_method :destroy_permanently!, :destroy - def destroy - other_record&.destroy - self.workflow_state = 'deleted' - self.save! - remove_linked_enrollments if primary_record? - end - - def not_same_user - self.errors.add(:observer_id, "Cannot observe yourself") if self.user_id == self.observer_id - end - - def create_linked_enrollments - self.class.connection.after_transaction_commit do - User.skip_updating_account_associations do - user.student_enrollments.shard(user).all_active_or_pending.order("course_id").each do |enrollment| - next unless enrollment.valid? - enrollment.create_linked_enrollment_for(observer) - end - - observer.update_account_associations - end - end - end - - def remove_linked_enrollments - observer.observer_enrollments.shard(observer).where(associated_user_id: user).find_each do |enrollment| - enrollment.workflow_state = 'deleted' - enrollment.save! - end - observer.update_account_associations - observer.touch - end - - def cross_shard? - Shard.shard_for(user_id) != Shard.shard_for(observer_id) - end - - def primary_record? - shard == Shard.shard_for(user_id) - end - - private - - def other_record - if cross_shard? - primary_record? ? shadow_record : self - end - end - - def primary_record - if cross_shard? && !primary_record? - Shard.shard_for(user_id).activate do - UserObserver.where(user_id: user_id, observer_id: observer_id).take! - end - else - self - end - end - - def shadow_record - if !cross_shard? || !primary_record? - self - else - Shard.shard_for(observer_id).activate do - UserObserver.where(user_id: user_id, observer_id: observer_id).take! - end - end - end -end +# to deal with any lingering references (e.g. jobs, plugins) +UserObserver = UserObservationLink +Autoextend.const_added("UserObserver", source: :inherited) diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 5395e1da920..ae0e20bf34e 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -117,7 +117,7 @@ class UserProfile < ActiveRecord::Base end def insert_observer_tabs(tabs, user) - if user&.user_observees&.active&.exists? + if user&.as_observer_observation_links&.active&.exists? tabs << { id: TAB_OBSERVEES, label: I18n.t('#tabs.observees', 'Observing'), diff --git a/db/migrate/20171019194659_repair_cross_shard_user_observers.rb b/db/migrate/20171019194659_repair_cross_shard_user_observers.rb index e4f9b954df3..662b6afee2c 100644 --- a/db/migrate/20171019194659_repair_cross_shard_user_observers.rb +++ b/db/migrate/20171019194659_repair_cross_shard_user_observers.rb @@ -5,10 +5,10 @@ class RepairCrossShardUserObservers < ActiveRecord::Migration[5.0] def up remove_foreign_key :user_observers, column: :observer_id - UserObserver.where("user_id/?<>observer_id/?", Shard::IDS_PER_SHARD, Shard::IDS_PER_SHARD).find_each do |uo| + UserObservationLink.where("user_id/?<>observer_id/?", Shard::IDS_PER_SHARD, Shard::IDS_PER_SHARD).find_each do |uo| # just "restore" it - will automatically create the missing side, and create enrollments that # may not have worked initially - UserObserver.create_or_restore(observer: uo.observer, observee: uo.user) + UserObservationLink.create_or_restore(observer: uo.observer, student: uo.student) end end end diff --git a/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb b/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb index 59d0585df67..88dcbe372b0 100644 --- a/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb +++ b/gems/plugins/account_reports/lib/account_reports/sis_exporter.rb @@ -830,7 +830,7 @@ module AccountReports p2.user_id AS observer_id, user_observers.workflow_state AS ob_state, user_observers.sis_batch_id AS o_batch_id"). - joins("INNER JOIN #{UserObserver.quoted_table_name} ON pseudonyms.user_id=user_observers.user_id + joins("INNER JOIN #{UserObservationLink.quoted_table_name} ON pseudonyms.user_id=user_observers.user_id INNER JOIN #{Pseudonym.quoted_table_name} AS p2 ON p2.user_id=user_observers.observer_id"). where("p2.account_id=pseudonyms.account_id") diff --git a/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb b/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb index 0f775922ce4..38f7e14d77a 100644 --- a/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb +++ b/gems/plugins/account_reports/spec_canvas/sis_provisioning_reports_spec.rb @@ -1257,10 +1257,10 @@ describe "Default Account Reports" do before(:once) do create_an_account create_some_users_with_pseudonyms - @uo1 = UserObserver.create_or_restore(observee: @user1, observer: @user2) - uo2 = UserObserver.create_or_restore(observee: @user3, observer: @user4) - UserObserver.create_or_restore(observee: @user6, observer: @user7) - UserObserver.where(id: [@uo1.id, uo2.id]).update_all(sis_batch_id: @sis.id) + @uo1 = UserObservationLink.create_or_restore(student: @user1, observer: @user2) + uo2 = UserObservationLink.create_or_restore(student: @user3, observer: @user4) + UserObservationLink.create_or_restore(student: @user6, observer: @user7) + UserObservationLink.where(id: [@uo1.id, uo2.id]).update_all(sis_batch_id: @sis.id) end it 'should run user_observer provisioning report' do diff --git a/lib/data_fixup/link_missing_sis_observer_enrollments.rb b/lib/data_fixup/link_missing_sis_observer_enrollments.rb index 25884dcc3d2..87f7ad85193 100644 --- a/lib/data_fixup/link_missing_sis_observer_enrollments.rb +++ b/lib/data_fixup/link_missing_sis_observer_enrollments.rb @@ -17,8 +17,8 @@ module DataFixup::LinkMissingSisObserverEnrollments def self.run - UserObserver.preload(:user, :observer).find_each do |uo| - uo.user.student_enrollments.active_or_pending.where("sis_batch_id IS NOT NULL").each do |enrollment| + UserObservationLink.preload(:student, :observer).find_each do |uo| + uo.student.student_enrollments.active_or_pending.where("sis_batch_id IS NOT NULL").each do |enrollment| if enrollment.linked_enrollment_for(uo.observer).nil? && uo.observer.can_be_enrolled_in_course?(enrollment.course) new_enrollment = uo.observer.observer_enrollments.build new_enrollment.associated_user_id = enrollment.user_id diff --git a/lib/data_fixup/remove_invalid_observers.rb b/lib/data_fixup/remove_invalid_observers.rb index 4d55dbc8d67..25fb304b0d1 100644 --- a/lib/data_fixup/remove_invalid_observers.rb +++ b/lib/data_fixup/remove_invalid_observers.rb @@ -17,7 +17,7 @@ module DataFixup::RemoveInvalidObservers def self.run - bad_observers = UserObserver.where("user_id = observer_id") + bad_observers = UserObservationLink.where("user_id = observer_id") bad_observers.find_ids_in_ranges do |first, last| bad_observers.where(id: first..last).delete_all end diff --git a/lib/enrollments_from_user_list.rb b/lib/enrollments_from_user_list.rb index de76b07b8af..0a178982d61 100644 --- a/lib/enrollments_from_user_list.rb +++ b/lib/enrollments_from_user_list.rb @@ -62,7 +62,7 @@ class EnrollmentsFromUserList end @user_ids_to_touch.uniq.each_slice(100) do |user_ids| User.where(id: user_ids).touch_all - User.where(id: UserObserver.where(user_id: user_ids).select(:observer_id)).touch_all + User.where(id: UserObservationLink.where(user_id: user_ids).select(:observer_id)).touch_all end @enrollments diff --git a/lib/sis/user_observer_importer.rb b/lib/sis/user_observer_importer.rb index 5eef0ec8b4e..fe2703f76cd 100644 --- a/lib/sis/user_observer_importer.rb +++ b/lib/sis/user_observer_importer.rb @@ -32,7 +32,7 @@ module SIS end importer.user_observers_to_update_sis_batch_ids.in_groups_of(1000, false) do |batch| - UserObserver.where(id: batch).update_all(sis_batch_id: @batch.id) + UserObservationLink.where(id: batch).update_all(sis_batch_id: @batch.id) end if @batch User.update_account_associations(importer.users_to_update_account_associations.to_a) @@ -80,9 +80,9 @@ module SIS def add_remove_observer(observer, student, observer_id, student_id, status) case status.downcase when 'active' - user_observer = UserObserver.create_or_restore(observer: observer, observee: student) + user_observer = UserObservationLink.create_or_restore(observer: observer, student: student) when 'deleted' - user_observer = observer.user_observees.active.where(user_id: student).take + user_observer = observer.as_observer_observation_links.where(user_id: student).take if user_observer user_observer.destroy else diff --git a/lib/user_merge.rb b/lib/user_merge.rb index 5fe0b02616c..c588faaec38 100644 --- a/lib/user_merge.rb +++ b/lib/user_merge.rb @@ -299,20 +299,20 @@ class UserMerge def move_observees(target_user, user_merge_data) # record all the records before destroying them # pass the from_user since user_id will be the observer - user_merge_data.add_more_data(from_user.user_observees, user: from_user) - user_merge_data.add_more_data(from_user.user_observers) + user_merge_data.add_more_data(from_user.as_observer_observation_links, user: from_user) + user_merge_data.add_more_data(from_user.as_student_observation_links) # delete duplicate or invalid observers/observees, move the rest - from_user.user_observees.where(user_id: target_user.user_observees.map(&:user_id)).destroy_all - from_user.user_observees.where(user_id: target_user).destroy_all - user_merge_data.add_more_data(target_user.user_observees.where(user_id: from_user), user: target_user) - target_user.user_observees.where(user_id: from_user).destroy_all - from_user.user_observees.active.update_all(observer_id: target_user.id) - xor_observer_ids = UserObserver.where(user_id: [from_user, target_user]).distinct.pluck(:observer_id) - from_user.user_observers.where(observer_id: target_user.user_observers.map(&:observer_id)).destroy_all - from_user.user_observers.active.update_all(user_id: target_user.id) + from_user.as_observer_observation_links.where(user_id: target_user.as_observer_observation_links.map(&:user_id)).destroy_all + from_user.as_observer_observation_links.where(user_id: target_user).destroy_all + user_merge_data.add_more_data(target_user.as_observer_observation_links.where(user_id: from_user), user: target_user) + target_user.as_observer_observation_links.where(user_id: from_user).destroy_all + from_user.as_observer_observation_links.update_all(observer_id: target_user.id) + xor_observer_ids = UserObservationLink.where(student: [from_user, target_user]).distinct.pluck(:observer_id) + from_user.as_student_observation_links.where(observer_id: target_user.as_student_observation_links.map(&:observer_id)).destroy_all + from_user.as_student_observation_links.update_all(user_id: target_user.id) # for any observers not already watching both users, make sure they have # any missing observer enrollments added - target_user.user_observers.where(observer_id: xor_observer_ids).each(&:create_linked_enrollments) + target_user.as_student_observation_links.where(observer_id: xor_observer_ids).each(&:create_linked_enrollments) end def destroy_conflicting_module_progressions(from_user, target_user) diff --git a/spec/apis/v1/assignments_api_spec.rb b/spec/apis/v1/assignments_api_spec.rb index 369ae9964da..a2f6d4cef83 100644 --- a/spec/apis/v1/assignments_api_spec.rb +++ b/spec/apis/v1/assignments_api_spec.rb @@ -1057,7 +1057,7 @@ describe AssignmentsApiController, type: :request do it "returns assignments for authorized observer" do course_with_student_submissions(:active_all => true) parent = User.create - parent.user_observees.create! do |uo| + parent.as_observer_observation_links.create! do |uo| uo.user_id = @student.id end parent.save! diff --git a/spec/apis/v1/calendar_events_api_spec.rb b/spec/apis/v1/calendar_events_api_spec.rb index 68724359454..ef4c06c25b0 100644 --- a/spec/apis/v1/calendar_events_api_spec.rb +++ b/spec/apis/v1/calendar_events_api_spec.rb @@ -2151,7 +2151,7 @@ describe CalendarEventsApiController, type: :request do enrollment_state: 'active', associated_user_id: @student.id ) - @observer.user_observees.create do |uo| + @observer.as_observer_observation_links.create do |uo| uo.user_id = @student.id end @contexts = [@course.asset_string] diff --git a/spec/apis/v1/courses_api_spec.rb b/spec/apis/v1/courses_api_spec.rb index c961db56889..e8764cc0b14 100644 --- a/spec/apis/v1/courses_api_spec.rb +++ b/spec/apis/v1/courses_api_spec.rb @@ -593,7 +593,7 @@ describe CoursesController, type: :request do end it "should return a course list for an observed students" do parent = User.create - parent.user_observees.create! do |uo| + parent.as_observer_observation_links.create! do |uo| uo.user_id = @me.id end json = api_call_as_user(parent,:get,"/api/v1/users/#{@me.id}/courses", @@ -606,7 +606,7 @@ describe CoursesController, type: :request do it "should fail if trying to view courses for student that is not observee" do # test to make sure it doesn't crash if user has not observees parent = User.create - expect(parent.user_observees).to eq [] + expect(parent.as_observer_observation_links).to eq [] api_call_as_user(parent,:get,"/api/v1/users/#{@me.id}/courses", { :user_id => @me.id, :controller => 'courses', :action => 'user_index', @@ -618,7 +618,7 @@ describe CoursesController, type: :request do @shard2.activate do a = Account.create parent = user_with_pseudonym(name: 'Zombo', username: 'nobody2@example.com', account: a) - parent.user_observees.create! do |uo| + parent.as_observer_observation_links.create! do |uo| uo.user_id = @me.id end parent.save! diff --git a/spec/apis/v1/enrollments_api_spec.rb b/spec/apis/v1/enrollments_api_spec.rb index 662bc9d344c..3116c49edf0 100644 --- a/spec/apis/v1/enrollments_api_spec.rb +++ b/spec/apis/v1/enrollments_api_spec.rb @@ -1671,7 +1671,7 @@ describe EnrollmentsApiController, type: :request do @course.enroll_student(@student, enrollment_state: 'active') end @observer = user_factory(active_all: true, active_state: 'active') - @observer.user_observees.create do |uo| + @observer.as_observer_observation_links.create do |uo| uo.user_id = @student.id end @user = @observer diff --git a/spec/apis/v1/user_observees_api_spec.rb b/spec/apis/v1/user_observees_api_spec.rb index 5800a64180b..1fcdeb1a279 100644 --- a/spec/apis/v1/user_observees_api_spec.rb +++ b/spec/apis/v1/user_observees_api_spec.rb @@ -156,33 +156,33 @@ describe UserObserveesController, type: :request do context 'GET #index' do specs_require_sharding it 'should list observees' do - parent.observed_users << student + parent.linked_students << student expect(index_call).to eq [student.id] end it 'should list observees (for self managed users)' do - parent.observed_users << student + parent.linked_students << student expect(index_call(api_user: parent)).to eq [student.id] end it 'should list observees (for external accounts)' do - external_parent.observed_users << external_student + external_parent.linked_students << external_student json = index_call(user_id: external_parent.id, api_user: multi_admin, domain_root_account: external_account) expect(json).to eq [external_student.id] end it 'should paginate' do - parent.observed_users << student - parent.observed_users << student2 + parent.linked_students << student + parent.linked_students << student2 expect(index_call(page: 1)).to eq [student2.id] expect(index_call(page: 2)).to eq [student.id] end it 'should not include deleted observers' do - parent.observed_users << student - parent.observed_users << student2 - parent.user_observees.where(user_id: student2).destroy_all + parent.linked_students << student + parent.linked_students << student2 + parent.as_observer_observation_links.where(user_id: student2).destroy_all expect(index_call).to eq [student.id] end @@ -192,7 +192,7 @@ describe UserObserveesController, type: :request do end it 'should not allow admins from an external account' do - external_parent.observed_users << external_student + external_parent.linked_students << external_student index_call(user_id: external_parent.id, domain_root_account: external_account, expected_status: 401) end @@ -206,7 +206,7 @@ describe UserObserveesController, type: :request do student.avatar_image_source = 'attachment' student.avatar_image_url = "/relative/canvas/path" student.save! - parent.observed_users << student + parent.linked_students << student opts = {:avatars=>true} json = raw_index_call(opts ) expect(json.map{|o| o['id'] }).to eq [student.id] @@ -223,7 +223,7 @@ describe UserObserveesController, type: :request do student.avatar_image_source = 'attachment' student.avatar_image_url = "/relative/canvas/path" student.save! - parent.observed_users << student + parent.linked_students << student parent.account.set_service_availability(:avatars, false) opts = {:avatars=>true} json = raw_index_call(opts ) @@ -240,7 +240,7 @@ describe UserObserveesController, type: :request do } expect(create_call({observee: observee})).to eq student.id - expect(parent.reload.observed_users).to eq [student] + expect(parent.reload.linked_students).to eq [student] end it 'should add an observee, given valid credentials (for self managed users)' do @@ -250,7 +250,7 @@ describe UserObserveesController, type: :request do } expect(create_call({observee: observee}, api_user: parent)).to eq student.id - expect(parent.reload.observed_users).to eq [student] + expect(parent.reload.linked_students).to eq [student] end it 'should add an observee, given valid credentails (for external accounts)' do @@ -261,7 +261,7 @@ describe UserObserveesController, type: :request do json = create_call({observee: observee}, user_id: external_parent.id, api_user: multi_admin, domain_root_account: external_account) expect(json).to eq external_student.id - expect(external_parent.reload.observed_users).to eq [external_student] + expect(external_parent.reload.linked_students).to eq [external_student] end it 'should not add an observee, given bad credentials' do @@ -271,7 +271,7 @@ describe UserObserveesController, type: :request do } create_call({observee: observee}, expected_status: 401) - expect(parent.reload.observed_users).to eq [] + expect(parent.reload.linked_students).to eq [] end it 'should not add an observee from an external account' do @@ -281,7 +281,7 @@ describe UserObserveesController, type: :request do } create_call({observee: observee}, domain_root_account: external_account, expected_status: 422) - expect(parent.reload.observed_users).to eq [] + expect(parent.reload.linked_students).to eq [] end it 'should not accept an invalid user' do @@ -307,7 +307,7 @@ describe UserObserveesController, type: :request do } create_call({observee: observee}, api_user: disallowed_admin, expected_status: 401) - expect(parent.reload.observed_users).to eq [] + expect(parent.reload.linked_students).to eq [] end it 'should not allow a user to observe oneself' do @@ -317,23 +317,23 @@ describe UserObserveesController, type: :request do } create_call({observee: observee}, api_user: student, expected_status: 401) - expect(student.reload.observed_users).to eq [] + expect(student.reload.linked_students).to eq [] end end context 'GET #show' do it 'should show an observee' do - parent.observed_users << student + parent.linked_students << student expect(show_call).to eq student.id end it 'should show an observee (for self managed users)' do - parent.observed_users << student + parent.linked_students << student expect(show_call(api_user: parent)).to eq student.id end it 'should show an observee (for external accounts)' do - external_parent.observed_users << external_student + external_parent.linked_students << external_student json = show_call(user_id: external_parent.id, observee_id: external_student.id, api_user: multi_admin, domain_root_account: external_account) expect(json).to eq external_student.id end @@ -343,17 +343,17 @@ describe UserObserveesController, type: :request do end it 'should not accept a non-observed user' do - parent.observed_users << student + parent.linked_students << student show_call(observee_id: student2.id, expected_status: 404) end it 'should not allow admins from an external account' do - external_parent.observed_users << external_student + external_parent.linked_students << external_student show_call(user_id: external_parent.id, observee_id: external_student.id, domain_root_account: external_account, expected_status: 401) end it 'should not allow unauthorized admins' do - parent.observed_users << student + parent.linked_students << student show_call(api_user: disallowed_admin, expected_status: 401) end end @@ -361,19 +361,19 @@ describe UserObserveesController, type: :request do context 'PUT #update' do it 'should add an observee by id' do expect(update_call).to eq student.id - expect(parent.reload.observed_users).to eq [student] + expect(parent.reload.linked_students).to eq [student] end it 'should not error if the observee already exists' do - parent.observed_users << student + parent.linked_students << student expect(update_call).to eq student.id - expect(parent.reload.observed_users).to eq [student] + expect(parent.reload.linked_students).to eq [student] end it 'should add an observee by id (for external accounts)' do json = update_call(user_id: external_parent.id, observee_id: external_student.id, api_user: multi_admin, domain_root_account: external_account) expect(json).to eq external_student.id - expect(external_parent.reload.observed_users).to eq [external_student] + expect(external_parent.reload.linked_students).to eq [external_student] end it 'should not accept an invalid user' do @@ -382,12 +382,12 @@ describe UserObserveesController, type: :request do it 'should not accept an invalid observee' do update_call(observee_id: 0, expected_status: 404) - expect(parent.reload.observed_users).to eq [] + expect(parent.reload.linked_students).to eq [] end it 'should not accept an observee from an external account' do update_call(observee_id: external_student.id, expected_status: 404) - expect(parent.reload.observed_users).to eq [] + expect(parent.reload.linked_students).to eq [] end it 'should not allow admins from an external account' do @@ -405,30 +405,30 @@ describe UserObserveesController, type: :request do context 'DELETE #destroy' do it 'should remove an observee by id' do - parent.observed_users << student + parent.linked_students << student course_factory.enroll_user(student) observer_enrollment = parent.observer_enrollments.first expect(delete_call).to eq student.id - expect(parent.observed_users.active_user_observers).to eq [] + expect(parent.reload.linked_students).to eq [] expect(observer_enrollment.reload).to be_deleted end it 'should remove an observee by id (for external accounts)' do - external_parent.observed_users << external_student + external_parent.linked_students << external_student course_factory(:account => external_account).enroll_user(external_student) observer_enrollment = external_parent.observer_enrollments.first json = delete_call(user_id: external_parent.id, observee_id: external_student.id, api_user: multi_admin, domain_root_account: external_account) expect(json).to eq external_student.id - expect(external_parent.reload.observed_users.active_user_observers).to eq [] + expect(external_parent.reload.linked_students).to eq [] expect(observer_enrollment.reload).to be_deleted end it 'should not succeed if the observee is not found' do - parent.observed_users << student + parent.linked_students << student delete_call(observee_id: student2.id, expected_status: 404) - expect(parent.reload.observed_users).to eq [student] + expect(parent.reload.linked_students).to eq [student] end it 'should not accept an invalid user' do @@ -444,17 +444,16 @@ describe UserObserveesController, type: :request do end it 'should not allow unauthorized admins' do - parent.observed_users << student + parent.linked_students << student delete_call(api_user: disallowed_admin, expected_status: 401) - expect(parent.reload.observed_users.active_user_observers).to eq [student] + expect(parent.reload.linked_students).to eq [student] end it 'should allow observer to remove observee' do - parent.observed_users << student + parent.linked_students << student delete_call(api_user: parent, expected_status: 200) - expect(parent.reload.observed_users.active_user_observers).to eq [] - expect(parent.observed_users).to eq [student] - expect(parent.user_observees.first.workflow_state).to eq 'deleted' + expect(parent.reload.linked_students).to eq [] + expect(UserObservationLink.where(:observer_id => parent).first.workflow_state).to eq 'deleted' end end @@ -462,12 +461,12 @@ describe UserObserveesController, type: :request do shared_examples "handle_observees_by_auth_token" do it 'should add an observee, given a valid access token' do expect(create_call({access_token: access_token_for_user(@token_student)})).to eq @token_student.id - expect(parent.reload.observed_users).to eq [@token_student] + expect(parent.reload.linked_students).to eq [@token_student] end it 'should not add an observee, given an invalid access token' do create_call({access_token: "Not A Valid Token"}, expected_status: 422) - expect(parent.reload.observed_users).to eq [] + expect(parent.reload.linked_students).to eq [] end end diff --git a/spec/apis/v1/users_api_spec.rb b/spec/apis/v1/users_api_spec.rb index 3969d76174c..b42c57a92ae 100644 --- a/spec/apis/v1/users_api_spec.rb +++ b/spec/apis/v1/users_api_spec.rb @@ -1953,7 +1953,7 @@ describe "Users API", type: :request do before :once do course_with_student(active_all: true) @observer = user_factory(active_all: true, active_state: 'active') - @observer.user_observees.create do |uo| + @observer.as_observer_observation_links.create do |uo| uo.user_id = @student.id end @user = @observer diff --git a/spec/controllers/communication_channels_controller_spec.rb b/spec/controllers/communication_channels_controller_spec.rb index ee50e5b8d71..3dd58c4e4db 100644 --- a/spec/controllers/communication_channels_controller_spec.rb +++ b/spec/controllers/communication_channels_controller_spec.rb @@ -502,7 +502,7 @@ describe CommunicationChannelsController do user_with_pseudonym(:username => 'jt+1@instructure.com', :active_all => 1) @logged_user = @user - @not_logged_user.observers << @logged_user + @not_logged_user.linked_observers << @logged_user user_session(@logged_user, @pseudonym) @@ -516,7 +516,7 @@ describe CommunicationChannelsController do user_with_pseudonym(:username => 'jt+1@instructure.com', :active_all => 1) @logged_user = @user - @logged_user.observers << @not_logged_user + @logged_user.linked_observers << @not_logged_user user_session(@logged_user, @pseudonym) diff --git a/spec/controllers/courses_controller_spec.rb b/spec/controllers/courses_controller_spec.rb index 5cf1ce932d4..42ca93721ad 100644 --- a/spec/controllers/courses_controller_spec.rb +++ b/spec/controllers/courses_controller_spec.rb @@ -273,7 +273,7 @@ describe CoursesController do expect(assigns[:future_enrollments]).to be_empty observer = user_with_pseudonym(active_all: true) - o = @student.user_observers.build; o.observer = observer; o.save! + o = @student.as_student_observation_links.build; o.observer = observer; o.save! user_session(observer) get 'index' expect(response).to be_success @@ -407,7 +407,7 @@ describe CoursesController do expect(assigns[:future_enrollments].map(&:course_id)).to eq [course1.id, course2.id] observer = user_with_pseudonym(active_all: true) - o = @student.user_observers.build; o.observer = observer; o.save! + o = @student.as_student_observation_links.build; o.observer = observer; o.save! user_session(observer) get 'index' expect(response).to be_success @@ -441,7 +441,7 @@ describe CoursesController do expect(assigns[:future_enrollments]).to eq [enrollment1] observer = user_with_pseudonym(active_all: true) - o = @student.user_observers.build; o.observer = observer; o.save! + o = @student.as_student_observation_links.build; o.observer = observer; o.save! user_session(observer) get 'index' expect(response).to be_success diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 9aa243acb3b..6b4082df2cd 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -276,7 +276,7 @@ describe UsersController do expect(response).to be_success new_pseudo = Pseudonym.where(unique_id: 'jane@example.com').first new_user = new_pseudo.user - expect(new_user.observed_users).to eq [@user] + expect(new_user.linked_students).to eq [@user] oe = new_user.observer_enrollments.first expect(oe.course).to eq @course expect(oe.associated_user).to eq @user @@ -484,7 +484,7 @@ describe UsersController do u = User.where(name: 'Jacob Fugal').first expect(u).to be_pre_registered expect(response).to be_success - expect(u.observed_users).to include(@user) + expect(u.linked_students).to include(@user) end end @@ -776,7 +776,7 @@ describe UsersController do let(:observer) { user_with_pseudonym(active_all: true) } it "returns the grade and the total for a student, filtered by grading period" do - student.observers << observer + student.linked_observers << observer user_session(observer) get('grades_for_student', params: {enrollment_id: student_enrollment.id, grading_period_id: grading_period.id}) @@ -798,7 +798,7 @@ describe UsersController do it "does not filter the grades by a grading period if " \ "'All Grading Periods' is selected" do - student.observers << observer + student.linked_observers << observer all_grading_periods_id = 0 user_session(observer) get('grades_for_student', params: {grading_period_id: all_grading_periods_id, @@ -850,8 +850,8 @@ describe UsersController do observer = user_with_pseudonym(active_all: true) course_with_user('StudentEnrollment', course: test_course, user: student1, active_all: true) course_with_user('StudentEnrollment', course: test_course, user: student2, active_all: true) - student1.observers << observer - student2.observers << observer + student1.linked_observers << observer + student2.linked_observers << observer observer end diff --git a/spec/lib/sis/csv/enrollment_importer_spec.rb b/spec/lib/sis/csv/enrollment_importer_spec.rb index 2e8169d9c53..2a0389eb4eb 100644 --- a/spec/lib/sis/csv/enrollment_importer_spec.rb +++ b/spec/lib/sis/csv/enrollment_importer_spec.rb @@ -788,7 +788,7 @@ describe SIS::CSV::EnrollmentImporter do student = Pseudonym.where(:unique_id => "user1").first.user observer = user_with_pseudonym(:account => @account) - student.observers << observer + student.linked_observers << observer process_csv_data_cleanly( "course_id,user_id,role,section_id,status,associated_user_id", diff --git a/spec/lib/sis/csv/user_observer_importer_spec.rb b/spec/lib/sis/csv/user_observer_importer_spec.rb index 8758efa75de..9f59f9c5558 100644 --- a/spec/lib/sis/csv/user_observer_importer_spec.rb +++ b/spec/lib/sis/csv/user_observer_importer_spec.rb @@ -25,7 +25,7 @@ describe SIS::CSV::UserObserverImporter do it 'should skip bad content' do user_with_managed_pseudonym(account: @account, sis_user_id: 'U001') user_with_managed_pseudonym(account: @account, sis_user_id: 'U002') - before_count = UserObserver.active.count + before_count = UserObservationLink.active.count importer = process_csv_data( 'observer_id,student_id,status', 'no_observer,U001,active', @@ -36,7 +36,7 @@ describe SIS::CSV::UserObserverImporter do 'U001,U002,dead', 'U001,U002,deleted' ) - expect(UserObserver.active.count).to eq before_count + expect(UserObservationLink.active.count).to eq before_count errors = importer.errors.map(&:last) expect(errors).to eq ["An observer referenced a non-existent user no_observer", @@ -51,17 +51,17 @@ describe SIS::CSV::UserObserverImporter do it "should add and remove user_observers" do user_with_managed_pseudonym(account: @account, sis_user_id: 'U001') user_with_managed_pseudonym(account: @account, sis_user_id: 'U002') - before_count = UserObserver.active.count + before_count = UserObservationLink.active.count process_csv_data_cleanly( "observer_id,student_id,status", "U001,U002,ACTIVE" ) - expect(UserObserver.active.count).to eq before_count + 1 + expect(UserObservationLink.active.count).to eq before_count + 1 process_csv_data_cleanly( "observer_id,student_id,status", "U001,U002,deleted" ) - expect(UserObserver.active.count).to eq before_count + expect(UserObservationLink.active.count).to eq before_count end end diff --git a/spec/lib/user_merge_spec.rb b/spec/lib/user_merge_spec.rb index 374115140dd..f8ec66c93bb 100644 --- a/spec/lib/user_merge_spec.rb +++ b/spec/lib/user_merge_spec.rb @@ -379,8 +379,8 @@ describe UserMerge do observer1 = user_with_pseudonym observer2 = user_with_pseudonym - user1.observers << observer1 << observer2 - user2.observers << observer2 + user1.linked_observers << observer1 << observer2 + user2.linked_observers << observer2 expect(ObserverEnrollment.count).to eql 3 Enrollment.where(user_id: observer2, associated_user_id: user1).update_all(workflow_state: 'completed') @@ -396,30 +396,30 @@ describe UserMerge do it "should move and uniquify observers" do observer1 = user_model observer2 = user_model - user1.observers << observer1 << observer2 - user2.observers << observer2 + user1.linked_observers << observer1 << observer2 + user2.linked_observers << observer2 UserMerge.from(user1).into(user2) data = UserMergeData.where(user_id: user2).first - expect(data.user_merge_data_records.where(context_type: 'UserObserver').count).to eq 2 + expect(data.user_merge_data_records.where(context_type: 'UserObservationLink').count).to eq 2 user1.reload - expect(user1.observers.active_user_observers).to be_empty - expect(user1.user_observers.first.workflow_state).to eq 'deleted' + expect(user1.linked_observers).to be_empty + expect(UserObservationLink.where(:student => user1).first.workflow_state).to eq 'deleted' user2.reload - expect(user2.observers.sort_by(&:id)).to eql [observer1, observer2] + expect(user2.linked_observers.sort_by(&:id)).to eql [observer1, observer2] end it "should move and uniquify observed users" do student1 = user_model student2 = user_model - user1.observed_users << student1 << student2 - user2.observed_users << student2 + user1.linked_students << student1 << student2 + user2.linked_students << student2 UserMerge.from(user1).into(user2) user1.reload - expect(user1.observed_users.active_user_observers).to be_empty + expect(user1.linked_students).to be_empty user2.reload - expect(user2.observed_users.sort_by(&:id)).to eql [student1, student2] + expect(user2.linked_students.sort_by(&:id)).to eql [student1, student2] end it "should move conversations to the new user" do diff --git a/spec/lib/user_search_spec.rb b/spec/lib/user_search_spec.rb index e0f7e1d9d1a..475a818ada6 100644 --- a/spec/lib/user_search_spec.rb +++ b/spec/lib/user_search_spec.rb @@ -121,7 +121,7 @@ describe UserSearch do ObserverEnrollment.create!(:user => ta, :course => course, :workflow_state => 'active') ta2 = User.create!(:name => 'Tyler Observer 2') ObserverEnrollment.create!(:user => ta2, :course => course, :workflow_state => 'active') - student.observers << ta2 + student.linked_observers << ta2 end it { is_expected.not_to include('Tyler Observer 2') } diff --git a/spec/migrations/link_missing_sis_observer_enrollments_spec.rb b/spec/migrations/link_missing_sis_observer_enrollments_spec.rb index 8dbd9ead309..be19f5bbe7f 100644 --- a/spec/migrations/link_missing_sis_observer_enrollments_spec.rb +++ b/spec/migrations/link_missing_sis_observer_enrollments_spec.rb @@ -24,7 +24,7 @@ describe 'DataFixup::LinkMissingSisObserverEnrollments' do course_with_student(:active_all => true) observer = user_with_pseudonym - @student.observers << observer + @student.linked_observers << observer @student.student_enrollments.first.update_attribute(:sis_batch_id, batch.id) diff --git a/spec/models/enrollment_spec.rb b/spec/models/enrollment_spec.rb index c2109b9b55e..6794a1a9136 100644 --- a/spec/models/enrollment_spec.rb +++ b/spec/models/enrollment_spec.rb @@ -868,7 +868,7 @@ describe Enrollment do course_with_teacher(:active_all => true) student = user_with_pseudonym observer = user_with_pseudonym - observer.observed_users << student + observer.linked_students << student @course.enroll_student(student, :no_notify => true) expect(student.messages).to be_empty @@ -888,7 +888,7 @@ describe Enrollment do course_with_teacher student = user_with_pseudonym observer = user_with_pseudonym - observer.observed_users << student + observer.linked_students << student @course.enroll_student(student) expect(observer.messages).to be_empty @@ -952,7 +952,7 @@ describe Enrollment do student = user_with_pseudonym observer = user_with_pseudonym old_time = observer.updated_at - observer.observed_users << student + observer.linked_students << student @course.enrollments.create(user: student, skip_touch_user: true, type: 'StudentEnrollment') expect(observer.reload.updated_at).to eq old_time end @@ -2427,7 +2427,7 @@ describe Enrollment do before :once do @student = user_factory(active_all: true) @parent = user_with_pseudonym(:active_all => true) - @student.observers << @parent + @student.linked_observers << @parent end it 'should get new observer enrollments when an observed user gets a new enrollment' do diff --git a/spec/models/split_user_spec.rb b/spec/models/split_user_spec.rb index d8547705400..8090c1eb1da 100644 --- a/spec/models/split_user_spec.rb +++ b/spec/models/split_user_spec.rb @@ -116,14 +116,14 @@ describe SplitUsers do it 'should handle user_observers' do observer1 = user_model observer2 = user_model - user1.observers << observer1 - user2.observers << observer2 + user1.linked_observers << observer1 + user2.linked_observers << observer2 UserMerge.from(user1).into(user2) SplitUsers.split_db_users(user2) - expect(user1.observers).to eq [observer1] - expect(user2.observers).to eq [observer2] + expect(user1.linked_observers).to eq [observer1] + expect(user2.linked_observers).to eq [observer2] end it 'should handle attachments' do @@ -148,48 +148,48 @@ describe SplitUsers do end it 'should handle when observing merged user' do - user2.observers << user1 + user2.linked_observers << user1 UserMerge.from(user1).into(user2) SplitUsers.split_db_users(user2) - expect(user1.user_observees).to eq UserObserver.where(user_id: user2, observer_id: user1) - expect(user2.user_observers).to eq UserObserver.where(user_id: user2, observer_id: user1) + expect(user1.as_observer_observation_links).to eq UserObservationLink.where(user_id: user2, observer_id: user1) + expect(user2.as_student_observation_links).to eq UserObservationLink.where(user_id: user2, observer_id: user1) end - it 'should handle user_observees' do + it 'should handle as_observer_observation_links' do observee1 = user_model observee2 = user_model - observee1.observers << user1 - observee2.observers << user2 + observee1.linked_observers << user1 + observee2.linked_observers << user2 UserMerge.from(user1).into(user2) SplitUsers.split_db_users(user2) - expect(user1.user_observees).to eq observee1.user_observers - expect(user2.user_observees).to eq observee2.user_observers + expect(user1.as_observer_observation_links).to eq observee1.as_student_observation_links + expect(user2.as_observer_observation_links).to eq observee2.as_student_observation_links end it 'should handle duplicate user_observers' do observer1 = user_model observee1 = user_model - observee1.observers << user1 - observee1.observers << user2 - user1.observers << observer1 - user2.observers << observer1 + observee1.linked_observers << user1 + observee1.linked_observers << user2 + user1.linked_observers << observer1 + user2.linked_observers << observer1 UserMerge.from(user1).into(user2) SplitUsers.split_db_users(user2) - expect(user1.user_observees.count).to eq 1 - expect(user2.user_observees.count).to eq 1 - expect(user1.observers).to eq [observer1] - expect(user2.observers).to eq [observer1] + expect(user1.as_observer_observation_links.count).to eq 1 + expect(user2.as_observer_observation_links.count).to eq 1 + expect(user1.linked_observers).to eq [observer1] + expect(user2.linked_observers).to eq [observer1] - expect(user1.user_observees.first.workflow_state).to eq 'active' - expect(user2.user_observees.first.workflow_state).to eq 'active' - expect(user1.user_observers.first.workflow_state).to eq 'active' - expect(user2.user_observers.first.workflow_state).to eq 'active' + expect(user1.as_observer_observation_links.first.workflow_state).to eq 'active' + expect(user2.as_observer_observation_links.first.workflow_state).to eq 'active' + expect(user1.as_student_observation_links.first.workflow_state).to eq 'active' + expect(user2.as_student_observation_links.first.workflow_state).to eq 'active' end it 'should only split users from merge_data when specified' do diff --git a/spec/models/user_observer_spec.rb b/spec/models/user_observer_spec.rb index 6ce1a9cf730..57e866b5fdf 100644 --- a/spec/models/user_observer_spec.rb +++ b/spec/models/user_observer_spec.rb @@ -18,25 +18,25 @@ require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper.rb') -describe UserObserver do +describe UserObservationLink do let_once(:student) { user_factory } it 'should fail when there is not observer or observee' do - expect { UserObserver.create_or_restore(observee: nil, observer: student) }. - to raise_error(ArgumentError, 'observee and observer are required') + expect { UserObservationLink.create_or_restore(student: nil, observer: student) }. + to raise_error(ArgumentError, 'student and observer are required') end it "should not allow a user to observe oneself" do - expect { student.observers << student }.to raise_error(ActiveRecord::RecordInvalid) + expect { student.linked_observers << student }.to raise_error(ActiveRecord::RecordInvalid) end it 'should restore deleted observees instead of creating a new one' do observer = user_with_pseudonym - student.observers << observer - observee = observer.user_observees.first + student.linked_observers << observer + observee = observer.as_observer_observation_links.first observee.destroy - re_observee = UserObserver.create_or_restore(observer: observer, observee: student) + re_observee = UserObservationLink.create_or_restore(observer: observer, student: student) expect(observee.id).to eq re_observee.id expect(re_observee.workflow_state).to eq 'active' end @@ -46,18 +46,18 @@ describe UserObserver do student_enroll = student_in_course(:user => student) observer = user_with_pseudonym - student.observers << observer + student.linked_observers << observer observer_enroll = observer.observer_enrollments.first observer_enroll.destroy - UserObserver.create_or_restore(observer: observer, observee: student) + UserObservationLink.create_or_restore(observer: observer, student: student) expect(observer_enroll.reload).to_not be_deleted end it 'should create an observees when one does not exist' do observer = user_with_pseudonym - re_observee = UserObserver.create_or_restore(observer: observer, observee: student) - expect(re_observee).to eq student.user_observers.first + re_observee = UserObservationLink.create_or_restore(observer: observer, student: student) + expect(re_observee).to eq student.as_student_observation_links.first end it "should enroll the observer in all pending/active courses and restore them after destroy" do @@ -70,7 +70,7 @@ describe UserObserver do e3.complete! observer = user_with_pseudonym - student.observers << observer + student.linked_observers << observer enrollments = observer.observer_enrollments.order(:course_id) expect(enrollments.size).to eql 2 @@ -83,7 +83,7 @@ describe UserObserver do p = observer.pseudonyms.first p.workflow_state = 'active' p.save! - UserObserver.create_or_restore(observer: observer, observee: student) + UserObservationLink.create_or_restore(observer: observer, student: student) observer.reload expect(enrollments.reload.map(&:workflow_state)).to eql ["active", "active"] end @@ -93,14 +93,14 @@ describe UserObserver do e1 = student_in_course(:course => c1, :user => student) observer = user_with_pseudonym - student.observers << observer + student.linked_observers << observer - preloaded_student = User.where(:id => student).preload(:observers).first - expect(preloaded_student.association(:observers).loaded?).to be_truthy - expect(preloaded_student.observers).to eq [observer] + preloaded_student = User.where(:id => student).preload(:linked_observers).first + expect(preloaded_student.association(:linked_observers).loaded?).to be_truthy + expect(preloaded_student.linked_observers).to eq [observer] - UserObserver.where(:user_id => student).update_all(:workflow_state => "deleted") - expect(User.where(:id => student).preload(:observers).first.observers).to eq [] + UserObservationLink.where(:user_id => student).update_all(:workflow_state => "deleted") + expect(User.where(:id => student).preload(:linked_observers).first.linked_observers).to eq [] end it "should enroll the observer in courses when the student is inactive" do @@ -109,7 +109,7 @@ describe UserObserver do enroll.deactivate observer = user_with_pseudonym - student.observers << observer + student.linked_observers << observer o_enroll = observer.observer_enrollments.first expect(o_enroll).to be_inactive @@ -125,13 +125,13 @@ describe UserObserver do c1 = course_factory(active_all: true) observer = user_with_pseudonym - student.observers << observer + student.linked_observers << observer - uo = student.user_observers.first + uo = student.as_student_observation_links.first uo.destroy! student.reload - expect(student.observers).to be_empty + expect(student.linked_observers).to be_empty enroll = student_in_course(:course => c1, :user => student) @@ -154,7 +154,7 @@ describe UserObserver do observer = user_with_pseudonym(account: a2) allow(@pseudonym).to receive(:works_for_account?).and_return(false) allow(@pseudonym).to receive(:works_for_account?).with(a2, true).and_return(true) - student.observers << observer + student.linked_observers << observer enrollments = observer.observer_enrollments expect(enrollments.size).to eql 1 @@ -167,7 +167,7 @@ describe UserObserver do @course = course_factory active_all: true @student_enrollment = student_in_course(course: @course, user: student, active_all: true) @observer = user_with_pseudonym - student.observers << @observer + student.linked_observers << @observer @observer_enrollment = @observer.enrollments.where(type: 'ObserverEnrollment', course_id: @course, associated_user_id: student).first end @@ -196,7 +196,7 @@ describe UserObserver do parent = nil @shard2.activate do parent = user_with_pseudonym(account: course.account, active_all: true) - UserObserver.create_or_restore(observer: parent, observee: student) + UserObservationLink.create_or_restore(observer: parent, student: student) end expect(parent.enrollments.shard(parent).first.course).to eq course end diff --git a/spec/selenium/grades/student_grades_page/student_grades_page_observer_spec.rb b/spec/selenium/grades/student_grades_page/student_grades_page_observer_spec.rb index 7de5c776c0c..23f08b6ae89 100644 --- a/spec/selenium/grades/student_grades_page/student_grades_page_observer_spec.rb +++ b/spec/selenium/grades/student_grades_page/student_grades_page_observer_spec.rb @@ -28,7 +28,7 @@ describe "gradebook" do @course.observers=[@observer] assignment_setup_defaults assignment_setup - @all_students.each {|s| s.observers=[@observer]} + @all_students.each {|s| s.linked_observers=[@observer]} user_session(@teacher) end