Enforce uniqueness constraint on external_id of study_groups

Similar to e9ab9f8, an external_id of study_groups should be unique. In addition to the previously-configured PostgreSQL constraint, we also add a Rails model validation.

Furthermore, we adjust the PostgreSQL index by specifying `nulls_not_distinct: true`. This is beneficial, since the external_id of study_groups CAN be NULL (in exactly one case, namely for the default study group per consumer). In all other cases, the external_id MUST be set (which this index enforces).

Finally, we use this opportunity to update the migrate_study_group.sql script in case it is needed.

Author: MrSerth

app/controllers/concerns/lti.rb

@@ -196,17 +196,19 @@ def set_user
   def set_study_group_membership
     group = if context_id?
               # Ensure to find the group independent of the name and set it only once.
-              StudyGroup.find_or_create_by(external_id: @provider.context_id, consumer: @consumer) do |new_group|
+              StudyGroup.find_or_create_by!(external_id: @provider.context_id, consumer: @consumer) do |new_group|
                 new_group.name = @provider.context_title
               end
             else
-              StudyGroup.find_or_create_by(external_id: @provider.resource_link_id, consumer: @consumer)
+              StudyGroup.find_or_create_by!(external_id: @provider.resource_link_id, consumer: @consumer)
             end
 
-    study_group_membership = StudyGroupMembership.find_or_create_by(study_group: group, user: @user)
+    study_group_membership = StudyGroupMembership.find_or_create_by!(study_group: group, user: @user)
     study_group_membership.update(role: external_user_role(@provider))
     session[:study_group_id] = group.id
     @user.store_current_study_group_id(group.id)
+  rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
+    retry
   end
 
   def set_embedding_options

app/models/study_group.rb

@@ -14,6 +14,8 @@ class StudyGroup < ApplicationRecord
   has_many :pair_programming_exercise_feedbacks
   belongs_to :consumer
 
+  validates :external_id, uniqueness: {scope: :consumer_id}
+
   def users
     external_users + internal_users
   end

db/migrate/20250502130326_change_uniqueness_constraint_for_study_group_external_id.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class ChangeUniquenessConstraintForStudyGroupExternalId < ActiveRecord::Migration[8.0]
+  def change
+    remove_index :study_groups, column: %i[external_id consumer_id], unique: true
+    add_index :study_groups, %i[external_id consumer_id], unique: true, nulls_not_distinct: true
+  end
+end

db/schema.rb

@@ -10,14 +10,45 @@ BEGIN
         RETURN;
     END IF;
 
+    UPDATE authentication_tokens SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
     UPDATE community_solution_contributions SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
+    UPDATE events SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
+    UPDATE events_synchronized_editor SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
+
+    WITH existing_lti_parameters AS (SELECT CONCAT(exercise_id, '_', external_user_id) as existing
+        FROM lti_parameters
+        WHERE study_group_id = target_study_group)
+    DELETE FROM lti_parameters
+    WHERE study_group_id = duplicated_study_group
+      AND CONCAT(exercise_id, '_', external_user_id) IN (SELECT existing FROM existing_lti_parameters);
+
+    UPDATE lti_parameters SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
+
+    WITH existing_pair_programming_exercise_feedbacks
+    AS (SELECT CONCAT(exercise_id, '_', user_type, '_', user_id, '_', programming_group_id) as existing
+        FROM pair_programming_exercise_feedbacks
+        WHERE study_group_id = target_study_group)
+    DELETE FROM pair_programming_exercise_feedbacks
+    WHERE study_group_id = duplicated_study_group
+      AND CONCAT(exercise_id, '_', user_type, '_', user_id, '_', programming_group_id) IN
+          (SELECT existing FROM existing_pair_programming_exercise_feedbacks);
+
+    UPDATE pair_programming_exercise_feedbacks SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
     UPDATE remote_evaluation_mappings SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
-    UPDATE authentication_tokens SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
-    UPDATE subscriptions SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
     UPDATE submissions SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
 
-    -- Preventing duplicated entries in exercises_proxy_exercises
-    -- The same proxy exercise should not have two entries for the same exercise it proxies.
+    WITH existing_subscriptions AS (SELECT CONCAT(request_for_comment_id, '_', user_type, '_', user_id) as existing
+        FROM subscriptions
+        WHERE study_group_id = target_study_group)
+    DELETE FROM subscriptions
+    WHERE study_group_id = duplicated_study_group
+      AND CONCAT(request_for_comment_id, '_', user_type, '_', user_id) IN (SELECT existing FROM existing_subscriptions);
+
+    -- We don't care about the subscription_type nor deleted flag here.
+    UPDATE subscriptions SET study_group_id = target_study_group WHERE study_group_id = duplicated_study_group;
+
+    -- Preventing duplicated entries in study_group_memberships
+    -- The same study group should not have two entries for the same user.
     DELETE
     FROM study_group_memberships
     WHERE study_group_id = duplicated_study_group
@@ -37,7 +68,7 @@ $$;
 /* Execute migration
 do $$
 begin
-	perform migrate_study_group(target_study_group := 237, duplicated_study_group := 695);
+    perform migrate_study_group(target_study_group := 237, duplicated_study_group := 695);
 end
 $$;
 */

db/scripts/migrate_study_group.sql

@@ -3,6 +3,7 @@
 FactoryBot.define do
   factory :study_group, class: 'StudyGroup' do
     consumer
+    external_id { SecureRandom.uuid }
     sequence :name do |n|
       "TestGroup#{n}"
     end