From 6affc1591a5f914e5a50d81ab8b197483be222b8 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Mon, 24 Jan 2022 15:43:47 +0100
Subject: [PATCH 01/21] WIP: create first draft of merge_into method

---
 app/models/user.rb | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/app/models/user.rb b/app/models/user.rb
index e2dd820fa6..c65b5d7c95 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -323,4 +323,45 @@ def split_last_name
     self.first_name = parts[0]
     self.last_name = parts[1]
   end
+
+  def institution_id
+    identities.map(&:provider).map(&:institution_id).uniq.pop
+  end
+
+  def merge_into(other, force: false)
+    course_memberships_with_different_status = other.course_memberships.join()
+    errors.add(:merge, 'User belongs to different institution') if other.institution_id != institution_id && other.institution_id.present? && institution_id.present?
+    unless force
+      errors.add(:merge, 'User has different permissions') if other.permission != permission
+    end
+    return false if errors.any?
+
+    activity_read_states.each { |ars| ars.update(user: other) }
+    submissions.each { |s| s.update(user: other) }
+
+    api_tokens.each { |at| at.update(user: other) }
+    identities.each { |i| i.update(user: other) }
+    events.each { |e| e.update(user: other) }
+    exports.each { |e| e.update(user: other) }
+    notifications.each { |n| n.update(user: other) }
+    rights_request.update(user: other)
+
+
+    courses.each { |c| c.update(user: other) }
+    course_memberships.each { |cm| cm.update(user: other) }
+    # subscribed_courses.each { |c| c.update(user: other) }
+    # favorite_courses.each { |c| c.update(user: other) }
+    # administrating_courses.each { |c| c.update(user: other) }
+    # enrolled_courses.each { |c| c.update(user: other) }
+    # pending_courses.each { |c| c.update(user: other) }
+    # unsubscribed_courses.each { |c| c.update(user: other) }
+    #
+    repositories.each { |r| r.update(user: other) }
+    repository_admins.each { |ra| ra.update(user: other) }
+
+    annotations.each { |a| a.update(user: other) }
+    questions.each { |q| q.update(user: other) }
+
+
+  end
 end

From 6364aa23913ecf029f159db12b34e10c34170b95 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Tue, 25 Jan 2022 14:26:44 +0100
Subject: [PATCH 02/21] Create merge function for users

---
 app/models/user.rb | 85 ++++++++++++++++++++++++----------------------
 1 file changed, 45 insertions(+), 40 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index c65b5d7c95..b6add329d3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -290,6 +290,51 @@ 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?
+
+    other.permission = permission if (permission == "staff" && other.permission == "student") \
+                                  || (permission == "zeus" && other.permission != "zeus")
+
+    activity_read_states.each { |ars| ars.update(user: other) }
+    submissions.each { |s| s.update(user: other) }
+
+    api_tokens.each { |at| at.update(user: other) }
+    identities.each { |i| i.update(user: other) unless other.identities.find { |oi| oi.provider_id == i.provider_id } }
+    events.each { |e| e.update(user: other) }
+    exports.each { |e| e.update(user: other) }
+    notifications.each { |n| n.update(user: other) }
+
+    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 == "pending" \
+        || (other_cm.status == "unsubscribed" && cm.status == "pending") \
+        || (other_cm.status == "student" && cm.status != "course_admin")
+        other_cm.update(favorite: true) if cm.favorite
+        cm.delete
+      else
+        cm.update(favorite: true) if other_cm.favorite
+        cm.update(user: other)
+        other_cm.delete
+      end
+    end
+
+    repository_admins.each { |ra| ra.update(user: other) unless other.repositories.find(ra.repository_id) }
+
+    annotations.each { |a| a.update(user: other) }
+    questions.each { |q| q.update(user: other) }
+
+    reload
+    destroy
+  end
+
   private
 
   def set_token
@@ -324,44 +369,4 @@ def split_last_name
     self.last_name = parts[1]
   end
 
-  def institution_id
-    identities.map(&:provider).map(&:institution_id).uniq.pop
-  end
-
-  def merge_into(other, force: false)
-    course_memberships_with_different_status = other.course_memberships.join()
-    errors.add(:merge, 'User belongs to different institution') if other.institution_id != institution_id && other.institution_id.present? && institution_id.present?
-    unless force
-      errors.add(:merge, 'User has different permissions') if other.permission != permission
-    end
-    return false if errors.any?
-
-    activity_read_states.each { |ars| ars.update(user: other) }
-    submissions.each { |s| s.update(user: other) }
-
-    api_tokens.each { |at| at.update(user: other) }
-    identities.each { |i| i.update(user: other) }
-    events.each { |e| e.update(user: other) }
-    exports.each { |e| e.update(user: other) }
-    notifications.each { |n| n.update(user: other) }
-    rights_request.update(user: other)
-
-
-    courses.each { |c| c.update(user: other) }
-    course_memberships.each { |cm| cm.update(user: other) }
-    # subscribed_courses.each { |c| c.update(user: other) }
-    # favorite_courses.each { |c| c.update(user: other) }
-    # administrating_courses.each { |c| c.update(user: other) }
-    # enrolled_courses.each { |c| c.update(user: other) }
-    # pending_courses.each { |c| c.update(user: other) }
-    # unsubscribed_courses.each { |c| c.update(user: other) }
-    #
-    repositories.each { |r| r.update(user: other) }
-    repository_admins.each { |ra| ra.update(user: other) }
-
-    annotations.each { |a| a.update(user: other) }
-    questions.each { |q| q.update(user: other) }
-
-
-  end
 end

From 8c58143baea66193208b5ee6dd1b4cb29e4cabca Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Tue, 25 Jan 2022 16:17:36 +0100
Subject: [PATCH 03/21] Add tests

---
 app/models/user.rb       |  48 +++++++++++-----
 test/models/user_test.rb | 116 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 14 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index b6add329d3..5027f06b66 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -295,28 +295,53 @@ def merge_into(other, force: false)
     errors.add(:merge, 'User has different permissions') if other.permission != permission && !force
     return false if errors.any?
 
-    other.permission = permission if (permission == "staff" && other.permission == "student") \
-                                  || (permission == "zeus" && other.permission != "zeus")
+    other.permission = permission if (permission == 'staff' && other.permission == 'student') \
+                                  || (permission == 'zeus' && other.permission != 'zeus')
 
-    activity_read_states.each { |ars| ars.update(user: other) }
-    submissions.each { |s| s.update(user: other) }
+    other.institution_id = institution_id if other.institution_id.nil?
+
+    rights_request.update(user: other) if !rights_request.nil? && other.permission == 'student' && other.rights_request.nil?
 
+    submissions.each { |s| s.update(user: other) }
     api_tokens.each { |at| at.update(user: other) }
-    identities.each { |i| i.update(user: other) unless other.identities.find { |oi| oi.provider_id == i.provider_id } }
     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) }
+
+    activity_read_states.each do |ars|
+      if other.activity_read_states.find { |oars| oars.activity_id == ars.activity_id }
+        ars.delete
+      else
+        ars.update(user: other)
+      end
+    end
+
+    identities.each do |i|
+      if other.identities.find { |oi| oi.provider_id == i.provider_id }
+        i.delete
+      else
+        i.update(user: other)
+      end
+    end
 
-    rights_request.update(user: other) if !rights_request.nil? && other.permission == "student" && other.rights_request.nil?
+    repository_admins.each do |ra|
+      if other.repositories.find(ra.repository_id)
+        other.delete
+      else
+        ra.update(user: other)
+      end
+    end
 
     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 == "pending" \
-        || (other_cm.status == "unsubscribed" && cm.status == "pending") \
-        || (other_cm.status == "student" && cm.status != "course_admin")
+        || other_cm.status == 'pending' \
+        || (other_cm.status == 'unsubscribed' && cm.status == 'pending') \
+        || (other_cm.status == 'student' && cm.status != 'course_admin')
         other_cm.update(favorite: true) if cm.favorite
         cm.delete
       else
@@ -326,11 +351,6 @@ def merge_into(other, force: false)
       end
     end
 
-    repository_admins.each { |ra| ra.update(user: other) unless other.repositories.find(ra.repository_id) }
-
-    annotations.each { |a| a.update(user: other) }
-    questions.each { |q| q.update(user: other) }
-
     reload
     destroy
   end
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index b11757e95d..c153288a3f 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -365,4 +365,120 @@ 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
+    i1 = create :institution
+    u1 = create :user, institution: i1
+    u2 = create :user, institution: i1
+
+    p1 = create :provider, institution: i1, mode: :prefer
+    p2 = create :provider, institution: i1, mode: :secondary
+    create :identity, user: u1, provider: p1
+    create :identity, user: u2, provider: p1
+    create :identity, user: u1, provider: p2
+
+    result = u1.merge_into(u2)
+
+    assert result
+    assert_not u1.persisted?
+    assert_equal 2, u2.identities.count
+  end
 end

From beb4f34000f3af19574a322a83ed28bfaf9ddea0 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Wed, 26 Jan 2022 10:35:50 +0100
Subject: [PATCH 04/21] Add repository and course membership merge tests

---
 app/models/user.rb       | 12 +++++-----
 test/models/user_test.rb | 52 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 5027f06b66..5d93d8bf57 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -327,8 +327,8 @@ def merge_into(other, force: false)
     end
 
     repository_admins.each do |ra|
-      if other.repositories.find(ra.repository_id)
-        other.delete
+      if other.repository_admins.find { |ora| ora.repository_id == ra.repository_id }
+        ra.delete
       else
         ra.update(user: other)
       end
@@ -339,15 +339,15 @@ def merge_into(other, force: false)
       if other_cm.nil?
         cm.update(user: other)
       elsif other_cm.status == cm.status \
-        || other_cm.status == 'pending' \
-        || (other_cm.status == 'unsubscribed' && cm.status == 'pending') \
-        || (other_cm.status == 'student' && cm.status != 'course_admin')
+        || 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.delete
       else
         cm.update(favorite: true) if other_cm.favorite
-        cm.update(user: other)
         other_cm.delete
+        cm.update(user: other)
       end
     end
 
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index c153288a3f..41c20d1560 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -481,4 +481,56 @@ def setup
     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 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
 end

From 421e3dfb12cf5a3d14a92c7ec9b33af3d226d868 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Wed, 26 Jan 2022 10:42:29 +0100
Subject: [PATCH 05/21] Fix merge identities test

---
 test/models/user_test.rb | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 41c20d1560..9feae60c76 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -465,15 +465,14 @@ def setup
   end
 
   test 'merge should only transfer unique identities to the other user' do
-    i1 = create :institution
-    u1 = create :user, institution: i1
-    u2 = create :user, institution: i1
+    u1 = create :user
+    u2 = create :user
 
-    p1 = create :provider, institution: i1, mode: :prefer
-    p2 = create :provider, institution: i1, mode: :secondary
-    create :identity, user: u1, provider: p1
-    create :identity, user: u2, provider: p1
-    create :identity, user: u1, provider: p2
+    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)
 

From 5b03663afaeceeaa9694b82d917f2b5b0d8a03ca Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Wed, 26 Jan 2022 10:43:37 +0100
Subject: [PATCH 06/21] Cleanup newline

---
 app/models/user.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 5d93d8bf57..82d336ad98 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -388,5 +388,4 @@ def split_last_name
     self.first_name = parts[0]
     self.last_name = parts[1]
   end
-
 end

From c35705a1ed695359a7351d5cc2e2844f913f4d89 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Wed, 26 Jan 2022 16:25:42 +0100
Subject: [PATCH 07/21] Make mergeing users a transaction

---
 app/models/user.rb | 102 +++++++++++++++++++++++----------------------
 1 file changed, 52 insertions(+), 50 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 82d336ad98..3e4cff6d0b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -295,64 +295,66 @@ def merge_into(other, force: false)
     errors.add(:merge, 'User has different permissions') if other.permission != permission && !force
     return false if errors.any?
 
-    other.permission = permission if (permission == 'staff' && other.permission == 'student') \
-                                  || (permission == 'zeus' && other.permission != 'zeus')
-
-    other.institution_id = institution_id if other.institution_id.nil?
-
-    rights_request.update(user: other) if !rights_request.nil? && other.permission == 'student' && other.rights_request.nil?
-
-    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) }
-
-    activity_read_states.each do |ars|
-      if other.activity_read_states.find { |oars| oars.activity_id == ars.activity_id }
-        ars.delete
-      else
-        ars.update(user: other)
+    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?
+
+      rights_request.update(user: other) if !rights_request.nil? && other.permission == 'student' && other.rights_request.nil?
+
+      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) }
+
+      activity_read_states.each do |ars|
+        if other.activity_read_states.find { |oars| oars.activity_id == ars.activity_id }
+          ars.delete
+        else
+          ars.update(user: other)
+        end
       end
-    end
 
-    identities.each do |i|
-      if other.identities.find { |oi| oi.provider_id == i.provider_id }
-        i.delete
-      else
-        i.update(user: other)
+      identities.each do |i|
+        if other.identities.find { |oi| oi.provider_id == i.provider_id }
+          i.delete
+        else
+          i.update(user: other)
+        end
       end
-    end
 
-    repository_admins.each do |ra|
-      if other.repository_admins.find { |ora| ora.repository_id == ra.repository_id }
-        ra.delete
-      else
-        ra.update(user: other)
+      repository_admins.each do |ra|
+        if other.repository_admins.find { |ora| ora.repository_id == ra.repository_id }
+          ra.delete
+        else
+          ra.update(user: other)
+        end
       end
-    end
 
-    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.delete
-      else
-        cm.update(favorite: true) if other_cm.favorite
-        other_cm.delete
-        cm.update(user: other)
+      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.delete
+        else
+          cm.update(favorite: true) if other_cm.favorite
+          other_cm.delete
+          cm.update(user: other)
+        end
       end
-    end
 
-    reload
-    destroy
+      reload
+      destroy
+    end
   end
 
   private

From 7b9fab25b6e236494fe4e905f1abdb36701024d4 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Wed, 26 Jan 2022 17:50:08 +0100
Subject: [PATCH 08/21] Create merge script

---
 app/runners/merge_users.rb | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 app/runners/merge_users.rb

diff --git a/app/runners/merge_users.rb b/app/runners/merge_users.rb
new file mode 100644
index 0000000000..56df2b626a
--- /dev/null
+++ b/app/runners/merge_users.rb
@@ -0,0 +1,38 @@
+LABEL_WIDTH = 20
+USER_WIDTH = 50
+
+u1_id = ARGV[0].to_i
+u2_id = ARGV[1].to_i
+
+u1 = User.find(u1_id)
+u2 = User.find(u2_id)
+
+puts 'Username: '.ljust(LABEL_WIDTH) + u1.username.to_s.ljust(USER_WIDTH) + u2.username
+puts 'Email: '.ljust(LABEL_WIDTH) + u1.email.to_s.ljust(USER_WIDTH) + u2.email
+puts 'Name: '.ljust(LABEL_WIDTH) + u1.full_name.to_s.ljust(USER_WIDTH) + u2.full_name
+puts 'Permission: '.ljust(LABEL_WIDTH) + u1.permission.to_s.ljust(USER_WIDTH) + u2.permission.to_s
+
+puts 'Courses: '.ljust(LABEL_WIDTH) + u1.courses.count.to_s.ljust(USER_WIDTH) + u2.courses.count.to_s
+puts 'Submissions: '.ljust(LABEL_WIDTH) + u1.submissions.count.to_s.ljust(USER_WIDTH) + u2.submissions.count.to_s
+puts 'Read states: '.ljust(LABEL_WIDTH) + u1.activity_read_states.count.to_s.ljust(USER_WIDTH) + u2.activity_read_states.count.to_s
+puts 'Repositories: '.ljust(LABEL_WIDTH) + u1.repositories.count.to_s.ljust(USER_WIDTH) + u2.repositories.count.to_s
+
+puts ''
+puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce"
+until %W[\r \n y n f].include?((c = $stdin.getch.downcase))
+  puts "Invalid input #{c}"
+  puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce"
+end
+
+case c
+when 'y'
+  success = u1.merge_into(u2)
+  puts u1.errors.full_messages.join('\n') unless success
+  puts "Successfully merged #{u1.username} into #{u2.username}" if success
+when 'f'
+  success = u1.merge_into(u2, force: true)
+  puts u1.errors.full_messages.join('\n') unless success
+  puts "Successfully merged #{u1.username} into #{u2.username}" if success
+else
+  puts "Merge cancelled"
+end

From 56cf7d210b673659d53bebf2f01c2a96505194c4 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Thu, 27 Jan 2022 09:48:53 +0100
Subject: [PATCH 09/21] Add swap argument

---
 app/runners/merge_users.rb | 55 ++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/app/runners/merge_users.rb b/app/runners/merge_users.rb
index 56df2b626a..aa68576257 100644
--- a/app/runners/merge_users.rb
+++ b/app/runners/merge_users.rb
@@ -1,38 +1,47 @@
 LABEL_WIDTH = 20
 USER_WIDTH = 50
 
+def print_users(*users)
+  puts 'Username: '.ljust(LABEL_WIDTH) + users.map { |u| u.username.to_s.ljust(USER_WIDTH) }.join
+  puts 'Email: '.ljust(LABEL_WIDTH) + users.map { |u| u.email.to_s.ljust(USER_WIDTH) }.join
+  puts 'Name: '.ljust(LABEL_WIDTH) + users.map { |u| u.full_name.to_s.ljust(USER_WIDTH) }.join
+  puts 'Permission: '.ljust(LABEL_WIDTH) + users.map { |u| u.permission.to_s.ljust(USER_WIDTH) }.join
+
+  puts 'Courses: '.ljust(LABEL_WIDTH) + users.map { |u| u.courses.count.to_s.ljust(USER_WIDTH) }.join
+  puts 'Submissions: '.ljust(LABEL_WIDTH) + users.map { |u| u.submissions.count.to_s.ljust(USER_WIDTH) }.join
+  puts 'Read states: '.ljust(LABEL_WIDTH) + users.map { |u| u.activity_read_states.count.to_s.ljust(USER_WIDTH) }.join
+  puts 'Repositories: '.ljust(LABEL_WIDTH) + users.map { |u| u.repositories.count.to_s.ljust(USER_WIDTH) }.join
+end
+
 u1_id = ARGV[0].to_i
 u2_id = ARGV[1].to_i
 
 u1 = User.find(u1_id)
 u2 = User.find(u2_id)
 
-puts 'Username: '.ljust(LABEL_WIDTH) + u1.username.to_s.ljust(USER_WIDTH) + u2.username
-puts 'Email: '.ljust(LABEL_WIDTH) + u1.email.to_s.ljust(USER_WIDTH) + u2.email
-puts 'Name: '.ljust(LABEL_WIDTH) + u1.full_name.to_s.ljust(USER_WIDTH) + u2.full_name
-puts 'Permission: '.ljust(LABEL_WIDTH) + u1.permission.to_s.ljust(USER_WIDTH) + u2.permission.to_s
-
-puts 'Courses: '.ljust(LABEL_WIDTH) + u1.courses.count.to_s.ljust(USER_WIDTH) + u2.courses.count.to_s
-puts 'Submissions: '.ljust(LABEL_WIDTH) + u1.submissions.count.to_s.ljust(USER_WIDTH) + u2.submissions.count.to_s
-puts 'Read states: '.ljust(LABEL_WIDTH) + u1.activity_read_states.count.to_s.ljust(USER_WIDTH) + u2.activity_read_states.count.to_s
-puts 'Repositories: '.ljust(LABEL_WIDTH) + u1.repositories.count.to_s.ljust(USER_WIDTH) + u2.repositories.count.to_s
+print_users u1, u2
 
 puts ''
-puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce"
-until %W[\r \n y n f].include?((c = $stdin.getch.downcase))
-  puts "Invalid input #{c}"
-  puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce"
+
+c = ''
+until %W[\r \n y n f].include? c
+  u1, u2 = u2, u1 if c == 's'
+  puts "Invalid input #{c}" unless c == 's'
+  puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce|(s)wap"
+  c = $stdin.getch.downcase
 end
 
-case c
-when 'y'
-  success = u1.merge_into(u2)
-  puts u1.errors.full_messages.join('\n') unless success
-  puts "Successfully merged #{u1.username} into #{u2.username}" if success
-when 'f'
-  success = u1.merge_into(u2, force: true)
-  puts u1.errors.full_messages.join('\n') unless success
-  puts "Successfully merged #{u1.username} into #{u2.username}" if success
-else
+if %W[\r \n n].include? c
   puts "Merge cancelled"
+  return
 end
+
+success = u1.merge_into(u2, force: c == 'f')
+if success
+  puts "Successfully merged #{u1.username} into #{u2.username}"
+  print_users u2
+else
+  puts "Merge failed"
+  puts u1.errors.full_messages.join('\n')
+end
+

From 0ff9aef172d16790c7f516da72669cbf41062024 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Thu, 27 Jan 2022 10:04:20 +0100
Subject: [PATCH 10/21] Fix linting

---
 app/runners/merge_users.rb | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/app/runners/merge_users.rb b/app/runners/merge_users.rb
index aa68576257..37715c2da9 100644
--- a/app/runners/merge_users.rb
+++ b/app/runners/merge_users.rb
@@ -1,3 +1,4 @@
+# rubocop:disable Rails/Output
 LABEL_WIDTH = 20
 USER_WIDTH = 50
 
@@ -32,7 +33,7 @@ def print_users(*users)
 end
 
 if %W[\r \n n].include? c
-  puts "Merge cancelled"
+  puts 'Merge cancelled'
   return
 end
 
@@ -41,7 +42,7 @@ def print_users(*users)
   puts "Successfully merged #{u1.username} into #{u2.username}"
   print_users u2
 else
-  puts "Merge failed"
+  puts 'Merge failed'
   puts u1.errors.full_messages.join('\n')
 end
-
+# rubocop:enable Rails/Output

From d6812957587bb9ddc503f0cfd8d14b387da6e889 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Thu, 27 Jan 2022 10:12:38 +0100
Subject: [PATCH 11/21] Remove invalid notice that shows top soon

---
 app/runners/merge_users.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/runners/merge_users.rb b/app/runners/merge_users.rb
index 37715c2da9..4a9103e9d7 100644
--- a/app/runners/merge_users.rb
+++ b/app/runners/merge_users.rb
@@ -27,11 +27,13 @@ def print_users(*users)
 c = ''
 until %W[\r \n y n f].include? c
   u1, u2 = u2, u1 if c == 's'
-  puts "Invalid input #{c}" unless c == 's'
+  puts "Invalid input #{c}" unless ['s', ''].include?(c)
   puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce|(s)wap"
   c = $stdin.getch.downcase
 end
 
+puts ''
+
 if %W[\r \n n].include? c
   puts 'Merge cancelled'
   return

From 9dfa2056e1ee50259f02c28f3317677db988efd2 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Thu, 27 Jan 2022 13:48:11 +0100
Subject: [PATCH 12/21] Move runner to take task and add tests

---
 app/runners/merge_users.rb     |  50 ----------------
 lib/tasks/merge_users.rake     |  55 +++++++++++++++++
 test/tasks/merge_users_test.rb | 105 +++++++++++++++++++++++++++++++++
 3 files changed, 160 insertions(+), 50 deletions(-)
 delete mode 100644 app/runners/merge_users.rb
 create mode 100644 lib/tasks/merge_users.rake
 create mode 100644 test/tasks/merge_users_test.rb

diff --git a/app/runners/merge_users.rb b/app/runners/merge_users.rb
deleted file mode 100644
index 4a9103e9d7..0000000000
--- a/app/runners/merge_users.rb
+++ /dev/null
@@ -1,50 +0,0 @@
-# rubocop:disable Rails/Output
-LABEL_WIDTH = 20
-USER_WIDTH = 50
-
-def print_users(*users)
-  puts 'Username: '.ljust(LABEL_WIDTH) + users.map { |u| u.username.to_s.ljust(USER_WIDTH) }.join
-  puts 'Email: '.ljust(LABEL_WIDTH) + users.map { |u| u.email.to_s.ljust(USER_WIDTH) }.join
-  puts 'Name: '.ljust(LABEL_WIDTH) + users.map { |u| u.full_name.to_s.ljust(USER_WIDTH) }.join
-  puts 'Permission: '.ljust(LABEL_WIDTH) + users.map { |u| u.permission.to_s.ljust(USER_WIDTH) }.join
-
-  puts 'Courses: '.ljust(LABEL_WIDTH) + users.map { |u| u.courses.count.to_s.ljust(USER_WIDTH) }.join
-  puts 'Submissions: '.ljust(LABEL_WIDTH) + users.map { |u| u.submissions.count.to_s.ljust(USER_WIDTH) }.join
-  puts 'Read states: '.ljust(LABEL_WIDTH) + users.map { |u| u.activity_read_states.count.to_s.ljust(USER_WIDTH) }.join
-  puts 'Repositories: '.ljust(LABEL_WIDTH) + users.map { |u| u.repositories.count.to_s.ljust(USER_WIDTH) }.join
-end
-
-u1_id = ARGV[0].to_i
-u2_id = ARGV[1].to_i
-
-u1 = User.find(u1_id)
-u2 = User.find(u2_id)
-
-print_users u1, u2
-
-puts ''
-
-c = ''
-until %W[\r \n y n f].include? c
-  u1, u2 = u2, u1 if c == 's'
-  puts "Invalid input #{c}" unless ['s', ''].include?(c)
-  puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce|(s)wap"
-  c = $stdin.getch.downcase
-end
-
-puts ''
-
-if %W[\r \n n].include? c
-  puts 'Merge cancelled'
-  return
-end
-
-success = u1.merge_into(u2, force: c == 'f')
-if success
-  puts "Successfully merged #{u1.username} into #{u2.username}"
-  print_users u2
-else
-  puts 'Merge failed'
-  puts u1.errors.full_messages.join('\n')
-end
-# rubocop:enable Rails/Output
diff --git a/lib/tasks/merge_users.rake b/lib/tasks/merge_users.rake
new file mode 100644
index 0000000000..6c429eaa0d
--- /dev/null
+++ b/lib/tasks/merge_users.rake
@@ -0,0 +1,55 @@
+
+task :merge_users, [:arg1, :arg2] => :environment do |task, args|
+  LABEL_WIDTH = 20
+  USER_WIDTH = 50
+
+  def print_users(*users)
+    puts 'Username: '.ljust(LABEL_WIDTH) + users.map { |u| u.username.to_s.ljust(USER_WIDTH) }.join
+    puts 'Email: '.ljust(LABEL_WIDTH) + users.map { |u| u.email.to_s.ljust(USER_WIDTH) }.join
+    puts 'Name: '.ljust(LABEL_WIDTH) + users.map { |u| u.full_name.to_s.ljust(USER_WIDTH) }.join
+    puts 'Permission: '.ljust(LABEL_WIDTH) + users.map { |u| u.permission.to_s.ljust(USER_WIDTH) }.join
+
+    puts 'Courses: '.ljust(LABEL_WIDTH) + users.map { |u| u.courses.count.to_s.ljust(USER_WIDTH) }.join
+    puts 'Submissions: '.ljust(LABEL_WIDTH) + users.map { |u| u.submissions.count.to_s.ljust(USER_WIDTH) }.join
+    puts 'Read states: '.ljust(LABEL_WIDTH) + users.map { |u| u.activity_read_states.count.to_s.ljust(USER_WIDTH) }.join
+    puts 'Repositories: '.ljust(LABEL_WIDTH) + users.map { |u| u.repositories.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
+
+    puts ''
+
+    c = ''
+    until %W[\r \n y n f].include? c
+      u1, u2 = u2, u1 if c == 's'
+      puts "Invalid input #{c}" unless ['s', ''].include?(c)
+      puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce|(s)wap"
+      c = $stdin.getch.downcase
+    end
+
+    puts ''
+
+    if %W[\r \n n].include? c
+      puts 'Merge cancelled'
+      return
+    end
+
+    success = u1.merge_into(u2, force: c == 'f')
+    if success
+      puts "Successfully merged #{u1.username} into #{u2.username}"
+      print_users u2
+    else
+      puts 'Merge failed'
+      puts u1.errors.full_messages.join('\n')
+    end
+  end
+
+  u1_id = args[:arg1].to_i
+  u2_id = args[:arg2].to_i
+
+  merge_users_interactive u1_id, u2_id
+end
diff --git a/test/tasks/merge_users_test.rb b/test/tasks/merge_users_test.rb
new file mode 100644
index 0000000000..8c1c17fae4
--- /dev/null
+++ b/test/tasks/merge_users_test.rb
@@ -0,0 +1,105 @@
+require 'test_helper'
+require 'rake'
+require 'stringio'
+
+class MergeUserTest < ActiveSupport::TestCase
+
+  def stub_stdin(*chars)
+    string_io = StringIO.new
+    string_io.puts chars.join
+    string_io.rewind
+    $stdin = string_io
+  end
+
+  def merge_users_interactive(u1_id, u2_id)
+    Rake.application.invoke_task "merge_users[#{u1_id}, #{u2_id}]"
+  end
+
+  setup do
+    Rake.application.rake_require 'tasks/merge_users'
+    Rake::Task.define_task(:environment)
+    $stdout = File.open(File::NULL, 'w')
+  end
+
+  teardown do
+    $stdin = STDIN
+    $stdout = STDOUT
+  end
+
+  test 'The script should cancel on no' do
+    u1 = create :user
+    u2 = create :user
+
+    stub_stdin('n')
+    merge_users_interactive u1.id, u2.id
+
+    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
+
+    stub_stdin('\n')
+    merge_users_interactive u1.id, u2.id
+
+    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
+
+    stub_stdin('y')
+    merge_users_interactive u1.id, u2.id
+
+    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
+
+    stub_stdin('s', 'y')
+    merge_users_interactive u1.id, u2.id
+
+    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
+
+    stub_stdin('a', 'z', 'd', 'y')
+    merge_users_interactive u1.id, u2.id
+
+    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'
+
+    stub_stdin('y')
+    merge_users_interactive u1.id, u2.id
+
+    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'
+
+    stub_stdin('f')
+    merge_users_interactive u1.id, u2.id
+
+    assert_not User.exists?(u1.id)
+    assert User.exists?(u2.id)
+  end
+end

From 7288c6b9abd14320d18dfd9fa1fa3227edf19297 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Thu, 27 Jan 2022 13:55:51 +0100
Subject: [PATCH 13/21] Fix linting

---
 test/tasks/merge_users_test.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/tasks/merge_users_test.rb b/test/tasks/merge_users_test.rb
index 8c1c17fae4..8ea94ba997 100644
--- a/test/tasks/merge_users_test.rb
+++ b/test/tasks/merge_users_test.rb
@@ -3,7 +3,6 @@
 require 'stringio'
 
 class MergeUserTest < ActiveSupport::TestCase
-
   def stub_stdin(*chars)
     string_io = StringIO.new
     string_io.puts chars.join

From 3581223f0a410544e2e0c8db780b78c5101fde4f Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Thu, 27 Jan 2022 14:50:07 +0100
Subject: [PATCH 14/21] Add unique user ids to reduce overlap when testing

---
 test/tasks/merge_users_test.rb | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/test/tasks/merge_users_test.rb b/test/tasks/merge_users_test.rb
index 8ea94ba997..412d74bf25 100644
--- a/test/tasks/merge_users_test.rb
+++ b/test/tasks/merge_users_test.rb
@@ -26,8 +26,8 @@ def merge_users_interactive(u1_id, u2_id)
   end
 
   test 'The script should cancel on no' do
-    u1 = create :user
-    u2 = create :user
+    u1 = create :user, id: 1_000_001
+    u2 = create :user, id: 1_000_002
 
     stub_stdin('n')
     merge_users_interactive u1.id, u2.id
@@ -37,8 +37,8 @@ def merge_users_interactive(u1_id, u2_id)
   end
 
   test 'The script should cancel on enter' do
-    u1 = create :user
-    u2 = create :user
+    u1 = create :user, id: 1_000_003
+    u2 = create :user, id: 1_000_004
 
     stub_stdin('\n')
     merge_users_interactive u1.id, u2.id
@@ -48,8 +48,8 @@ def merge_users_interactive(u1_id, u2_id)
   end
 
   test 'The script should merge on yes' do
-    u1 = create :user
-    u2 = create :user
+    u1 = create :user, id: 1_000_005
+    u2 = create :user, id: 1_000_006
 
     stub_stdin('y')
     merge_users_interactive u1.id, u2.id
@@ -59,8 +59,8 @@ def merge_users_interactive(u1_id, u2_id)
   end
 
   test 'The script should swap on swap' do
-    u1 = create :user
-    u2 = create :user
+    u1 = create :user, id: 1_000_007
+    u2 = create :user, id: 1_000_008
 
     stub_stdin('s', 'y')
     merge_users_interactive u1.id, u2.id
@@ -70,8 +70,8 @@ def merge_users_interactive(u1_id, u2_id)
   end
 
   test 'The script should ignore incorrect input' do
-    u1 = create :user
-    u2 = create :user
+    u1 = create :user, id: 1_000_009
+    u2 = create :user, id: 1_000_010
 
     stub_stdin('a', 'z', 'd', 'y')
     merge_users_interactive u1.id, u2.id
@@ -81,8 +81,8 @@ def merge_users_interactive(u1_id, u2_id)
   end
 
   test 'The script should fail on yes with different permissions' do
-    u1 = create :user, permission: 'student'
-    u2 = create :user, permission: 'zeus'
+    u1 = create :user, permission: 'student', id: 1_000_011
+    u2 = create :user, permission: 'zeus', id: 1_000_012
 
     stub_stdin('y')
     merge_users_interactive u1.id, u2.id
@@ -92,8 +92,8 @@ def merge_users_interactive(u1_id, u2_id)
   end
 
   test 'The script should merge on force  with different permissions' do
-    u1 = create :user, permission: 'student'
-    u2 = create :user, permission: 'zeus'
+    u1 = create :user, permission: 'student', id: 1_000_013
+    u2 = create :user, permission: 'zeus', id: 1_000_014
 
     stub_stdin('f')
     merge_users_interactive u1.id, u2.id

From d02e9829ad1ea1a76b23ca6ea15bb4177eb2a7b5 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Fri, 28 Jan 2022 09:49:54 +0100
Subject: [PATCH 15/21] Fix io stubbing in tests

---
 lib/tasks/merge_users.rake     | 53 +++++++++++++-----------
 test/tasks/merge_users_test.rb | 73 ++++++++++++++--------------------
 2 files changed, 59 insertions(+), 67 deletions(-)

diff --git a/lib/tasks/merge_users.rake b/lib/tasks/merge_users.rake
index 6c429eaa0d..b90f134dd2 100644
--- a/lib/tasks/merge_users.rake
+++ b/lib/tasks/merge_users.rake
@@ -1,18 +1,30 @@
 
 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)
-    puts 'Username: '.ljust(LABEL_WIDTH) + users.map { |u| u.username.to_s.ljust(USER_WIDTH) }.join
-    puts 'Email: '.ljust(LABEL_WIDTH) + users.map { |u| u.email.to_s.ljust(USER_WIDTH) }.join
-    puts 'Name: '.ljust(LABEL_WIDTH) + users.map { |u| u.full_name.to_s.ljust(USER_WIDTH) }.join
-    puts 'Permission: '.ljust(LABEL_WIDTH) + users.map { |u| u.permission.to_s.ljust(USER_WIDTH) }.join
-
-    puts 'Courses: '.ljust(LABEL_WIDTH) + users.map { |u| u.courses.count.to_s.ljust(USER_WIDTH) }.join
-    puts 'Submissions: '.ljust(LABEL_WIDTH) + users.map { |u| u.submissions.count.to_s.ljust(USER_WIDTH) }.join
-    puts 'Read states: '.ljust(LABEL_WIDTH) + users.map { |u| u.activity_read_states.count.to_s.ljust(USER_WIDTH) }.join
-    puts 'Repositories: '.ljust(LABEL_WIDTH) + users.map { |u| u.repositories.count.to_s.ljust(USER_WIDTH) }.join
+    @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
   end
 
   def merge_users_interactive(u1_id, u2_id)
@@ -21,35 +33,30 @@ task :merge_users, [:arg1, :arg2] => :environment do |task, args|
 
     print_users u1, u2
 
-    puts ''
+    @output.puts ''
 
     c = ''
     until %W[\r \n y n f].include? c
       u1, u2 = u2, u1 if c == 's'
-      puts "Invalid input #{c}" unless ['s', ''].include?(c)
-      puts "Are you sure you want to merge #{u1.username} into #{u2.username}? (y)es|(N)o|(f)orce|(s)wap"
-      c = $stdin.getch.downcase
+      @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
 
-    puts ''
+    @output.puts ''
 
     if %W[\r \n n].include? c
-      puts 'Merge cancelled'
+      @output.puts 'Merge cancelled'
       return
     end
 
     success = u1.merge_into(u2, force: c == 'f')
     if success
-      puts "Successfully merged #{u1.username} into #{u2.username}"
+      @output.puts "Successfully merged #{u1.username} into #{u2.username}"
       print_users u2
     else
-      puts 'Merge failed'
-      puts u1.errors.full_messages.join('\n')
+      @output.puts 'Merge failed'
+      @output.puts u1.errors.full_messages.join('\n')
     end
   end
-
-  u1_id = args[:arg1].to_i
-  u2_id = args[:arg2].to_i
-
-  merge_users_interactive u1_id, u2_id
 end
diff --git a/test/tasks/merge_users_test.rb b/test/tasks/merge_users_test.rb
index 412d74bf25..835378ef51 100644
--- a/test/tasks/merge_users_test.rb
+++ b/test/tasks/merge_users_test.rb
@@ -1,102 +1,87 @@
 require 'test_helper'
-require 'rake'
 require 'stringio'
+require 'rake'
+load './lib/tasks/merge_users.rake'
 
 class MergeUserTest < ActiveSupport::TestCase
   def stub_stdin(*chars)
     string_io = StringIO.new
-    string_io.puts chars.join
+    chars.each { |c| string_io.print c }
     string_io.rewind
-    $stdin = string_io
-  end
-
-  def merge_users_interactive(u1_id, u2_id)
-    Rake.application.invoke_task "merge_users[#{u1_id}, #{u2_id}]"
-  end
-
-  setup do
-    Rake.application.rake_require 'tasks/merge_users'
-    Rake::Task.define_task(:environment)
-    $stdout = File.open(File::NULL, 'w')
+    string_io
   end
 
-  teardown do
-    $stdin = STDIN
-    $stdout = STDOUT
+  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, id: 1_000_001
-    u2 = create :user, id: 1_000_002
+    u1 = create :user
+    u2 = create :user
 
-    stub_stdin('n')
-    merge_users_interactive u1.id, u2.id
+    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, id: 1_000_003
-    u2 = create :user, id: 1_000_004
+    u1 = create :user
+    u2 = create :user
 
-    stub_stdin('\n')
-    merge_users_interactive u1.id, u2.id
+    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, id: 1_000_005
-    u2 = create :user, id: 1_000_006
+    u1 = create :user
+    u2 = create :user
 
-    stub_stdin('y')
-    merge_users_interactive u1.id, u2.id
+    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, id: 1_000_007
-    u2 = create :user, id: 1_000_008
+    u1 = create :user
+    u2 = create :user
 
-    stub_stdin('s', 'y')
-    merge_users_interactive u1.id, u2.id
+    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, id: 1_000_009
-    u2 = create :user, id: 1_000_010
+    u1 = create :user
+    u2 = create :user
 
-    stub_stdin('a', 'z', 'd', 'y')
-    merge_users_interactive u1.id, u2.id
+    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', id: 1_000_011
-    u2 = create :user, permission: 'zeus', id: 1_000_012
+    u1 = create :user, permission: 'student'
+    u2 = create :user, permission: 'zeus'
 
-    stub_stdin('y')
-    merge_users_interactive u1.id, u2.id
+    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', id: 1_000_013
-    u2 = create :user, permission: 'zeus', id: 1_000_014
+    u1 = create :user, permission: 'student'
+    u2 = create :user, permission: 'zeus'
 
-    stub_stdin('f')
-    merge_users_interactive u1.id, u2.id
+    merge_users_interactive u1.id, u2.id, 'f'
 
     assert_not User.exists?(u1.id)
     assert User.exists?(u2.id)

From 6dcc200b9b99b0326cd02108679fe59a22527aea Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Mon, 31 Jan 2022 13:11:22 +0100
Subject: [PATCH 16/21] Merge evaluations and test caching refresh

---
 app/models/user.rb       |  9 ++++++
 test/models/user_test.rb | 64 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/app/models/user.rb b/app/models/user.rb
index 3e4cff6d0b..672c692a4a 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,
@@ -311,6 +312,14 @@ def merge_into(other, force: false)
       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.delete
+        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.delete
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 9feae60c76..3b18674f41 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -498,6 +498,23 @@ def setup
     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
@@ -532,4 +549,51 @@ def setup
     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

From b1859ba435620d508a29c7199c6ed2a40be4ddae Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Mon, 31 Jan 2022 13:15:26 +0100
Subject: [PATCH 17/21] Print evaluations

---
 lib/tasks/merge_users.rake | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/tasks/merge_users.rake b/lib/tasks/merge_users.rake
index b90f134dd2..d3a1afb5df 100644
--- a/lib/tasks/merge_users.rake
+++ b/lib/tasks/merge_users.rake
@@ -25,6 +25,7 @@ class MergeUsers
     @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)

From bd3090937daa6fee1f55a4b973a3755da73afd45 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Mon, 31 Jan 2022 14:03:12 +0100
Subject: [PATCH 18/21] Update order in merge

---
 app/models/user.rb       | 50 ++++++++++++++++++++--------------------
 test/models/user_test.rb |  2 ++
 2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 672c692a4a..1c4dd6f77f 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -302,8 +302,33 @@ def merge_into(other, force: false)
 
       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.delete
+        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.delete
+        else
+          cm.update(favorite: true) if other_cm.favorite
+          other_cm.delete
+          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) }
@@ -328,14 +353,6 @@ def merge_into(other, force: false)
         end
       end
 
-      identities.each do |i|
-        if other.identities.find { |oi| oi.provider_id == i.provider_id }
-          i.delete
-        else
-          i.update(user: other)
-        end
-      end
-
       repository_admins.each do |ra|
         if other.repository_admins.find { |ora| ora.repository_id == ra.repository_id }
           ra.delete
@@ -344,23 +361,6 @@ def merge_into(other, force: false)
         end
       end
 
-      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.delete
-        else
-          cm.update(favorite: true) if other_cm.favorite
-          other_cm.delete
-          cm.update(user: other)
-        end
-      end
-
       reload
       destroy
     end
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 3b18674f41..32bce7c44f 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -595,5 +595,7 @@ def setup
     assert_equal true, s1.completed?(user: u2)
     assert_equal true, s2.started?(user: u2)
     assert_equal true, s2.wrong?(user: u2)
+    assert_equal true, s3.completed?(user: u2)
+    assert_equal 1, c2.correct_solutions(user_id: u2.id)
   end
 end

From 84ae9b82d842639e688eda45fd4ed4a56cc21170 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Mon, 31 Jan 2022 14:03:41 +0100
Subject: [PATCH 19/21] Fix incorrect test

---
 test/models/user_test.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index 32bce7c44f..3b18674f41 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -595,7 +595,5 @@ def setup
     assert_equal true, s1.completed?(user: u2)
     assert_equal true, s2.started?(user: u2)
     assert_equal true, s2.wrong?(user: u2)
-    assert_equal true, s3.completed?(user: u2)
-    assert_equal 1, c2.correct_solutions(user_id: u2.id)
   end
 end

From fb8ab8602dc13c1457de65ca0cafcb6488d090af Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Mon, 31 Jan 2022 14:08:13 +0100
Subject: [PATCH 20/21] Replace delete by destroy

---
 app/models/user.rb | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 1c4dd6f77f..bb60c659f5 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -304,7 +304,7 @@ def merge_into(other, force: false)
 
       identities.each do |i|
         if other.identities.find { |oi| oi.provider_id == i.provider_id }
-          i.delete
+          i.destroy
         else
           i.update(user: other)
         end
@@ -321,10 +321,10 @@ def merge_into(other, force: false)
           || (other_cm.status == 'student' && cm.status != 'course_admin') \
           || (other_cm.status == 'unsubscribed' && cm.status == 'pending')
           other_cm.update(favorite: true) if cm.favorite
-          cm.delete
+          cm.destroy
         else
           cm.update(favorite: true) if other_cm.favorite
-          other_cm.delete
+          other_cm.destroy
           cm.update(user: other)
         end
       end
@@ -339,7 +339,7 @@ def merge_into(other, force: false)
 
       evaluation_users.each do |eu|
         if other.evaluation_users.find { |oeu| oeu.evaluation_id == eu.evaluation_id }
-          eu.delete
+          eu.destroy
         else
           eu.update(user: other)
         end
@@ -347,7 +347,7 @@ def merge_into(other, force: false)
 
       activity_read_states.each do |ars|
         if other.activity_read_states.find { |oars| oars.activity_id == ars.activity_id }
-          ars.delete
+          ars.destroy
         else
           ars.update(user: other)
         end
@@ -355,7 +355,7 @@ def merge_into(other, force: false)
 
       repository_admins.each do |ra|
         if other.repository_admins.find { |ora| ora.repository_id == ra.repository_id }
-          ra.delete
+          ra.destroy
         else
           ra.update(user: other)
         end

From 2b245b6c6cc311c0f31a35ebd3b8053aba3e2de8 Mon Sep 17 00:00:00 2001
From: jorg-vr <jorg.vr@live.be>
Date: Mon, 31 Jan 2022 14:14:40 +0100
Subject: [PATCH 21/21] Replace delete and update by destroy! and update!

---
 app/models/user.rb | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index bb60c659f5..cd793d9f07 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -304,65 +304,65 @@ def merge_into(other, force: false)
 
       identities.each do |i|
         if other.identities.find { |oi| oi.provider_id == i.provider_id }
-          i.destroy
+          i.destroy!
         else
-          i.update(user: other)
+          i.update!(user: other)
         end
       end
 
-      rights_request.update(user: other) if !rights_request.nil? && other.permission == 'student' && other.rights_request.nil?
+      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)
+          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
+          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)
+          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) }
+      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
+          eu.destroy!
         else
-          eu.update(user: other)
+          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
+          ars.destroy!
         else
-          ars.update(user: other)
+          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
+          ra.destroy!
         else
-          ra.update(user: other)
+          ra.update!(user: other)
         end
       end
 
       reload
-      destroy
+      destroy!
     end
   end