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, 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