diff --git a/app/models/user.rb b/app/models/user.rb index e2dd820fa6..cd793d9f07 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -44,6 +44,7 @@ class User < ApplicationRecord has_many :events, dependent: :restrict_with_error has_many :exports, dependent: :destroy has_many :notifications, dependent: :destroy + has_many :evaluation_users, inverse_of: :user, dependent: :restrict_with_error has_one :rights_request, dependent: :destroy has_many :subscribed_courses, @@ -290,6 +291,81 @@ def set_search self.search = "#{username || ''} #{first_name || ''} #{last_name || ''}" end + def merge_into(other, force: false) + errors.add(:merge, 'User belongs to different institution') if other.institution_id != institution_id && other.institution_id.present? && institution_id.present? + errors.add(:merge, 'User has different permissions') if other.permission != permission && !force + return false if errors.any? + + transaction do + other.permission = permission if (permission == 'staff' && other.permission == 'student') \ + || (permission == 'zeus' && other.permission != 'zeus') + + other.institution_id = institution_id if other.institution_id.nil? + + identities.each do |i| + if other.identities.find { |oi| oi.provider_id == i.provider_id } + i.destroy! + else + i.update!(user: other) + end + end + + rights_request.update!(user: other) if !rights_request.nil? && other.permission == 'student' && other.rights_request.nil? + + course_memberships.each do |cm| + other_cm = other.course_memberships.find { |ocm| ocm.course_id == cm.course_id } + if other_cm.nil? + cm.update!(user: other) + elsif other_cm.status == cm.status \ + || other_cm.status == 'course_admin' \ + || (other_cm.status == 'student' && cm.status != 'course_admin') \ + || (other_cm.status == 'unsubscribed' && cm.status == 'pending') + other_cm.update!(favorite: true) if cm.favorite + cm.destroy! + else + cm.update!(favorite: true) if other_cm.favorite + other_cm.destroy! + cm.update!(user: other) + end + end + + submissions.each { |s| s.update!(user: other) } + api_tokens.each { |at| at.update!(user: other) } + events.each { |e| e.update!(user: other) } + exports.each { |e| e.update!(user: other) } + notifications.each { |n| n.update!(user: other) } + annotations.each { |a| a.update!(user: other, last_updated_by_id: other.id) } + questions.each { |q| q.update!(user: other) } + + evaluation_users.each do |eu| + if other.evaluation_users.find { |oeu| oeu.evaluation_id == eu.evaluation_id } + eu.destroy! + else + eu.update!(user: other) + end + end + + activity_read_states.each do |ars| + if other.activity_read_states.find { |oars| oars.activity_id == ars.activity_id } + ars.destroy! + else + ars.update!(user: other) + end + end + + repository_admins.each do |ra| + if other.repository_admins.find { |ora| ora.repository_id == ra.repository_id } + ra.destroy! + else + ra.update!(user: other) + end + end + + reload + destroy! + end + end + private def set_token diff --git a/lib/tasks/merge_users.rake b/lib/tasks/merge_users.rake new file mode 100644 index 0000000000..d3a1afb5df --- /dev/null +++ b/lib/tasks/merge_users.rake @@ -0,0 +1,63 @@ + +task :merge_users, [:arg1, :arg2] => :environment do |task, args| + u1_id = args[:arg1].to_i + u2_id = args[:arg2].to_i + + MergeUsers.new.merge_users_interactive u1_id, u2_id +end + +class MergeUsers + LABEL_WIDTH = 20 + USER_WIDTH = 50 + + def initialize(input = $stdin, output = $stdout) + @input = input + @output = output + end + + def print_users(*users) + @output.puts 'Username: '.ljust(LABEL_WIDTH) + users.map { |u| u.username.to_s.ljust(USER_WIDTH) }.join + @output.puts 'Email: '.ljust(LABEL_WIDTH) + users.map { |u| u.email.to_s.ljust(USER_WIDTH) }.join + @output.puts 'Name: '.ljust(LABEL_WIDTH) + users.map { |u| u.full_name.to_s.ljust(USER_WIDTH) }.join + @output.puts 'Permission: '.ljust(LABEL_WIDTH) + users.map { |u| u.permission.to_s.ljust(USER_WIDTH) }.join + + @output.puts 'Courses: '.ljust(LABEL_WIDTH) + users.map { |u| u.courses.count.to_s.ljust(USER_WIDTH) }.join + @output.puts 'Submissions: '.ljust(LABEL_WIDTH) + users.map { |u| u.submissions.count.to_s.ljust(USER_WIDTH) }.join + @output.puts 'Read states: '.ljust(LABEL_WIDTH) + users.map { |u| u.activity_read_states.count.to_s.ljust(USER_WIDTH) }.join + @output.puts 'Repositories: '.ljust(LABEL_WIDTH) + users.map { |u| u.repositories.count.to_s.ljust(USER_WIDTH) }.join + @output.puts 'Evaluations: '.ljust(LABEL_WIDTH) + users.map { |u| u.evaluation_users.count.to_s.ljust(USER_WIDTH) }.join + end + + def merge_users_interactive(u1_id, u2_id) + u1 = User.find(u1_id) + u2 = User.find(u2_id) + + print_users u1, u2 + + @output.puts '' + + c = '' + until %W[\r \n y n f].include? c + u1, u2 = u2, u1 if c == 's' + @output.puts "Invalid input #{c}" unless ['s', ''].include?(c) + @output.puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce|(s)wap" + c = @input.getch.downcase + end + + @output.puts '' + + if %W[\r \n n].include? c + @output.puts 'Merge cancelled' + return + end + + success = u1.merge_into(u2, force: c == 'f') + if success + @output.puts "Successfully merged #{u1.username} into #{u2.username}" + print_users u2 + else + @output.puts 'Merge failed' + @output.puts u1.errors.full_messages.join('\n') + end + end +end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index b11757e95d..3b18674f41 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -365,4 +365,235 @@ def setup @user.subscribed_courses.first.update(year: '2') assert_equal [@user.subscribed_courses.first], @user.drawer_courses end + + test 'user should be removed after merge' do + u1 = create :user + u2 = create :user + u1.merge_into(u2) + + assert_not u1.persisted? + end + + test 'merge should fail if institutions are different' do + i1 = create :institution + i2 = create :institution + u1 = create :user, institution: i1 + u2 = create :user, institution: i2 + + result = u1.merge_into(u2) + + assert_not result + assert u1.persisted? + end + + test 'merge should succeed if only one institution is set' do + i1 = create :institution + u1 = create :user, institution: i1 + u2 = create :user + + result = u1.merge_into(u2) + + assert result + assert_not u1.persisted? + assert_equal i1, u2.institution + end + + test 'merge should fail if permissions are different' do + u1 = create :user, permission: 'student' + u2 = create :user, permission: 'staff' + + result = u1.merge_into(u2) + + assert_not result + assert u1.persisted? + end + + test 'merge should take highest permission if force is used' do + u1 = create :user, permission: 'zeus' + u2 = create :user, permission: 'staff' + + result = u1.merge_into(u2, force: true) + + assert result + assert_not u1.persisted? + assert_equal 'zeus', u2.permission + end + + test 'merge should transfer all associated objects to the other user' do + u1 = create :user + u2 = create :user + + [u1, u2].each do |u| + c = create :course + s = create :submission, user: u, course: c + create :api_token, user: u + create :event, user: u + create :export, user: u + create :notification, user: u + create :annotation, user: u, submission: s + create :question, submission: s + end + + result = u1.merge_into(u2) + + assert result + assert_not u1.persisted? + assert_equal 2, u2.submissions.count + assert_equal 2, u2.api_tokens.count + assert_equal 2, u2.events.count + assert_equal 2, u2.exports.count + assert_equal 4, u2.notifications.count + assert_equal 4, u2.annotations.count + assert_equal 2, u2.questions.count + end + + test 'merge should only transfer unique read states to the other user' do + u1 = create :user + u2 = create :user + + a1 = create :content_page + a2 = create :content_page + create :activity_read_state, user: u1, activity: a1 + create :activity_read_state, user: u2, activity: a1 + create :activity_read_state, user: u1, activity: a2 + + result = u1.merge_into(u2) + + assert result + assert_not u1.persisted? + assert_equal 2, u2.activity_read_states.count + end + + test 'merge should only transfer unique identities to the other user' do + u1 = create :user + u2 = create :user + + p1 = create :provider + p2 = create :provider + Identity.create user: u1, provider: p1, identifier: 'a' + Identity.create user: u2, provider: p1, identifier: 'b' + Identity.create user: u1, provider: p2, identifier: 'c' + + result = u1.merge_into(u2) + + assert result + assert_not u1.persisted? + assert_equal 2, u2.identities.count + end + + test 'merge should only transfer unique repositories to the other user' do + u1 = create :user + u2 = create :user + + r1 = create :repository, :git_stubbed + r2 = create :repository, :git_stubbed + RepositoryAdmin.create user: u1, repository: r1 + RepositoryAdmin.create user: u2, repository: r1 + RepositoryAdmin.create user: u1, repository: r2 + + result = u1.merge_into(u2) + + assert result + assert_not u1.persisted? + assert_equal 2, u2.repository_admins.count + end + + test 'merge should only transfer unique evaluations to the other user' do + u1 = create :user + u2 = create :user + + e1 = create :evaluation + e2 = create :evaluation + EvaluationUser.create user: u1, evaluation: e1 + EvaluationUser.create user: u2, evaluation: e1 + EvaluationUser.create user: u1, evaluation: e2 + + result = u1.merge_into(u2) + + assert result + assert_not u1.persisted? + assert_equal 2, u2.evaluation_users.count + end + + test 'merge should transfer course membership with most rights to the other user' do + u1 = create :user + u2 = create :user + + c1 = create :course + c2 = create :course + c3 = create :course + c4 = create :course + c5 = create :course + CourseMembership.create user: u1, course: c1, status: 'student', favorite: true + CourseMembership.create user: u1, course: c2, status: 'pending' + CourseMembership.create user: u1, course: c3, status: 'unsubscribed' + CourseMembership.create user: u1, course: c4, status: 'student' + CourseMembership.create user: u1, course: c5, status: 'course_admin' + + CourseMembership.create user: u2, course: c2, status: 'pending' + CourseMembership.create user: u2, course: c3, status: 'course_admin', favorite: true + CourseMembership.create user: u2, course: c4, status: 'unsubscribed' + CourseMembership.create user: u2, course: c5, status: 'student' + + result = u1.merge_into(u2) + + assert_equal 0, u1.course_memberships.count + + assert result + assert_not u1.persisted? + assert_equal 5, u2.course_memberships.count + assert_equal 4, u2.subscribed_courses.count + assert_equal 2, u2.favorite_courses.count + assert_equal 2, u2.administrating_courses.count + assert_equal 2, u2.enrolled_courses.count + assert_equal 1, u2.pending_courses.count + assert_equal 0, u2.unsubscribed_courses.count + end + + test 'merge should transfer update cached values' do + u1 = create :user + u2 = create :user + + c = create :course + c2 = create :course + CourseMembership.create user: u1, course: c, status: 'student' + CourseMembership.create user: u2, course: c, status: 'student' + s1 = create :series, course: c, exercise_count: 0 + s2 = create :series, course: c2, exercise_count: 0 + e1 = create :exercise + e2 = create :exercise + e3 = create :exercise + SeriesMembership.create series: s1, activity: e1 + SeriesMembership.create series: s1, activity: e2 + SeriesMembership.create series: s2, activity: e3 + create :correct_submission, user: u2, course: c, exercise: e1 + create :wrong_submission, user: u2, course: c, exercise: e2 + create :correct_submission, user: u1, course: c, exercise: e1 + create :correct_submission, user: u1, course: c, exercise: e2 + create :wrong_submission, user: u1, course: c2, exercise: e3 + + assert_equal 3, c.correct_solutions + assert_equal 2, c.subscribed_members_count + assert_equal 1, u2.correct_exercises + assert_equal 2, u2.attempted_exercises + assert_equal 2, e1.users_correct + assert_equal 2, e1.users_tried + assert_equal false, s1.completed?(user: u2) + assert_equal false, s2.started?(user: u2) + assert_equal false, s2.wrong?(user: u2) + + result = u1.merge_into(u2) + + assert result + assert_not u1.persisted? + assert_equal 1, c.subscribed_members_count + assert_equal 2, c.correct_solutions + assert_equal 2, u2.correct_exercises + assert_equal 3, u2.attempted_exercises + assert_equal 1, e1.users_correct + assert_equal 1, e1.users_tried + assert_equal true, s1.completed?(user: u2) + assert_equal true, s2.started?(user: u2) + assert_equal true, s2.wrong?(user: u2) + end end diff --git a/test/tasks/merge_users_test.rb b/test/tasks/merge_users_test.rb new file mode 100644 index 0000000000..835378ef51 --- /dev/null +++ b/test/tasks/merge_users_test.rb @@ -0,0 +1,89 @@ +require 'test_helper' +require 'stringio' +require 'rake' +load './lib/tasks/merge_users.rake' + +class MergeUserTest < ActiveSupport::TestCase + def stub_stdin(*chars) + string_io = StringIO.new + chars.each { |c| string_io.print c } + string_io.rewind + string_io + end + + def merge_users_interactive(u1_id, u2_id, *chars) + input = stub_stdin(*chars) + output = File.open(File::NULL, 'w') + MergeUsers.new(input, output).merge_users_interactive(u1_id, u2_id) + end + + test 'The script should cancel on no' do + u1 = create :user + u2 = create :user + + merge_users_interactive u1.id, u2.id, 'n' + + assert User.exists?(u1.id) + assert User.exists?(u2.id) + end + + test 'The script should cancel on enter' do + u1 = create :user + u2 = create :user + + merge_users_interactive u1.id, u2.id, '\n' + + assert User.exists?(u1.id) + assert User.exists?(u2.id) + end + + test 'The script should merge on yes' do + u1 = create :user + u2 = create :user + + merge_users_interactive u1.id, u2.id, 'y' + + assert_not User.exists?(u1.id) + assert User.exists?(u2.id) + end + + test 'The script should swap on swap' do + u1 = create :user + u2 = create :user + + merge_users_interactive u1.id, u2.id, 's', 'y' + + assert User.exists?(u1.id) + assert_not User.exists?(u2.id) + end + + test 'The script should ignore incorrect input' do + u1 = create :user + u2 = create :user + + merge_users_interactive u1.id, u2.id, 'a', 'z', 'd', 'y' + + assert_not User.exists?(u1.id) + assert User.exists?(u2.id) + end + + test 'The script should fail on yes with different permissions' do + u1 = create :user, permission: 'student' + u2 = create :user, permission: 'zeus' + + merge_users_interactive u1.id, u2.id, 'y' + + assert User.exists?(u1.id) + assert User.exists?(u2.id) + end + + test 'The script should merge on force with different permissions' do + u1 = create :user, permission: 'student' + u2 = create :user, permission: 'zeus' + + merge_users_interactive u1.id, u2.id, 'f' + + assert_not User.exists?(u1.id) + assert User.exists?(u2.id) + end +end