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

Ext ID is now accepted in bulk invite CSVs. For existing enrolled users and pending invitations, if external ID is provided, it upserts the current external ID. Conflicts due to duplicate emails or external IDs route to Needs Attention, with clear reasons explaining what to be done.

Author: LWS49

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, duplicate_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,
+                               duplicate_users: duplicate_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

@@ -27,20 +27,20 @@ def normalize_external_id
   # @return [void]
   def validate_unique_external_id_within_course
     return if external_id.blank?
+    return unless external_id_taken_by_invitation? || external_id_taken_by_course_user?
 
-    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?
+    errors.add(:external_id, :taken)
+  end
 
-    return unless invitation_exists || course_user_exists
+  def external_id_taken_by_invitation?
+    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
 
-    errors.add(:external_id, :taken)
+  def external_id_taken_by_course_user?
+    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/services/concerns/course/user_invitation_service/process_invitation_concern.rb

@@ -67,14 +67,30 @@ 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)
+      @duplicate_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))
@@ -129,14 +145,30 @@ 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
+
+    if csv_ext_id.nil? || csv_ext_id == current_ext_id
+      existing_invitations << invitation
+    elsif @taken_external_ids.include?(csv_ext_id)
+      @duplicate_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))

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, duplicate_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
+    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,
+      duplicate_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,
+     duplicate_users, updated_invitations, updated_course_users]
   end
 
   # Resends invitation emails to CourseUsers to the given course.
@@ -77,6 +86,8 @@ def resend_invitation(invitations)
   def invite_users(users)
     unique_users, parse_duplicates = parse_invitations(users)
     @duplicate_users = parse_duplicates
-    process_invitations(unique_users) + [@duplicate_users]
+    @updated_invitations = []
+    @updated_course_users = []
+    process_invitations(unique_users) + [@duplicate_users, @updated_invitations, @updated_course_users]
   end
 end

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

@@ -38,7 +38,7 @@ json.existingCourseUsers existing_course_users.each do |course_user|
   json.phantom course_user.phantom?
 end
 
-json.duplicateUsers duplicate_users.each do |duplicate_user, index|
+json.duplicateUsers duplicate_users.each.with_index do |duplicate_user, index|
   json.id index
   json.name duplicate_user[:name]
   json.email duplicate_user[:email]
@@ -47,3 +47,25 @@ json.duplicateUsers duplicate_users.each do |duplicate_user, index|
   json.phantom duplicate_user[:phantom]
   json.reason duplicate_user[:reason]
 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
+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?
+end