feat(invitations): complete external ID bulk invite flow and redesign result dialog

app/controllers/course/user_invitations_controller.rb

@@ -218,12 +218,16 @@ def invalid_invitations
 
   # Returns the invitation response based on file or entry invitation.
   def parse_invitation_result(new_invitations, existing_invitations, new_course_users,
-                              existing_course_users, duplicate_users)
-    render_to_string(partial: 'invitation_result_data', locals: { new_invitations: new_invitations,
-                                                                  existing_invitations: existing_invitations,
-                                                                  new_course_users: new_course_users,
-                                                                  existing_course_users: existing_course_users,
-                                                                  duplicate_users: duplicate_users })
+                              existing_course_users, failed_users,
+                              updated_invitations, updated_course_users)
+    render_to_string(partial: 'invitation_result_data',
+                     locals: { new_invitations: new_invitations,
+                               existing_invitations: existing_invitations,
+                               new_course_users: new_course_users,
+                               existing_course_users: existing_course_users,
+                               failed_users: failed_users,
+                               updated_invitations: updated_invitations,
+                               updated_course_users: updated_course_users })
   end
 
   # Enables or disables registration codes in the given course.

app/models/concerns/course/unique_external_id_concern.rb

@@ -33,14 +33,14 @@ def validate_unique_external_id_within_course
   end
 
   def external_id_taken_by_invitation?
-    query = Course::UserInvitation.unconfirmed.where(course_id: course_id, external_id: external_id)
-    query = query.where.not(id: id) if is_a?(Course::UserInvitation)
-    query.exists?
+    scope = Course::UserInvitation.unconfirmed.where(course_id: course_id, external_id: external_id)
+    scope = scope.where.not(id: id) if is_a?(Course::UserInvitation)
+    scope.exists?
   end
 
   def external_id_taken_by_course_user?
-    query = CourseUser.where(course_id: course_id, external_id: external_id)
-    query = query.where.not(id: id) if is_a?(CourseUser)
-    query.exists?
+    scope = CourseUser.where(course_id: course_id, external_id: external_id)
+    scope = scope.where.not(id: id) if is_a?(CourseUser)
+    scope.exists?
   end
 end

app/models/user.rb

@@ -129,7 +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,
+                       external_id: invitation.external_id.presence,
                        creator: self,
                        updater: self)
   end

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

@@ -50,20 +50,20 @@ def partition_unique_users(users)
     seen_emails = Set.new
     seen_external_ids = Set.new
     unique_users = []
-    duplicate_users = []
+    failed_users = []
     users.each do |user|
       ext_id = user[:external_id].presence
       if seen_emails.include?(user[:email])
-        duplicate_users.push(user.merge(reason: :duplicate_email_in_file))
+        failed_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))
+        failed_users.push(user.merge(reason: :duplicate_external_id_in_file))
       else
         seen_emails.add(user[:email])
         seen_external_ids.add(ext_id) if ext_id
         unique_users << user
       end
     end
-    [unique_users, duplicate_users]
+    [unique_users, failed_users]
   end
 
   # Change all invitees' roles to :student if inviter is a teaching_assistant.
@@ -151,6 +151,10 @@ def strip_row(row)
   def parse_file_row(row)
     return nil if row[1].blank?
 
+    if !@current_course.show_personalized_timeline_features? && row.length > 5
+      raise I18n.t('errors.course.user_invitations.timeline_template_mismatch')
+    end
+
     row[0] = row[1] if row[0].blank?
     role = parse_file_role(row[2])
     phantom = parse_file_phantom(row[3])

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

@@ -20,7 +20,7 @@ module Course::UserInvitationService::ProcessInvitationConcern
   # @return
   #   [Array<(Array<Course::UserInvitation>, Array<Course::UserInvitation>, Array<CourseUser>, Array<CourseUser>)>]
   #   A tuple containing the users newly invited, already invited, newly registered and already registered respectively.
-  #   Conflicts are accumulated into +@duplicate_users+ as a side effect.
+  #   Conflicts are accumulated into +@failed_users+ as a side effect.
   def process_invitations(users)
     @taken_external_ids = load_existing_external_ids
     augment_user_objects(users)
@@ -67,17 +67,33 @@ def add_existing_users(users)
     new_course_users = []
     users.each do |user|
       if (course_user = all_course_users[user[:user].id])
-        existing_course_users << course_user
+        handle_existing_course_user(user, course_user, existing_course_users)
       else
         enroll_new_user(user, user[:external_id].presence, new_course_users)
       end
     end
     [new_course_users, existing_course_users]
   end
 
+  def handle_existing_course_user(user, course_user, existing_course_users)
+    csv_ext_id = user[:external_id].presence
+    current_ext_id = course_user.external_id.presence
+
+    if csv_ext_id.nil? || csv_ext_id == current_ext_id
+      existing_course_users << course_user
+    elsif @taken_external_ids.include?(csv_ext_id)
+      @failed_users.push(user.merge(reason: :external_id_taken))
+    else
+      @taken_external_ids.delete(current_ext_id) if current_ext_id
+      @taken_external_ids.add(csv_ext_id)
+      course_user.external_id = csv_ext_id
+      @updated_course_users << { record: course_user, previous_external_id: current_ext_id }
+    end
+  end
+
   def enroll_new_user(user, ext_id, new_course_users)
     if ext_id && @taken_external_ids.include?(ext_id)
-      @duplicate_users.push(user.merge(reason: :external_id_taken))
+      @failed_users.push(user.merge(reason: :external_id_taken))
     else
       @taken_external_ids.add(ext_id) if ext_id
       new_course_users << build_course_user(user)
@@ -88,7 +104,7 @@ def enroll_new_user(user, ext_id, new_course_users)
   def build_course_user(user)
     @current_course.course_users.build(user: user[:user], name: user[:name],
                                        role: user[:role], phantom: user[:phantom],
-                                       timeline_algorithm: @current_course.default_timeline_algorithm,
+                                       timeline_algorithm: user[:timeline_algorithm],
                                        external_id: user[:external_id],
                                        creator: @current_user, updater: @current_user)
   end
@@ -129,17 +145,35 @@ def invite_new_users(users)
     users.each do |user|
       invitation = all_invitations[user[:email]]
       if invitation
-        existing_invitations << invitation
+        handle_existing_invitation(user, invitation, existing_invitations)
       else
         add_to_new_invitations(user, user[:external_id].presence, new_invitations)
       end
     end
     [new_invitations, existing_invitations]
   end
 
+  def handle_existing_invitation(user, invitation, existing_invitations)
+    csv_ext_id = user[:external_id].presence
+    current_ext_id = invitation.external_id.presence
+
+    # Non-retryable invitations are surfaced as existing invitations, not errors —
+    # the request succeeded; the prior delivery failure is informational.
+    if csv_ext_id.nil? || csv_ext_id == current_ext_id || invitation.is_retryable == false
+      existing_invitations << invitation
+    elsif @taken_external_ids.include?(csv_ext_id)
+      @failed_users.push(user.merge(reason: :external_id_taken))
+    else
+      @taken_external_ids.delete(current_ext_id) if current_ext_id
+      @taken_external_ids.add(csv_ext_id)
+      invitation.external_id = csv_ext_id
+      @updated_invitations << { record: invitation, previous_external_id: current_ext_id }
+    end
+  end
+
   def add_to_new_invitations(user, ext_id, new_invitations)
     if ext_id && @taken_external_ids.include?(ext_id)
-      @duplicate_users.push(user.merge(reason: :external_id_taken))
+      @failed_users.push(user.merge(reason: :external_id_taken))
     else
       @taken_external_ids.add(ext_id) if ext_id
       new_invitations << build_invitation(user)

app/services/course/statistics/assessments_score_summary_download_service.rb

@@ -75,7 +75,7 @@ def download_score_summary(csv)
     # content
     @all_students.each do |student|
       csv << [student.name, student.user.email, student.phantom? ? 'phantom' : 'normal',
-              *(@include_external_id ? [student.external_id || ''] : []),
+              *(@include_external_id ? [student.external_id.presence || ''] : []),
               *@assessments.flat_map { |a| @submission_grade_hash[[student.id, a.id]] || '' }]
     end
   end

app/services/course/user_invitation_service.rb

@@ -24,28 +24,37 @@ def initialize(current_course_user, current_user, current_course)
   # because Rails does not handle duplicate nested attribute uniqueness constraints.
   #
   # @param [Array<Hash>|File|TempFile] users Invites the given users.
-  # @return [Array<Integer>|nil] An array containing the the size of new_invitations, existing_invitations,
-  #   new_course_users and existing_course_users, duplicate_users respectively if success. nil when fail.
+  # @return [Array<Integer>|nil] An array containing the size of new_invitations, existing_invitations,
+  #   new_course_users, existing_course_users, failed_users, updated_invitations, updated_course_users
+  #   respectively if success. nil when fail.
   # @raise [CSV::MalformedCSVError] When the file provided is invalid.
   def invite(users)
     new_invitations = nil
     existing_invitations = nil
     new_course_users = nil
     existing_course_users = nil
-    duplicate_users = nil
+    failed_users = nil
+    updated_invitations = nil
+    updated_course_users = nil
 
     success = Course.transaction do
       new_invitations, existing_invitations,
-      new_course_users, existing_course_users, duplicate_users = invite_users(users)
+      new_course_users, existing_course_users,
+      failed_users, updated_invitations, updated_course_users = invite_users(users)
+      raise ActiveRecord::Rollback unless updated_invitations.all? { |u| u[:record].save }
+      raise ActiveRecord::Rollback unless updated_course_users.all? { |u| u[:record].save }
       raise ActiveRecord::Rollback unless new_invitations.all?(&:save)
       raise ActiveRecord::Rollback unless new_course_users.all?(&:save)
 
       true
     end
 
-    send_registered_emails(new_course_users) if success
-    send_invitation_emails(new_invitations) if success
-    success ? [new_invitations, existing_invitations, new_course_users, existing_course_users, duplicate_users] : nil
+    return unless success
+
+    send_registered_emails(new_course_users)
+    send_invitation_emails(new_invitations)
+    [new_invitations, existing_invitations, new_course_users, existing_course_users,
+     failed_users, updated_invitations, updated_course_users]
   end
 
   # Resends invitation emails to CourseUsers to the given course.
@@ -76,7 +85,9 @@ def resend_invitation(invitations)
   # @raise [CSV::MalformedCSVError] When the file provided is invalid.
   def invite_users(users)
     unique_users, parse_duplicates = parse_invitations(users)
-    @duplicate_users = parse_duplicates
-    process_invitations(unique_users) + [@duplicate_users]
+    @failed_users = parse_duplicates
+    @updated_invitations = []
+    @updated_course_users = []
+    process_invitations(unique_users) + [@failed_users, @updated_invitations, @updated_course_users]
   end
 end

app/views/course/user_invitations/_invitation_result_data.json.jbuilder

@@ -8,6 +8,7 @@ json.newInvitations new_invitations.each do |invitation|
   json.role invitation.role
   json.phantom invitation.phantom
   json.sentAt invitation.sent_at
+  json.timelineAlgorithm invitation.timeline_algorithm
 end
 
 json.existingInvitations existing_invitations.each do |invitation|
@@ -18,6 +19,8 @@ json.existingInvitations existing_invitations.each do |invitation|
   json.role invitation.role
   json.phantom invitation.phantom
   json.sentAt invitation.sent_at
+  json.isRetryable invitation.is_retryable
+  json.timelineAlgorithm invitation.timeline_algorithm
 end
 
 json.newCourseUsers new_course_users.each do |course_user|
@@ -27,6 +30,7 @@ json.newCourseUsers new_course_users.each do |course_user|
   json.externalId course_user.external_id
   json.role course_user.role
   json.phantom course_user.phantom?
+  json.timelineAlgorithm course_user.timeline_algorithm
 end
 
 json.existingCourseUsers existing_course_users.each do |course_user|
@@ -36,14 +40,40 @@ json.existingCourseUsers existing_course_users.each do |course_user|
   json.externalId course_user.external_id
   json.role course_user.role
   json.phantom course_user.phantom?
+  json.timelineAlgorithm course_user.timeline_algorithm
 end
 
-json.duplicateUsers duplicate_users.each do |duplicate_user, index|
+json.failedUsers failed_users.each.with_index do |failed_user, index|
   json.id index
-  json.name duplicate_user[:name]
-  json.email duplicate_user[:email]
-  json.externalId duplicate_user[:external_id]
-  json.role duplicate_user[:role]
-  json.phantom duplicate_user[:phantom]
-  json.reason duplicate_user[:reason]
+  json.name failed_user[:name]
+  json.email failed_user[:email]
+  json.externalId failed_user[:external_id]
+  json.role failed_user[:role]
+  json.phantom failed_user[:phantom]
+  json.reason failed_user[:reason]
+  json.timelineAlgorithm failed_user[:timeline_algorithm]
+end
+
+json.updatedInvitations updated_invitations.each do |item|
+  inv = item[:record]
+  json.id            inv.id
+  json.name          inv.name
+  json.email         inv.email
+  json.externalId    inv.external_id
+  json.previousExternalId item[:previous_external_id]
+  json.role          inv.role
+  json.phantom       inv.phantom
+  json.timelineAlgorithm inv.timeline_algorithm
+end
+
+json.updatedCourseUsers updated_course_users.each do |item|
+  cu = item[:record]
+  json.id             cu.id if cu.id
+  json.name           cu.name.strip
+  json.email          cu.user.email
+  json.externalId     cu.external_id
+  json.previousExternalId item[:previous_external_id]
+  json.role           cu.role
+  json.phantom        cu.phantom?
+  json.timelineAlgorithm cu.timeline_algorithm
 end

app/views/course/user_invitations/index.json.jbuilder

@@ -8,4 +8,5 @@ end
 json.manageCourseUsersData do
   json.partial! 'course/users/tabs_data', current_course: current_course
   json.defaultTimelineAlgorithm current_course.default_timeline_algorithm
+  json.showPersonalizedTimelineFeatures current_course.show_personalized_timeline_features
 end

client/app/bundles/course/enrol-requests/store.ts

@@ -32,6 +32,7 @@ const initialState: EnrolRequestsState = {
     requestsCount: 0,
     invitationsCount: 0,
     defaultTimelineAlgorithm: 'fixed',
+    showPersonalizedTimelineFeatures: false,
   },
 };