diff --git a/app/models/split_users.rb b/app/models/split_users.rb index 3672f5e0d5b..46aeaa3efbb 100644 --- a/app/models/split_users.rb +++ b/app/models/split_users.rb @@ -81,230 +81,255 @@ class SplitUsers context_id: 'courses.id'}.freeze ].freeze + attr_accessor :source_user, :restored_user, :merge_data + + def initialize(source_user, merge_data) + @source_user = source_user + @merge_data = merge_data + @restored_user = nil + end + def self.split_db_users(user, merge_data = nil) if merge_data - users = split_users(user, merge_data) + users = new(user, merge_data).split_users else users = [] UserMergeData.active.splitable.where(user_id: user).shard(user).find_each do |data| - splitters = split_users(user, data) + splitters = new(user, data).split_users users = splitters | users end end users end - class << self - private - - def split_users(user, merge_data) - # user is the active user that was the destination of the user merge - user.shard.activate do - ActiveRecord::Base.transaction do - records = merge_data.user_merge_data_records - old_user = User.find(merge_data.from_user_id) - old_user, pseudonyms = restore_old_user(old_user, records) - records = check_and_update_local_ids(old_user, records) if merge_data.from_user_id > Shard::IDS_PER_SHARD - records = records.preload(:context) - restore_merge_items(old_user) - move_records_to_old_user(user, old_user, records, pseudonyms) - # update account associations for each split out user - users = [old_user, user] - User.update_account_associations(users, all_shards: (old_user.shard != user.shard)) - merge_data.destroy - User.where(id: users).touch_all - users - end - end - end - - def restore_merge_items(old_user) - Shard.with_each_shard(old_user.associated_shards + old_user.associated_shards(:weak) + old_user.associated_shards(:shadow)) do - UserPastLtiIds.where(user: old_user, user_lti_id: old_user.lti_id).delete_all - end - end - - def check_and_update_local_ids(old_user, records) - if records.where("previous_user_id(course) { course.shard }) do |shard_course| - source_user_id = source_user.id - target_user_id = target_user.id - ENROLLMENT_DATA_UPDATES.each do |update| - relation = update[:table].classify.constantize.all - relation = relation.instance_exec(&update[:scope]) if update[:scope] - - relation. - where((update[:context_id] || :context_id) => shard_course, - (update[:foreign_key] || :user_id) => source_user_id). - update_all((update[:foreign_key] || :user_id) => target_user_id) - end - end - end - - # source_user is the destination user of the user merge - # user is the old user that is being restored - # enrollments are enrollments that have been created since the merge event, - # but for a pseudonym that was moved back to the old user. - # Also work that has happened since the merge event should moved if the - # enrollment is moved. - def move_submissions(source_user, user, enrollments) - # there should be no conflicts here because this is only called for - # enrollments that were updated which already excluded conflicts, but we - # will add the scope to protect against a FK violation. - source_user.submissions.where(assignment_id: Assignment.where(context_id: enrollments.map(&:course_id))). - where.not(assignment_id: user.all_submissions.select(:assignment_id)). - update_all(user_id: user.id) - source_user.quiz_submissions.where(quiz_id: Quizzes::Quiz.where(context_id: enrollments.map(&:course_id))). - where.not(quiz_id: user.quiz_submissions.select(:quiz_id)). - update_all(user_id: user.id) - end - - def handle_submissions(source_user, user, records) - [[:submissions, 'fk_rails_8d85741475'], - [:'quizzes/quiz_submissions', 'fk_rails_04850db4b4']].each do |table, foreign_key| - model = table.to_s.classify.constantize - - ids_by_shard = records.where(context_type: model.to_s, previous_user_id: user).pluck(:context_id).group_by{|id| Shard.shard_for(id)} - other_ids_by_shard = records.where(context_type: model.to_s, previous_user_id: source_user).pluck(:context_id).group_by{|id| Shard.shard_for(id)} - - (ids_by_shard.keys + other_ids_by_shard.keys).uniq.each do |shard| - ids = ids_by_shard[shard] || [] - other_ids = other_ids_by_shard[shard] || [] - shard.activate do - model.transaction do - # there is a unique index on assignment_id and user_id or quiz_id - # and user_id. Unique indexes are checked after every row during - # an update statement to get around this and to allow us to swap - # we are setting the user_id to the negative user_id and then back - # to the user_id after the conflicting rows have been updated. - model.connection.execute("SET CONSTRAINTS #{model.connection.quote_table_name(foreign_key)} DEFERRED") - model.where(id: ids).update_all(user_id: -user.id) - model.where(id: other_ids).update_all(user_id: source_user.id) - model.where(id: ids).update_all(user_id: user.id) - end - Enrollment.send_later(:recompute_due_dates_and_scores, source_user.id) - Enrollment.send_later(:recompute_due_dates_and_scores, user.id) - end - end - end - end - - def restore_workflow_states_from_records(records) - records.each do |r| - c = r.context - next unless c && c.class.columns_hash.key?('workflow_state') - c.workflow_state = r.previous_workflow_state unless c.class == Attachment - c.file_state = r.previous_workflow_state if c.class == Attachment - c.save! if c.changed? && c.valid? + def split_users + source_user.shard.activate do + ActiveRecord::Base.transaction do + @restored_user = User.find(merge_data.from_user_id) + records = merge_data.records + pseudonyms = restore_users + records = check_and_update_local_ids(records) if merge_data.from_user_id > Shard::IDS_PER_SHARD + records = records.preload(:context) + restore_merge_items + move_records_to_old_user(records, pseudonyms) + # update account associations for each split out user + users = [restored_user, source_user] + User.update_account_associations(users, all_shards: (restored_user.shard != source_user.shard)) + merge_data.destroy + User.where(id: users).touch_all + users end end end + + private + + def restore_merge_items + Shard.with_each_shard(restored_user.associated_shards + restored_user.associated_shards(:weak) + restored_user.associated_shards(:shadow)) do + UserPastLtiIds.where(user: restored_user, user_lti_id: restored_user.lti_id).delete_all + end + end + + def check_and_update_local_ids(records) + if records.where("previous_user_id(course) {course.shard}) do |shard_course| + source_user_id = source_user.id + target_user_id = restored_user.id + ENROLLMENT_DATA_UPDATES.each do |update| + relation = update[:table].classify.constantize.all + relation = relation.instance_exec(&update[:scope]) if update[:scope] + + relation. + where((update[:context_id] || :context_id) => shard_course, + (update[:foreign_key] || :user_id) => source_user_id). + update_all((update[:foreign_key] || :user_id) => target_user_id) + end + end + end + + # enrollments are enrollments that have been created since the merge event, + # but for a pseudonym that was moved back to the old user. + # Also work that has happened since the merge event should moved if the + # enrollment is moved. + def move_submissions(enrollments) + # there should be no conflicts here because this is only called for + # enrollments that were updated which already excluded conflicts, but we + # will add the scope to protect against a FK violation. + source_user.submissions.where(assignment_id: Assignment.where(context_id: enrollments.map(&:course_id))). + where.not(assignment_id: restored_user.all_submissions.select(:assignment_id)). + update_all(user_id: restored_user.id) + source_user.quiz_submissions.where(quiz_id: Quizzes::Quiz.where(context_id: enrollments.map(&:course_id))). + where.not(quiz_id: restored_user.quiz_submissions.select(:quiz_id)). + update_all(user_id: restored_user.id) + end + + def handle_submissions(records) + [[:submissions, 'fk_rails_8d85741475'], + [:'quizzes/quiz_submissions', 'fk_rails_04850db4b4']].each do |table, foreign_key| + model = table.to_s.classify.constantize + + ids_by_shard = records.where(context_type: model.to_s, previous_user_id: restored_user).pluck(:context_id).group_by {|id| Shard.shard_for(id)} + other_ids_by_shard = records.where(context_type: model.to_s, previous_user_id: source_user).pluck(:context_id).group_by {|id| Shard.shard_for(id)} + + (ids_by_shard.keys + other_ids_by_shard.keys).uniq.each do |shard| + ids = ids_by_shard[shard] || [] + other_ids = other_ids_by_shard[shard] || [] + shard.activate do + model.transaction do + # there is a unique index on assignment_id and user_id or quiz_id + # and user_id. Unique indexes are checked after every row during + # an update statement to get around this and to allow us to swap + # we are setting the user_id to the negative user_id and then back + # to the user_id after the conflicting rows have been updated. + model.connection.execute("SET CONSTRAINTS #{model.connection.quote_table_name(foreign_key)} DEFERRED") + model.where(id: ids).update_all(user_id: -restored_user.id) + model.where(id: other_ids).update_all(user_id: source_user.id) + model.where(id: ids).update_all(user_id: restored_user.id) + end + Enrollment.send_later(:recompute_due_dates_and_scores, source_user.id) + Enrollment.send_later(:recompute_due_dates_and_scores, restored_user.id) + end + end + end + end + + def restore_workflow_states_from_records(records) + records.each do |r| + c = r.context + next unless c && c.class.columns_hash.key?('workflow_state') + c.workflow_state = r.previous_workflow_state unless c.class == Attachment + c.file_state = r.previous_workflow_state if c.class == Attachment + c.save! if c.changed? && c.valid? + end + end end diff --git a/app/models/user_merge_data.rb b/app/models/user_merge_data.rb index 8f8bff57086..b61b311d290 100644 --- a/app/models/user_merge_data.rb +++ b/app/models/user_merge_data.rb @@ -18,7 +18,8 @@ class UserMergeData < ActiveRecord::Base belongs_to :user belongs_to :from_user, class_name: 'User' - has_many :user_merge_data_records + has_many :records, class_name: 'UserMergeDataRecord', inverse_of: :merge_data + has_many :items, class_name: 'UserMergeDataItem', inverse_of: :merge_data scope :active, -> { where.not(workflow_state: 'deleted') } scope :splitable, -> { where('created_at > ?', split_time) } @@ -37,7 +38,7 @@ class UserMergeData < ActiveRecord::Base self.shard.activate do objects.each do |o| user ||= o.user_id - r = self.user_merge_data_records.new(context: o, previous_user_id: user) + r = self.records.new(context: o, previous_user_id: user) r.previous_workflow_state = o.workflow_state if o.class.columns_hash.key?('workflow_state') r.previous_workflow_state = o.file_state if o.class == Attachment r.previous_workflow_state = workflow_state if workflow_state diff --git a/app/models/user_merge_data_item.rb b/app/models/user_merge_data_item.rb new file mode 100644 index 00000000000..df75b7c1a21 --- /dev/null +++ b/app/models/user_merge_data_item.rb @@ -0,0 +1,23 @@ +# +# Copyright (C) 2019 - 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 . +# +class UserMergeDataItem < ActiveRecord::Base + belongs_to :user + belongs_to :merge_data, class_name: 'UserMergeData', inverse_of: :items + + serialize :item +end diff --git a/app/models/user_merge_data_record.rb b/app/models/user_merge_data_record.rb index 37640428455..e87b2dc3a85 100644 --- a/app/models/user_merge_data_record.rb +++ b/app/models/user_merge_data_record.rb @@ -17,7 +17,7 @@ # class UserMergeDataRecord < ActiveRecord::Base belongs_to :previous_user, class_name: 'User' - belongs_to :user_merge_data + belongs_to :merge_data, class_name: 'UserMergeData', inverse_of: :records 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/db/migrate/20160818202512_recompute_merged_enrollments.rb b/db/migrate/20160818202512_recompute_merged_enrollments.rb index 122655acd21..27ac70900f1 100644 --- a/db/migrate/20160818202512_recompute_merged_enrollments.rb +++ b/db/migrate/20160818202512_recompute_merged_enrollments.rb @@ -22,7 +22,8 @@ class RecomputeMergedEnrollments < ActiveRecord::Migration[4.2] def up start_date = DateTime.parse("2016-08-05") merged_enrollment_ids = UserMergeDataRecord.where(:context_type => "Enrollment"). - joins(:user_merge_data).where("user_merge_data.updated_at > ?", start_date).pluck(:context_id) + joins("INNER JOIN #{UserMergeData.quoted_table_name} ON user_merge_data_records.user_merge_data_id = user_merge_data.id"). + where("user_merge_data.updated_at > ?", start_date).pluck(:context_id) if merged_enrollment_ids.any? Shard.partition_by_shard(merged_enrollment_ids) do |sliced_ids| diff --git a/db/migrate/20190214060931_user_merge_data_items.rb b/db/migrate/20190214060931_user_merge_data_items.rb new file mode 100644 index 00000000000..137975b042a --- /dev/null +++ b/db/migrate/20190214060931_user_merge_data_items.rb @@ -0,0 +1,29 @@ +# +# Copyright (C) 2019 - 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 . + +class UserMergeDataItems < ActiveRecord::Migration[5.1] + tag :predeploy + + def change + create_table :user_merge_data_items do |t| + t.references :user_merge_data, limit: 8, foreign_key: true, index: true, null: false + t.references :user, limit: 8, foreign_key: true, index: true, null: false + t.string :item_type, null: false, limit: 255 + t.text :item, null: false + end + end +end diff --git a/lib/user_merge.rb b/lib/user_merge.rb index 5409a70c67e..4ee658db037 100644 --- a/lib/user_merge.rb +++ b/lib/user_merge.rb @@ -22,29 +22,45 @@ class UserMerge end attr_reader :from_user - attr_accessor :data + attr_accessor :target_user, :data, :merge_data def initialize(from_user) @from_user = from_user + @target_user = nil + @merge_data = nil @data = [] end def into(target_user) return unless target_user return if target_user == from_user + @target_user = target_user target_user.associate_with_shard(from_user.shard, :shadow) # we also store records for the from_user on the target shard for a split from_user.associate_with_shard(target_user.shard, :shadow) - user_merge_data = target_user.shard.activate do - UserMergeData.create!(user: target_user, from_user: from_user) - end + target_user.shard.activate do + @merge_data = UserMergeData.create!(user: target_user, from_user: from_user) - if target_user.avatar_state == :none && from_user.avatar_state != :none - [:avatar_image_source, :avatar_image_url, :avatar_image_updated_at, :avatar_state].each do |attr| - target_user[attr] = from_user[attr] + items = [] + if target_user.avatar_state == :none && from_user.avatar_state != :none + [:avatar_image_source, :avatar_image_url, :avatar_image_updated_at, :avatar_state].each do |attr| + items << merge_data.items.new(user: from_user, item_type: attr.to_s, item: from_user[attr]) if from_user[attr] + target_user[attr] = from_user[attr] + end end + + # record the users names and preferences in case of split. + items << merge_data.items.new(user: from_user, item_type: 'user_name', item: from_user.name) + items << merge_data.items.new(user: target_user, item_type: 'user_name', item: target_user.name) + UserMergeDataItem.bulk_insert_objects(items) + + # bulk insert doesn't play nice with the hash values of preferences. + merge_data.items.create!(user: from_user, item_type: 'user_preferences', item: from_user.preferences) + merge_data.items.create!(user: target_user, item_type: 'user_preferences', item: target_user.preferences) + + target_user.preferences = target_user.preferences.merge(from_user.preferences) + target_user.save if target_user.changed? end - target_user.save if target_user.changed? [:strong, :weak, :shadow].each do |strength| from_user.associated_shards(strength).each do |shard| @@ -52,25 +68,22 @@ class UserMerge end end - populate_past_lti_ids(target_user) - - handle_communication_channels(target_user, user_merge_data) - - destroy_conflicting_module_progressions(@from_user, target_user) - - move_enrollments(target_user, user_merge_data) + populate_past_lti_ids + handle_communication_channels + destroy_conflicting_module_progressions + move_enrollments Shard.with_each_shard(from_user.associated_shards + from_user.associated_shards(:weak) + from_user.associated_shards(:shadow)) do max_position = Pseudonym.where(user_id: target_user).order(:position).last.try(:position) || 0 pseudonyms_to_move = Pseudonym.where(user_id: from_user) - user_merge_data.add_more_data(pseudonyms_to_move) + merge_data.add_more_data(pseudonyms_to_move) pseudonyms_to_move.update_all(["user_id=?, position=position+?", target_user, max_position]) target_user.communication_channels.email.unretired.each do |cc| Rails.cache.delete([cc.path, 'invited_enrollments2'].cache_key) end - handle_submissions(target_user, user_merge_data) + handle_submissions from_user.all_conversations.find_each { |c| c.move_to_user(target_user) } @@ -86,11 +99,11 @@ class UserMerge end account_users = AccountUser.where(user_id: from_user) - user_merge_data.add_more_data(account_users) + merge_data.add_more_data(account_users) account_users.update_all(user_id: target_user.id) attachments = Attachment.where(user_id: from_user) - user_merge_data.add_more_data(attachments) + merge_data.add_more_data(attachments) Attachment.send_later(:migrate_attachments, from_user, target_user) updates = {} @@ -114,7 +127,7 @@ class UserMerge scope = klass.where(column => from_user) klass.transaction do if version_updates.include?(table) - update_versions(from_user, target_user, scope, table, column) + update_versions(scope, table, column) end scope.update_all(column => target_user.id) end @@ -131,7 +144,7 @@ class UserMerge update_all(context_id: target_user.id, context_code: target_user.asset_string) end - move_observees(target_user, user_merge_data) + move_observees Enrollment.send_later(:recompute_due_dates_and_scores, target_user.id) target_user.update_account_associations @@ -142,7 +155,7 @@ class UserMerge from_user.destroy end - def populate_past_lti_ids(target_user) + def populate_past_lti_ids Shard.with_each_shard(from_user.associated_shards + from_user.associated_shards(:weak) + from_user.associated_shards(:shadow)) do lti_ids = [] {enrollments: :course, group_memberships: :group, account_users: :account}.each do |klass, type| @@ -160,62 +173,62 @@ class UserMerge end end - def handle_communication_channels(target_user, user_merge_data) + def handle_communication_channels max_position = target_user.communication_channels.last.try(:position) || 0 to_retire_ids = [] known_ccs = target_user.communication_channels.pluck(:id) from_user.communication_channels.each do |cc| # have to find conflicting CCs, and make sure we don't have conflicts - target_cc = detect_conflicting_cc(cc, target_user) + target_cc = detect_conflicting_cc(cc) if !target_cc && from_user.shard != target_user.shard User.clone_communication_channel(cc, target_user, max_position) new_cc = target_user.communication_channels.where.not(id: known_ccs).take known_ccs << new_cc.id - user_merge_data.build_more_data([new_cc], user: target_user, workflow_state: 'non_existent', data: data) + merge_data.build_more_data([new_cc], user: target_user, workflow_state: 'non_existent', data: data) end next unless target_cc - to_retire = handle_conflicting_ccs(cc, target_cc, target_user, max_position) + to_retire = handle_conflicting_ccs(cc, target_cc, max_position) if to_retire keeper = ([target_cc, cc] - [to_retire]).first - copy_notificaion_policies(to_retire, keeper, user_merge_data) + copy_notificaion_policies(to_retire, keeper) to_retire_ids << to_retire.id end end - finish_ccs(max_position, target_user, to_retire_ids, user_merge_data) + finish_ccs(max_position, to_retire_ids) end - def detect_conflicting_cc(source_cc, target_user) + def detect_conflicting_cc(source_cc) target_user.communication_channels.detect do |c| c.path.downcase == source_cc.path.downcase && c.path_type == source_cc.path_type end end - def finish_ccs(max_position, target_user, to_retire_ids, user_merge_data) + def finish_ccs(max_position, to_retire_ids) if from_user.shard != target_user.shard - handle_cross_shard_cc(target_user, user_merge_data) + handle_cross_shard_cc else from_user.shard.activate do ccs = CommunicationChannel.where(id: to_retire_ids).where.not(workflow_state: 'retired') - user_merge_data.build_more_data(ccs, data: data) unless to_retire_ids.empty? + merge_data.build_more_data(ccs, data: data) unless to_retire_ids.empty? ccs.update_all(workflow_state: 'retired') unless to_retire_ids.empty? end scope = from_user.communication_channels.where.not(workflow_state: 'retired') scope = scope.where.not(id: to_retire_ids) unless to_retire_ids.empty? unless scope.empty? - user_merge_data.build_more_data(scope, data: data) + merge_data.build_more_data(scope, data: data) scope.update_all(["user_id=?, position=position+?", target_user, max_position]) end end - user_merge_data.bulk_insert_merge_data(data) + merge_data.bulk_insert_merge_data(data) @data = [] end - def handle_cross_shard_cc(target_user, user_merge_data) + def handle_cross_shard_cc ccs = from_user.communication_channels.where.not(workflow_state: 'retired') - user_merge_data.build_more_data(ccs, data: data) unless ccs.empty? + merge_data.build_more_data(ccs, data: data) unless ccs.empty? ccs.update_all(workflow_state: 'retired') unless ccs.empty? from_user.user_services.each do |us| @@ -223,13 +236,13 @@ class UserMerge new_us.shard = target_user.shard new_us.user = target_user new_us.save! - user_merge_data.build_more_data([new_us], user: target_user, workflow_state: 'non_existent', data: data) + merge_data.build_more_data([new_us], user: target_user, workflow_state: 'non_existent', data: data) end - user_merge_data.build_more_data(from_user.user_services, data: data) + merge_data.build_more_data(from_user.user_services, data: data) from_user.user_services.delete_all end - def handle_conflicting_ccs(source_cc, target_cc, target_user, max_position) + def handle_conflicting_ccs(source_cc, target_cc, max_position) # we prefer keeping the "most" active one, preferring the target user if they're equal # the comments inline show all the different cases, with the source cc on the left, # target cc on the right. The * indicates the CC that will be retired in order @@ -265,7 +278,7 @@ class UserMerge to_retire end - def copy_notificaion_policies(to_retire, keeper, user_merge_data) + def copy_notificaion_policies(to_retire, keeper) # cross shard channels get cloned and so do notification_policies return unless to_retire.shard == keeper.shard # if the communication_channel is already retired, don't bother. @@ -282,15 +295,15 @@ class UserMerge NotificationPolicy.bulk_insert_objects(new_nps) end - def move_observees(target_user, user_merge_data) + def move_observees # record all the records before destroying them # pass the from_user since user_id will be the observer - user_merge_data.build_more_data(from_user.as_observer_observation_links, user: from_user, data: data) - user_merge_data.build_more_data(from_user.as_student_observation_links, data: data) + merge_data.build_more_data(from_user.as_observer_observation_links, user: from_user, data: data) + merge_data.build_more_data(from_user.as_student_observation_links, data: data) # delete duplicate or invalid observers/observees, move the rest 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, data: data) + merge_data.add_more_data(target_user.as_observer_observation_links.where(user_id: from_user), user: target_user, data: data) @data = [] target_user.as_observer_observation_links.where(user_id: from_user).destroy_all target_user.associate_with_shard(from_user.shard) if from_user.as_observer_observation_links.exists? @@ -304,7 +317,7 @@ class UserMerge 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) + def destroy_conflicting_module_progressions # there is a unique index on the context_module_progressions table # we need to delete all the conflicting context_module_progressions # without impacting the users module progress and without having to @@ -374,7 +387,7 @@ class UserMerge keeper.destroy end - def handle_conflicts(column, target_user, user_merge_data) + def handle_conflicts(column) users = [from_user, target_user] # get each pair of conflicts and "handle them" @@ -391,49 +404,49 @@ class UserMerge # both target and from users records will be recorded in case of a split. if to_update.exists? # record both records state since both will change - user_merge_data.build_more_data(scope, data: data) + merge_data.build_more_data(scope, data: data) update_enrollment_state(scope, keeper) end # identify if the from users records are worse states than target user to_delete = scope.active.where.not(id: keeper).where(column => from_user) # record the current state in case of split - user_merge_data.build_more_data(to_delete, data: data) + merge_data.build_more_data(to_delete, data: data) # mark all conflicts on from_user as deleted so they will not be moved later to_delete.destroy_all end end - def remove_self_observers(target_user, user_merge_data) + def remove_self_observers # prevent observing self by marking them as deleted to_delete = Enrollment.active.where("type = 'ObserverEnrollment' AND (associated_user_id = :target_user AND user_id = :from_user OR associated_user_id = :from_user AND user_id = :target_user)", {target_user: target_user, from_user: from_user}) - user_merge_data.build_more_data(to_delete, data: data) + merge_data.build_more_data(to_delete, data: data) to_delete.destroy_all end - def move_enrollments(target_user, user_merge_data) + def move_enrollments [:associated_user_id, :user_id].each do |column| Shard.with_each_shard(from_user.associated_shards) do Enrollment.transaction do - handle_conflicts(column, target_user, user_merge_data) - remove_self_observers(target_user, user_merge_data) + handle_conflicts(column) + remove_self_observers # move all the enrollments that have not been marked as deleted to the target user to_move = Enrollment.active.where(column => from_user) # upgrade to strong association if there are any enrollments target_user.associate_with_shard(from_user.shard) if to_move.exists? - user_merge_data.build_more_data(to_move, data: data) + merge_data.build_more_data(to_move, data: data) to_move.update_all(column => target_user.id) end end end - user_merge_data.bulk_insert_merge_data(data) + merge_data.bulk_insert_merge_data(data) @data = [] end - def handle_submissions(target_user, user_merge_data) + def handle_submissions [ [:assignment_id, :submissions], [:quiz_id, :'quizzes/quiz_submissions'] @@ -457,9 +470,9 @@ class UserMerge to_move_ids += scope.having_submission.select(unique_id).where.not(unique_id => already_scope.having_submission.select(unique_id), id: to_move_ids).pluck(:id) to_move = scope.where(id: to_move_ids).to_a move_back = already_scope.where(unique_id => to_move.map(&unique_id)).to_a - user_merge_data.build_more_data(to_move, data: data) - user_merge_data.build_more_data(move_back, data: data) - swap_submission(model, move_back, table, target_user, to_move, to_move_ids, 'fk_rails_8d85741475') + merge_data.build_more_data(to_move, data: data) + merge_data.build_more_data(move_back, data: data) + swap_submission(model, move_back, table, to_move, to_move_ids, 'fk_rails_8d85741475') elsif model.name == "Quizzes::QuizSubmission" subscope = already_scope.to_a to_move = model.where(user_id: from_user).joins(:submission).where(submissions: {user_id: target_user}).to_a @@ -467,19 +480,19 @@ class UserMerge to_move += scope.where("#{unique_id} NOT IN (?)", [subscope.map(&unique_id), move_back.map(&unique_id)].flatten).to_a move_back += already_scope.where(unique_id => to_move.map(&unique_id)).to_a - user_merge_data.build_more_data(to_move, data: data) - user_merge_data.build_more_data(move_back, data: data) - swap_submission(model, move_back, table, target_user, to_move, to_move, 'fk_rails_04850db4b4') + merge_data.build_more_data(to_move, data: data) + merge_data.build_more_data(move_back, data: data) + swap_submission(model, move_back, table, to_move, to_move, 'fk_rails_04850db4b4') end rescue => e Rails.logger.error "migrating #{table} column user_id failed: #{e}" end end - user_merge_data.bulk_insert_merge_data(data) + merge_data.bulk_insert_merge_data(data) @data = [] end - def swap_submission(model, move_back, table, target_user, to_move, to_move_ids, fk) + def swap_submission(model, move_back, table, to_move, to_move_ids, fk) model.transaction do # there is a unique index on assignment_id and user_id. Unique # indexes are checked after every row during an update statement @@ -491,12 +504,12 @@ class UserMerge model.where(id: move_back).update_all(user_id: -from_user.id) if target_user.shard == from_user.shard model.where(id: to_move_ids).update_all(user_id: target_user.id) model.where(id: move_back).update_all(user_id: from_user.id) - update_versions(from_user, target_user, model.where(id: to_move), table, :user_id) - update_versions(target_user, from_user, model.where(id: move_back), table, :user_id) + update_versions(model.where(id: to_move), table, :user_id) + update_versions(model.where(id: move_back), table, :user_id) end end - def update_versions(from_user, target_user, scope, table, column) + def update_versions(scope, table, column) scope.find_ids_in_batches do |ids| versionable_type = table.to_s.classify # TODO: This is a hack to support namespacing diff --git a/spec/lib/user_merge_spec.rb b/spec/lib/user_merge_spec.rb index ce4310f70b4..1990a8f088f 100644 --- a/spec/lib/user_merge_spec.rb +++ b/spec/lib/user_merge_spec.rb @@ -38,7 +38,7 @@ describe UserMerge do UserMerge.from(user2).into(user1) merge_data = UserMergeData.where(user_id: user1).first expect(merge_data.from_user_id).to eq user2.id - expect(merge_data.user_merge_data_records.pluck(:context_id).sort).to eq [pseudonym.id, pseudonym2.id].sort + expect(merge_data.records.pluck(:context_id).sort).to eq [pseudonym.id, pseudonym2.id].sort user2.reload expect(user2.pseudonyms).to be_empty user1.reload @@ -60,7 +60,7 @@ describe UserMerge do UserMerge.from(user2).into(user1) merge_data = UserMergeData.where(user_id: user1).first expect(merge_data.from_user_id).to eq user2.id - expect(merge_data.user_merge_data_records.where(context_type: 'AccountUser').first.context_id).to eq admin.id + expect(merge_data.records.where(context_type: 'AccountUser').first.context_id).to eq admin.id user1.reload expect(user1.account_users.first.id).to eq admin.id end @@ -237,7 +237,7 @@ describe UserMerge do UserMerge.from(user1).into(user2) user1.reload user2.reload - records = UserMergeData.where(user_id: user2).take.user_merge_data_records + records = UserMergeData.where(user_id: user2).take.records expect(records.count).to eq 8 record = records.where(context_id: cc1).take expect(record.previous_user_id).to eq user1.id @@ -286,12 +286,12 @@ describe UserMerge do UserMerge.from(user1).into(user2) merge_data = UserMergeData.where(user_id: user2).first - expect(merge_data.user_merge_data_records.pluck(:context_id).sort). + expect(merge_data.records.pluck(:context_id).sort). to eq [enrollment1.id, enrollment3.id, enrollment4.id].sort enrollment1.reload expect(enrollment1.user).to eq user1 expect(enrollment1).to be_deleted - merge_data_record = merge_data.user_merge_data_records.where(context_id: enrollment1).first + merge_data_record = merge_data.records.where(context_id: enrollment1).first expect(merge_data_record.previous_workflow_state).to eq 'invited' enrollment2.reload expect(enrollment2).to be_active @@ -312,20 +312,20 @@ describe UserMerge do UserMerge.from(user2).into(user1) merge_data = UserMergeData.where(user_id: user1).first - expect(merge_data.user_merge_data_records.pluck(:context_id).sort). + expect(merge_data.records.pluck(:context_id).sort). to eq [enrollment1.id, enrollment2.id].sort enrollment1.reload expect(enrollment1.user).to eq user1 expect(enrollment1.workflow_state).to eq 'active' expect(enrollment1.enrollment_state.state).to eq 'active' - merge_data_record = merge_data.user_merge_data_records.where(context_id: enrollment1).first + merge_data_record = merge_data.records.where(context_id: enrollment1).first expect(merge_data_record.previous_workflow_state).to eq 'invited' enrollment2.reload expect(enrollment2.user).to eq user2 expect(enrollment2.workflow_state).to eq 'deleted' expect(enrollment2.enrollment_state.state).to eq 'deleted' - merge_data_record2 = merge_data.user_merge_data_records.where(context_id: enrollment2).first + merge_data_record2 = merge_data.records.where(context_id: enrollment2).first expect(merge_data_record2.previous_workflow_state).to eq 'active' end @@ -366,7 +366,7 @@ describe UserMerge do UserMerge.from(user1).into(user2) merge_data = UserMergeData.where(user_id: user2).first - o = merge_data.user_merge_data_records.where(context_id: enrollment2).first + o = merge_data.records.where(context_id: enrollment2).first expect(o.previous_workflow_state).to eq 'active' expect(enrollment1.reload.user).to eql user2 expect(enrollment2.reload.workflow_state).to eql 'deleted' @@ -412,7 +412,7 @@ describe UserMerge do UserMerge.from(user1).into(user2) data = UserMergeData.where(user_id: user2).first - expect(data.user_merge_data_records.where(context_type: 'UserObservationLink').count).to eq 2 + expect(data.records.where(context_type: 'UserObservationLink').count).to eq 2 user1.reload expect(user1.linked_observers).to be_empty expect(UserObservationLink.where(:student => user1).first.workflow_state).to eq 'deleted' diff --git a/spec/models/split_user_spec.rb b/spec/models/split_user_spec.rb index bc8cb152d37..42de709a5e6 100644 --- a/spec/models/split_user_spec.rb +++ b/spec/models/split_user_spec.rb @@ -28,6 +28,58 @@ describe SplitUsers do let(:account1) { Account.default } let(:sub_account) { account1.sub_accounts.create! } + it 'should restore terms_of use one way' do + user1.accept_terms + user1.save! + UserMerge.from(user2).into(user1) + SplitUsers.split_db_users(user1) + expect(user2.reload.preferences[:accepted_terms]).to be_nil + expect(user1.reload.preferences[:accepted_terms]).to_not be_nil + end + + it 'should restore terms_of use other way' do + user1.accept_terms + user1.save! + UserMerge.from(user1).into(user2) + expect(user2.reload.preferences[:accepted_terms]).to_not be_nil + SplitUsers.split_db_users(user2) + expect(user1.reload.preferences[:accepted_terms]).to_not be_nil + expect(user2.reload.preferences[:accepted_terms]).to be_nil + end + + it 'should restore terms_of use no way' do + UserMerge.from(user1).into(user2) + user2.accept_terms + user2.save! + SplitUsers.split_db_users(user2) + expect(user2.reload.preferences[:accepted_terms]).to be_nil + expect(user1.reload.preferences[:accepted_terms]).to be_nil + end + + it 'should restore terms_of use both ways' do + user1.accept_terms + user1.save! + user2.accept_terms + user2.save! + UserMerge.from(user1).into(user2) + SplitUsers.split_db_users(user2) + expect(user2.reload.preferences[:accepted_terms]).to_not be_nil + expect(user1.reload.preferences[:accepted_terms]).to_not be_nil + end + + it 'should restore names' do + user1.name = "jimmy one" + user1.save! + user2.name = "jenny one" + user2.save! + UserMerge.from(user1).into(user2) + user2.name = "other name" + user2.save! + SplitUsers.split_db_users(user2) + expect(user1.reload.name).to eq "jimmy one" + expect(user2.reload.name).to eq "jenny one" + end + it 'should restore pseudonyms to the original user' do pseudonym1 = user1.pseudonyms.create!(unique_id: 'sam1@example.com') pseudonym2 = user2.pseudonyms.create!(unique_id: 'sam2@example.com')