Merge complex institutions with overlapping users

Gemfile

@@ -134,6 +134,9 @@ gem 'stackprof', '~> 0.2.17'
 # Datadog
 gem 'ddtrace', '~> 0.54.2'
 
+# Make sure filesystem changes only happen at the end of a transaction
+gem 'after_commit_everywhere', '~> 1.1.0'
+
 group :development, :test do
   # Use mocha for stubbing and mocking
   gem 'mocha', '~> 1.13.0'

Gemfile.lock

@@ -64,6 +64,9 @@ GEM
     addressable (2.8.0)
       public_suffix (>= 2.0.2, < 5.0)
     aes_key_wrap (1.1.0)
+    after_commit_everywhere (1.1.0)
+      activerecord (>= 4.2)
+      activesupport
     airbrussh (1.4.0)
       sshkit (>= 1.6.1, != 1.7.0)
     annotate (3.1.1)
@@ -491,6 +494,7 @@ PLATFORMS
 
 DEPENDENCIES
   ace-rails-ap (~> 4.4)
+  after_commit_everywhere (~> 1.1.0)
   annotate (~> 3.1.1)
   autoprefixer-rails (~> 10.4.2)
   bcrypt_pbkdf

app/models/institution.rb

@@ -59,7 +59,7 @@ def unmark_generated
   end
 
   def merge_into(other)
-    errors.add(:merge, 'has overlapping usernames') if other.users.exists?(username: users.pluck(:username))
+    errors.add(:merge, "has overlapping usernames. Run `bin/rake merge_institutions[#{id},#{other.id}]` on the server to solve this using an interactive script.") if other.users.exists?(username: users.pluck(:username))
     errors.add(:merge, 'has link provider') if providers.any?(&:link?)
     return false if errors.any?
 

app/models/submission.rb

@@ -497,8 +497,11 @@ def update_fs
     new_path = fs_path
     return if old_path == new_path
 
-    FileUtils.mkdir_p File.dirname new_path
-    FileUtils.move old_path, new_path
+    # Only apply the changes on the filesystem, after the complete transaction has executed successfully
+    AfterCommitEverywhere.after_commit do
+      FileUtils.mkdir_p File.dirname new_path
+      FileUtils.move old_path, new_path
+    end
   end
 
   def report_if_internal_error

app/models/user.rb

@@ -291,8 +291,9 @@ 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?
+  # Be careful when using force institution. This expects the providers to be updated externally
+  def merge_into(other, force: false, force_institution: false)
+    errors.add(:merge, 'User belongs to different institution') if !force_institution && 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?
 

db/seeds.rb

@@ -68,6 +68,44 @@ def submission_summary(status)
                 institution: ugent
   end
 
+  overlapping_students_ugent = Array.new(3) do |i|
+    first_name = Faker::Name.first_name
+    last_name = Faker::Name.last_name
+    username = "test" + i.to_s
+    User.create first_name: first_name,
+                last_name: last_name,
+                username: username,
+                email: "#{first_name}.#{last_name}.#{username}@UGent.BE".downcase,
+                permission: :student,
+                institution: ugent
+  end
+
+  overlapping_students_artevelde = Array.new(3) do |i|
+    first_name = Faker::Name.first_name
+    last_name = Faker::Name.last_name
+    username = "test" + i.to_s
+    User.create first_name: first_name,
+                last_name: last_name,
+                username: username,
+                email: "#{first_name}.#{last_name}.#{username}@Artevelde.BE".downcase,
+                permission: :student,
+                institution: artevelde
+  end
+
+  unique_students_artevelde = Array.new(50) do
+    first_name = Faker::Name.first_name
+    last_name = Faker::Name.last_name
+    username = Faker::Internet.unique.user_name()
+    User.create first_name: first_name,
+                last_name: last_name,
+                username: username,
+                email: "#{first_name}.#{last_name}.#{username}@Artevelde.BE".downcase,
+                permission: :student,
+                institution: artevelde
+  end
+
+  students = students + overlapping_students_ugent + overlapping_students_artevelde + unique_students_artevelde
+
   puts "Creating identities (#{Time.now - start})"
 
   User.find_each do |user|

lib/tasks/merge_institutions.rake

@@ -0,0 +1,7 @@
+task :merge_institutions, [:arg1, :arg2] => :environment do |task, args|
+  i1_id = args[:arg1].to_i
+  i2_id = args[:arg2].to_i
+
+  require_relative 'merge_institutions.rb'
+  MergeInstitutions.new.merge_institutions_interactive(i1_id, i2_id)
+end

lib/tasks/merge_institutions.rb

@@ -0,0 +1,75 @@
+require_relative 'merge_users.rb'
+
+class MergeInstitutions
+  LABEL_WIDTH = 20
+  INSTITUTION_WIDTH = 50
+
+  def initialize(input = $stdin, output = $stdout)
+    @input = input
+    @output = output
+  end
+
+  def print_institutions(*institutions)
+    @output.puts 'Short name: '.ljust(LABEL_WIDTH) + institutions.map { |i| i.short_name.to_s.ljust(INSTITUTION_WIDTH) }.join
+    @output.puts 'Name: '.ljust(LABEL_WIDTH) + institutions.map { |i| i.name.to_s.ljust(INSTITUTION_WIDTH) }.join
+
+    @output.puts 'Courses: '.ljust(LABEL_WIDTH) + institutions.map { |i| i.courses.count.to_s.ljust(INSTITUTION_WIDTH) }.join
+    @output.puts 'Users: '.ljust(LABEL_WIDTH) + institutions.map { |i| i.users.count.to_s.ljust(INSTITUTION_WIDTH) }.join
+    @output.puts 'Providers: '.ljust(LABEL_WIDTH) + institutions.map { |i| i.providers.count.to_s.ljust(INSTITUTION_WIDTH) }.join
+  end
+
+  def merge_institutions_interactive(i1_id, i2_id)
+    i1 = Institution.find(i1_id)
+    i2 = Institution.find(i2_id)
+
+    print_institutions i1, i2
+
+    c = ''
+    until %W[\r \n y n].include? c
+      i1, i2 = i2, i1 if c == 's'
+      @output.puts "Invalid input #{c}" unless ['s', ''].include?(c)
+      @output.puts "Are you sure you want to merge #{i1.short_name} into #{i2.short_name}? (y)es|(N)o|(s)wap"
+      c = @input.getch.downcase
+    end
+
+    if %W[\r \n n].include? c
+      @output.puts 'Merge cancelled'
+      return
+    end
+
+    i1.transaction do
+      if (overlap = i1.users.where(username: i2.users.pluck(:username)).count) > 0
+        @output.puts "There are #{overlap} overlapping users."
+        @output.puts 'These users will be merged before the institution can be merged.'
+
+        c = ''
+        until %W[\r \n y n].include? c
+          @output.puts "Invalid input #{c}" if c.present?
+          @output.puts 'Do you want to continue the merge? (y)es|(N)o'
+          c = @input.getch.downcase
+        end
+        return unless c == 'y'
+
+        i1.users.where(username: i2.users.pluck(:username)).each do |u1|
+          u2 = i2.users.find { |u| u.username == u1.username }
+          next if u2.nil?
+
+          @output.puts ''
+          success = MergeUsers.new(@input, @output).merge_users_interactive(u1.id, u2.id, force_institution: true)
+          raise ActiveRecord::Rollback unless success
+        end
+      end
+
+      success = i1.merge_into(i2)
+      if success
+        @output.puts "Successfully merged #{i1.short_name} into #{i2.short_name}"
+        print_institutions i2
+      else
+        @output.puts 'Merge failed'
+        @output.puts i1.errors.full_messages.join('\n')
+        raise ActiveRecord::Rollback
+      end
+      success
+    end
+  end
+end

lib/tasks/merge_users.rake

@@ -3,61 +3,6 @@ task :merge_users, [:arg1, :arg2] => :environment do |task, args|
   u1_id = args[:arg1].to_i
   u2_id = args[:arg2].to_i
 
+  require_relative 'merge_users.rb'
   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

lib/tasks/merge_users.rb

@@ -0,0 +1,57 @@
+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 'Id: '.ljust(LABEL_WIDTH) + users.map { |u| u.id.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
+    @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, force_institution: false)
+    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.id} into #{u2.id}? (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', force_institution: force_institution)
+    if success
+      @output.puts "Successfully merged #{u1.id} into #{u2.id}"
+      print_users u2
+    else
+      @output.puts 'Merge failed'
+      @output.puts u1.errors.full_messages.join('\n')
+    end
+    success
+  end
+end