feat: add optional external_id field

app/controllers/concerns/course/users_controller_management_concern.rb

@@ -123,7 +123,7 @@ def should_update_personalized_timeline
 
   def course_user_params
     @course_user_params ||= params.require(:course_user).permit(
-      :user_id, :name, :timeline_algorithm, :role, :phantom, :reference_timeline_id
+      :user_id, :name, :timeline_algorithm, :role, :phantom, :reference_timeline_id, :external_id
     )
   end
 

app/controllers/course/user_invitations_controller.rb

@@ -19,7 +19,8 @@ def create
       create_invitation_success(result)
     else
       propagate_errors
-      render json: { errors: current_course.errors.full_messages.to_sentence }, status: :bad_request
+      errors = current_course.errors[:base]
+      render json: { errors: errors }, status: :bad_request
     end
   end
 
@@ -59,7 +60,8 @@ def course_user_invitation_params
     @course_user_invitation_params ||= begin
       params[:course] = { invitations_attributes: {} } unless params.key?(:course)
       params.require(:course).permit(:invitations_file, :registration_key,
-                                     invitations_attributes: [:name, :email, :role, :phantom, :timeline_algorithm])
+                                     invitations_attributes: [:name, :email, :role, :phantom,
+                                                              :timeline_algorithm, :external_id])
     end
   end
 
@@ -120,7 +122,7 @@ def invite_by_file?
   def invite
     invitation_service.invite(invitation_params)
   rescue CSV::MalformedCSVError => e
-    current_course.errors.add(:invitations_file, e.message)
+    current_course.errors.add(:base, e.message)
     false
   end
 
@@ -135,15 +137,7 @@ def invitation_service
   #
   # @return [void]
   def propagate_errors
-    propagate_errors_to_file if invite_by_file?
-  end
-
-  # Propagates errors from the generated records to the file.
-  #
-  # @return [void]
-  def propagate_errors_to_file
-    errors = aggregate_errors
-    current_course.errors.add(:invitations_file, errors.to_sentence) unless errors.empty?
+    aggregate_errors.each { |msg| current_course.errors.add(:base, msg) }
   end
 
   # Aggregates errors from all the known sources of failure.
@@ -157,9 +151,24 @@ def aggregate_errors
   #
   # @return [Array<String>]
   def invalid_course_user_errors
-    invalid_course_users.map do |course_user|
-      user = self.class.helpers.display_course_user(course_user)
-      t('errors.course.user_invitations.duplicate_user', user: user)
+    invalid_course_users.flat_map do |course_user|
+      email = course_user.user&.email || course_user.name
+      errors = []
+      if course_user.errors.of_kind?(:external_id, :taken)
+        errors << t('errors.course.user_invitations.duplicate_external_id',
+                    email: email, external_id: course_user.external_id)
+      end
+      if course_user.errors.of_kind?(:user, :taken) || course_user.errors.of_kind?(:course, :taken)
+        errors << t('errors.course.user_invitations.already_enrolled', email: email)
+      end
+      other_errors = course_user.errors.reject { |e| %w[external_id user course].include?(e.attribute.to_s) }
+      # .any? is intentional: this is a catch-all for unrecognised errors; all messages are forwarded verbatim
+      if other_errors.any?
+        message = other_errors.map { |e| course_user.errors.full_message(e.attribute, e.message) }.
+                  to_sentence
+        errors << t('errors.course.user_invitations.other_error', email: email, message: message)
+      end
+      errors
     end
   end
 
@@ -174,9 +183,29 @@ def invalid_course_users
   #
   # @return [Array<String>]
   def invalid_invitation_email_errors
-    invalid_invitations.map do |invitation|
-      message = invitation.errors.full_messages.to_sentence
-      t('errors.course.user_invitations.invalid_email', email: invitation.email, message: message)
+    invalid_invitations.flat_map do |invitation|
+      email = invitation.email
+      errors = []
+      if invitation.errors.of_kind?(:external_id, :taken)
+        errors << t('errors.course.user_invitations.duplicate_external_id',
+                    email: email, external_id: invitation.external_id)
+      end
+      # .any? is intentional: invalid_email is a broad wrapper; the actual messages are forwarded in `message`
+      if invitation.errors[:email].any?
+        message = invitation.errors[:email].to_sentence
+        errors << t('errors.course.user_invitations.invalid_email', email: email, message: message)
+      end
+      if invitation.errors.of_kind?(:base, :existing_invitation)
+        errors << t('errors.course.user_invitations.duplicate_invitation', email: email)
+      end
+      other_errors = invitation.errors.reject { |e| %w[external_id email base].include?(e.attribute.to_s) }
+      # .any? is intentional: this is a catch-all for unrecognised errors; all messages are forwarded verbatim
+      if other_errors.any?
+        message = other_errors.map { |e| invitation.errors.full_message(e.attribute, e.message) }.
+                  to_sentence
+        errors << t('errors.course.user_invitations.other_error', email: email, message: message)
+      end
+      errors
     end
   end
 
@@ -254,7 +283,7 @@ def destroy_invitation_success
 
   def destroy_invitation_failure
     respond_to do |format|
-      format.json { render json: { errors: @invitation.errors.full_messages.to_sentence }, status: :bad_request }
+      format.json { render json: { errors: @invitation.errors.full_messages }, status: :bad_request }
     end
   end
 

app/models/concerns/course/unique_external_id_concern.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+# This concern validates that external IDs are unique within a course,
+# across both course users and pending invitations.
+#
+# Nil and blank external IDs are allowed.
+module Course::UniqueExternalIdConcern
+  extend ActiveSupport::Concern
+
+  included do
+    before_validation :normalize_external_id
+
+    validate :validate_unique_external_id_within_course, if: -> { new_record? || external_id_changed? }
+  end
+
+  private
+
+  # Normalizes blank external IDs to nil.
+  #
+  # @return [void]
+  def normalize_external_id
+    self.external_id = nil if external_id.blank?
+  end
+
+  # Validates that the external ID is unique within the course,
+  # across both course users and invitations.
+  #
+  # @return [void]
+  def validate_unique_external_id_within_course
+    return if external_id.blank?
+
+    invitation_exists = Course::UserInvitation.
+                        unconfirmed.
+                        where(course_id: course_id, external_id: external_id).
+                        where.not(id: id).
+                        exists?
+
+    course_user_exists = CourseUser.
+                         where(course_id: course_id, external_id: external_id).
+                         where.not(id: id).
+                         exists?
+
+    return unless invitation_exists || course_user_exists
+
+    errors.add(:external_id, :taken)
+  end
+end

app/models/course/user_invitation.rb

@@ -4,6 +4,8 @@ class Course::UserInvitation < ApplicationRecord
   after_initialize :set_defaults, if: :new_record?
   before_validation :set_defaults, if: :new_record?
 
+  include Course::UniqueExternalIdConcern
+
   validates :email, format: { with: Devise.email_regexp }, if: :email_changed?
   validates :name, presence: true
   validates :role, presence: true

app/models/course_user.rb

@@ -3,6 +3,7 @@ class CourseUser < ApplicationRecord
   include CourseUser::StaffConcern
   include CourseUser::LevelProgressConcern
   include CourseUser::TodoConcern
+  include Course::UniqueExternalIdConcern
 
   after_initialize :set_defaults, if: :new_record?
   before_validation :set_defaults, if: :new_record?

app/models/user.rb

@@ -129,6 +129,7 @@ def build_course_user_from_invitation(invitation)
                        phantom: invitation.phantom,
                        timeline_algorithm: invitation.timeline_algorithm ||
                           invitation.course&.default_timeline_algorithm,
+                       external_id: invitation.external_id,
                        creator: self,
                        updater: self)
   end

app/models/user/email.rb

@@ -35,8 +35,14 @@ def accept_all_pending_invitations
           unconfirmed_invitation.confirm!(confirmer: user)
           next
         end
-        user.build_course_user_from_invitation(unconfirmed_invitation)
-        unconfirmed_invitation.confirm!(confirmer: user) if user.save && user.persisted?
+        # Confirm the invitation before saving the CourseUser so that the
+        # UniqueExternalIdConcern validation doesn't reject the new CourseUser
+        # for sharing an external_id with what is now a confirmed invitation.
+        CourseUser.transaction(requires_new: true) do
+          unconfirmed_invitation.confirm!(confirmer: user)
+          user.build_course_user_from_invitation(unconfirmed_invitation)
+          raise ActiveRecord::Rollback unless user.save && user.persisted?
+        end
       end
     end
   end

app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb

@@ -47,16 +47,23 @@ def parse_invitations(users)
   # ]
   def partition_unique_users(users)
     users.each { |user| user[:email] = user[:email].downcase }
-    unique_users = {}
+    seen_emails = Set.new
+    seen_external_ids = Set.new
+    unique_users = []
     duplicate_users = []
     users.each do |user|
-      if unique_users.key?(user[:email])
-        duplicate_users.push(user)
+      ext_id = user[:external_id].presence
+      if seen_emails.include?(user[:email])
+        duplicate_users.push(user.merge(reason: :duplicate_email_in_file))
+      elsif ext_id && seen_external_ids.include?(ext_id)
+        duplicate_users.push(user.merge(reason: :duplicate_external_id_in_file))
       else
-        unique_users[user[:email]] = user
+        seen_emails.add(user[:email])
+        seen_external_ids.add(ext_id) if ext_id
+        unique_users << user
       end
     end
-    [unique_users.values, duplicate_users]
+    [unique_users, duplicate_users]
   end
 
   # Change all invitees' roles to :student if inviter is a teaching_assistant.
@@ -86,7 +93,8 @@ def parse_from_form(users)
         email: value[:email],
         role: value[:role],
         phantom: phantom,
-        timeline_algorithm: value[:timeline_algorithm] }
+        timeline_algorithm: value[:timeline_algorithm],
+        external_id: value[:external_id] }
     end
   end
 
@@ -144,11 +152,17 @@ def parse_file_row(row)
     return nil if row[1].blank?
 
     row[0] = row[1] if row[0].blank?
-
     role = parse_file_role(row[2])
     phantom = parse_file_phantom(row[3])
-    timeline_algorithm = parse_file_timeline_algorithm(row[4])
-    { name: row[0], email: row[1], role: role, phantom: phantom, timeline_algorithm: timeline_algorithm }
+    if @current_course.show_personalized_timeline_features?
+      timeline_algorithm = parse_file_timeline_algorithm(row[4])
+      external_id = parse_file_external_id(row[5])
+    else
+      external_id = parse_file_external_id(row[4])
+      timeline_algorithm = parse_file_timeline_algorithm(nil)
+    end
+    { name: row[0], email: row[1], role: role, phantom: phantom,
+      timeline_algorithm: timeline_algorithm, external_id: external_id }
   end
 
   # Parses the role column from the CSV file.
@@ -188,6 +202,17 @@ def parse_file_timeline_algorithm(timeline_algorithm)
     symbol || @current_course.default_timeline_algorithm
   end
 
+  # Parses file value for an invitation's external ID.
+  # Returns nil if value is not specified.
+  #
+  # @param [String|nil] external_id External ID as specified in the CSV file.
+  # @return [String|nil] The external ID string, or nil if blank.
+  def parse_file_external_id(external_id)
+    return nil if external_id.blank?
+
+    external_id
+  end
+
   # Removes the UTF-8 byte order mark (BOM) from the string.
   # The BOM exists at the start of in CSVs (optionally) to indicate the
   # encoding of the file.