fix: ensure validation works correctly for tutorial enrolments with r…

…estricted groups

Ensure that destroying the enrolment checks if this breaks groups.
Ensure that changing enrolments correctly checks groups.
Ensure this works with multiple groups in the same tutorial

app/models/project.rb

@@ -104,14 +104,22 @@ def specific_permission_hash(role, perm_hash, _other)
   end
 
   def enrol_in(tutorial)
+    tutorial_enrolment = matching_enrolment(tutorial)
+    return tutorial_enrolment if tutorial_enrolment.present? && tutorial_enrolment.tutorial_id == tutorial.id
+
     # Check if multiple enrolments changing to a single enrolment - due to no stream.
     # No need to delete if only 1, as that would be updated as well.
     if tutorial_enrolments.count > 1 && tutorial.tutorial_stream.nil?
-      # So remove current enrolments
-      tutorial_enrolments.delete_all()
+      begin
+        # So remove current enrolments
+        tutorial_enrolments.destroy_all()
+        # and there is no longer an associated tutorial enrolment
+        tutorial_enrolment = nil
+      rescue ActiveRecord::RecordNotDestroyed => e
+        raise ActiveRecord::RecordNotDestroyed.new("Unable to change tutorial due to group enrolment in current tutorials.", e.record)
+      end
     end
 
-    tutorial_enrolment = matching_enrolment(tutorial)
     if tutorial_enrolment.nil?
       tutorial_enrolment = TutorialEnrolment.new
       tutorial_enrolment.tutorial = tutorial

app/models/tutorial_enrolment.rb

@@ -29,7 +29,10 @@ class TutorialEnrolment < ApplicationRecord
   validate :ensure_cannot_enrol_in_tutorial_with_no_stream_when_enrolled_in_stream
 
   # Ensure in the same tutorial as a group if needed
-  validate :must_be_in_group_tutorials
+  validate :validate_tutorial_change, if: :will_save_change_to_tutorial_id?
+
+  # Ensure we check if we can leave the tutorial... and remove groups if needed
+  before_destroy :remove_from_groups_on_destroy
 
   def unit_must_be_same
     if project.unit.present? and tutorial.unit.present? and not project.unit.eql? tutorial.unit
@@ -80,22 +83,80 @@ def ensure_only_one_tutorial_per_stream
     end
   end
 
-  #
-  # Check to see if the student has a valid tutorial
-  #
-  def must_be_in_group_tutorials
-    project.groups.each do |g|
-      next unless g.limit_members_to_tutorial?
-      next if tutorial_id == g.tutorial_id
-
-      if g.group_set.allow_students_to_manage_groups
-        # leave group
-        g.remove_member(project)
+  def action_on_student_leave_tutorial(for_tutorial_id = nil)
+    # If there are no groups then you can change the tutorial
+    return :none_can_leave if project.groups.count == 0
+
+    result = :none_can_leave
+
+    # Now get the group
+    project.groups.where(tutorial_id: for_tutorial_id || tutorial_id).each do |grp|
+
+      # You can move if the tutorial allows it
+      next unless grp.limit_members_to_tutorial?
+
+      # Remove from the group if we can... otherwise this is an error!
+      if grp.group_set.allow_students_to_manage_groups
+        result = :leave_after_remove_from_group
       else
-        errors.add(:groups, "require #{project.student.name} to be in tutorial #{g.tutorial.abbreviation}")
-        break
+        # We have a group that cannot be left - the student cannot remove themselves and they must be in this tutorial
+        return :leave_denied
       end
     end
+
+    result
   end
 
+  private
+    # You can change the tutorial unless you are in a group that must be in this tutorial
+    def validate_tutorial_change
+      # If there is no change of tutorial id then you can change the tutorial
+      return unless tutorial_id_change_to_be_saved
+
+      # Get ids from change
+      id_from = tutorial_id_change_to_be_saved[0]
+      id_to = tutorial_id_change_to_be_saved[1]
+
+      # If no real change... no problem
+      return if id_from == id_to
+
+      # What action needs to occur when the student leaves this tutorial?
+      action = action_on_student_leave_tutorial(id_from)
+
+      return if action == :none_can_leave
+
+      if action == :leave_denied
+        abbr = Tutorial.find(id_from).abbreviation
+        errors.add(:groups, "require #{project.student.name} to be in tutorial #{abbr}")
+      else # leave after remove from group
+        project.groups.where(tutorial_id: id_from).each do |grp|
+          # Skip groups that can be in other tutorials
+          next unless grp.limit_members_to_tutorial?
+
+          # Remove from the group if we can... otherwise this is an error!
+          if grp.group_set.allow_students_to_manage_groups
+            grp.remove_member(project)
+          else
+            errors.add(:groups, "require #{project.student.name} to be in tutorial #{grp.tutorial.abbreviation}")
+          end
+        end
+      end
+    end
+
+    # Check group removal on delete
+    def remove_from_groups_on_destroy
+      project.groups.where(tutorial_id: tutorial_id).each do |grp|
+        # Skip groups that can be in other tutorials
+        next unless grp.limit_members_to_tutorial?
+
+        # Remove from the group if we can... otherwise this is an error!
+        if grp.group_set.allow_students_to_manage_groups
+          grp.remove_member(project)
+        else
+          errors.add(:groups, "require #{project.student.name} to be in tutorial #{grp.tutorial.abbreviation}")
+          throw :abort
+        end
+      end
+    end
+
 end