diff --git a/app/controllers/course/user_invitations_controller.rb b/app/controllers/course/user_invitations_controller.rb index 4e1e99bf2ff..0092e460342 100644 --- a/app/controllers/course/user_invitations_controller.rb +++ b/app/controllers/course/user_invitations_controller.rb @@ -15,7 +15,7 @@ def index def create result = invite - if result + if result.is_a?(Array) create_invitation_success(result) else propagate_errors @@ -24,6 +24,13 @@ def create end end + def update_external_ids + invitation_service.update_external_ids(external_id_updates_params) + head :ok + rescue Course::UserInvitationService::ExternalIdConflict => e + render json: { conflictingExternalId: e.external_id }, status: :conflict + end + def destroy if @invitation.destroy destroy_invitation_success @@ -43,7 +50,8 @@ def resend_invitation end def resend_invitations - if invitation_service.resend_invitation(load_invitations) + @invitations = load_invitations + if invitation_service.resend_invitation(@invitations) resend_invitations_success else resend_invitations_failure @@ -56,6 +64,12 @@ def toggle_registration private + def external_id_updates_params + params.permit(updates: [:type, :id, :external_id])[:updates].to_a.map do |u| + { type: u[:type], id: u[:id].to_i, external_id: u[:external_id] } + end + end + def course_user_invitation_params @course_user_invitation_params ||= begin params[:course] = { invitations_attributes: {} } unless params.key?(:course) @@ -93,7 +107,7 @@ def resend_invitation_params # 1) Single invitation - specified with the user_invitation_id param # 2) All un-confirmed invitation - if user_invitation_id param was not found def load_invitations - @invitations ||= begin + @load_invitations ||= begin ids = resend_invitation_params ids ||= current_course.invitations.retryable.unconfirmed.select(:id) if ids.blank? @@ -118,7 +132,8 @@ def invite_by_file? # Invites the users via the service object. # - # @return [Boolean] True if the invitation was successful. + # @return [Array] On success. + # @return [Boolean] false on failure. def invite invitation_service.invite(invitation_params) rescue CSV::MalformedCSVError => e @@ -218,12 +233,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, + pending_invitation_updates, pending_course_user_updates) + 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, + pending_invitation_updates: pending_invitation_updates, + pending_course_user_updates: pending_course_user_updates }) end # Enables or disables registration codes in the given course. @@ -264,7 +283,7 @@ def resend_invitation_failure def resend_invitations_success respond_to do |format| format.json do - render partial: 'course_user_invitation_list', locals: { invitations: @invitations.reload }, status: :ok + render partial: 'course_user_invitation_list', locals: { invitations: @invitations }, status: :ok end end end diff --git a/app/models/concerns/course/unique_external_id_concern.rb b/app/models/concerns/course/unique_external_id_concern.rb index c06993a7649..7d271a39069 100644 --- a/app/models/concerns/course/unique_external_id_concern.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 85923635741..58a50cbe5ff 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,6 +7,7 @@ class User < ApplicationRecord include UserSearchConcern include TimeZoneConcern include Generic::CollectionConcern + model_stamper acts_as_reader mount_uploader :profile_photo, ImageUploader @@ -93,7 +94,7 @@ def deleted # # @return [Boolean] def built_in? - id == User::SYSTEM_USER_ID || id == User::DELETED_USER_ID + [User::SYSTEM_USER_ID, User::DELETED_USER_ID].include?(id) end # Pick the default email and set it as primary email. This method would immediately set the @@ -129,7 +130,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 diff --git a/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb b/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb index d65bf61ec92..1d76601e533 100644 --- a/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb +++ b/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb @@ -9,6 +9,8 @@ module Course::UserInvitationService::ParseInvitationConcern extend ActiveSupport::Autoload TRUE_VALUES = ['t', 'true', 'y', 'yes'].freeze + EXPECTED_HEADERS_WITH_TIMELINE = %w[name email role phantom timeline externalid].freeze + EXPECTED_HEADERS_WITHOUT_TIMELINE = %w[name email role phantom externalid].freeze private @@ -35,35 +37,48 @@ def parse_invitations(users) parse_from_form(users) end - partition_unique_users(restrict_invitee_role(result)) + restrict_invitee_role(result) end - # Partition users into unique (including first duplicate instance) and duplicate users. + # Partition users into unique and duplicate users. + # + # Email dedup applies to all rows. External-ID dedup applies only to rows + # whose email is NOT in +existing_account_emails+ — users with existing platform + # accounts are handled by the DB-aware process phase, so pre-deduping their + # external IDs here would incorrectly fail rows that the process phase would + # accept (e.g. an enrolled user re-uploaded with their current external ID). # # @param [Array] users - # @return [ - # [Array], - # [Array] - # ] - def partition_unique_users(users) + # @param [Set] existing_account_emails Downcased emails of users who + # already have a platform account. These rows skip external-ID dedup. + # @return [[Array, Array]] + def partition_unique_users(users, existing_account_emails = Set.new) users.each { |user| user[:email] = user[:email].downcase } seen_emails = Set.new seen_external_ids = Set.new - unique_users = [] - duplicate_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)) - elsif ext_id && seen_external_ids.include?(ext_id) - duplicate_users.push(user.merge(reason: :duplicate_external_id_in_file)) + users.each_with_object([[], []]) do |user, (unique, failed)| + reason = duplicate_reason(user, seen_emails, seen_external_ids, existing_account_emails) + if reason + failed << user.merge(reason: reason) else - seen_emails.add(user[:email]) - seen_external_ids.add(ext_id) if ext_id - unique_users << user + track_seen!(user, seen_emails, seen_external_ids, existing_account_emails) + unique << user end end - [unique_users, duplicate_users] + end + + def duplicate_reason(user, seen_emails, seen_external_ids, existing_account_emails) + return :duplicate_email_in_file if seen_emails.include?(user[:email]) + + ext_id = user[:external_id].presence + return :duplicate_external_id_in_file \ + if ext_id && !existing_account_emails.include?(user[:email]) && seen_external_ids.include?(ext_id) + end + + def track_seen!(user, seen_emails, seen_external_ids, existing_account_emails) + seen_emails.add(user[:email]) + ext_id = user[:external_id].presence + seen_external_ids.add(ext_id) if ext_id && !existing_account_emails.include?(user[:email]) end # Change all invitees' roles to :student if inviter is a teaching_assistant. @@ -116,8 +131,13 @@ def parse_from_file(file) row_num = row_number row[0] = remove_utf8_byte_order_mark(row[0]) if row_number == 1 row = strip_row(row) - # Ignore first row if it's a header row. - next if row_number == 1 && header_row?(row) + if row_number == 1 && looks_like_header?(row) + unless valid_header_row?(row) + raise I18n.t('errors.course.user_invitations.invalid_headers', + expected: expected_headers.join(',')) + end + next + end invite = parse_file_row(row) invites << invite if invite @@ -127,12 +147,23 @@ def parse_from_file(file) raise CSV::MalformedCSVError.new(e, row_num), e.message end - # Returns a boolean to determine whether the row is a header row. - # - # @param[Array] row Array read from CSV file. - # @return [Boolean] Whether the row is a header row - def header_row?(row) - row[0].casecmp('Name') == 0 && row[1].casecmp('Email') == 0 + def looks_like_header?(row) + row[0]&.casecmp('Name')&.zero? && row[1]&.casecmp('Email')&.zero? + end + + def expected_headers + if @current_course.show_personalized_timeline_features? + EXPECTED_HEADERS_WITH_TIMELINE + else + EXPECTED_HEADERS_WITHOUT_TIMELINE + end + end + + def valid_header_row?(row) + return false unless looks_like_header?(row) + + provided = row.map { |h| h&.downcase }.compact + expected_headers.first(provided.length) == provided end # Strips a row of whitespaces. diff --git a/app/services/concerns/course/user_invitation_service/process_invitation_concern.rb b/app/services/concerns/course/user_invitation_service/process_invitation_concern.rb index a93b6831135..3e9c34fe9df 100644 --- a/app/services/concerns/course/user_invitation_service/process_invitation_concern.rb +++ b/app/services/concerns/course/user_invitation_service/process_invitation_concern.rb @@ -20,9 +20,10 @@ module Course::UserInvitationService::ProcessInvitationConcern # @return # [Array<(Array, Array, Array, Array)>] # 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 + Rails.logger.info("Processing invitations with taken external IDs: #{@taken_external_ids.to_a}") augment_user_objects(users) existing_users, new_users = users.partition { |user| user[:user].present? } [*invite_new_users(new_users), @@ -36,7 +37,7 @@ def process_invitations(users) # @return [void] def augment_user_objects(users) email_user_mapping = find_existing_users(users.map { |user| user[:email] }) - users.each { |user| user[:user] = email_user_mapping[user[:email]] } + users.each { |user| user[:user] ||= email_user_mapping[user[:email]] } end # Given a list of email addresses, returns a Hash containing the mappings from email addresses @@ -67,7 +68,7 @@ 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 @@ -75,9 +76,28 @@ def add_existing_users(users) [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.add(csv_ext_id) + @pending_course_user_updates << { + record: course_user, + previous_external_id: current_ext_id, + new_external_id: csv_ext_id + } + existing_course_users << course_user + 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 +108,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,7 +149,7 @@ 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 @@ -137,9 +157,30 @@ def invite_new_users(users) [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.add(csv_ext_id) + @pending_invitation_updates << { + record: invitation, + previous_external_id: current_ext_id, + new_external_id: csv_ext_id + } + existing_invitations << invitation + 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) diff --git a/app/services/course/statistics/assessments_score_summary_download_service.rb b/app/services/course/statistics/assessments_score_summary_download_service.rb index f3375bbf6d0..88e98756c6a 100644 --- a/app/services/course/statistics/assessments_score_summary_download_service.rb +++ b/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 diff --git a/app/services/course/user_invitation_service.rb b/app/services/course/user_invitation_service.rb index eb77da78879..f49051c14d6 100644 --- a/app/services/course/user_invitation_service.rb +++ b/app/services/course/user_invitation_service.rb @@ -6,6 +6,15 @@ class Course::UserInvitationService include ProcessInvitationConcern include EmailInvitationConcern + class ExternalIdConflict < StandardError + attr_reader :external_id + + def initialize(external_id) + @external_id = external_id + super("External ID '#{external_id}' is already used by another member") + end + end + # Constructor for the user invitation service object. # # @param [CourseUser|nil] current_course_user The course user performing this action. @@ -24,29 +33,48 @@ def initialize(current_course_user, current_user, current_course) # because Rails does not handle duplicate nested attribute uniqueness constraints. # # @param [Array|File|TempFile] users Invites the given users. - # @return [Array|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|nil] An array containing the size of new_invitations, existing_invitations, + # new_course_users, existing_course_users, failed_users, pending_invitation_updates, + # pending_course_user_updates 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 + result = nil success = Course.transaction do - new_invitations, existing_invitations, - new_course_users, existing_course_users, duplicate_users = invite_users(users) - raise ActiveRecord::Rollback unless new_invitations.all?(&:save) - raise ActiveRecord::Rollback unless new_course_users.all?(&:save) - + result = invite_users(users) + save_invitation_records!(result) 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 + + new_invitations, _, new_course_users, = result + send_registered_emails(new_course_users) + send_invitation_emails(new_invitations) + result + end + + # Atomically updates external IDs for course users and/or invitations. + # + # @param [Array] updates Each hash must contain :type ('course_user' or 'invitation'), + # :id, and :external_id (blank value clears it). + # @raise [ExternalIdConflict] if any new external_id is already taken by another member. + # rubocop:disable Metrics/AbcSize + def update_external_ids(updates) + course_user_updates = updates.select { |u| u[:type].to_s == 'course_user' } + invitation_updates = updates.select { |u| u[:type].to_s == 'invitation' } + + course_users = @current_course.course_users.where(id: course_user_updates.map { |u| u[:id] }).index_by(&:id) + invitations = @current_course.invitations.where(id: invitation_updates.map { |u| u[:id] }).index_by(&:id) + + taken = external_ids_taken_outside(course_users.keys, invitations.keys) + + Course.transaction do + apply_external_id_update(course_user_updates, course_users, taken) + apply_external_id_update(invitation_updates, invitations, taken) + end end + # rubocop:enable Metrics/AbcSize # Resends invitation emails to CourseUsers to the given course. # This method disregards CourseUsers that do not have an 'invited' status. @@ -55,11 +83,41 @@ def invite(users) # @return [Boolean] True if there were no errors in sending invitations. # If all provided CourseUsers have already registered, method also returns true. def resend_invitation(invitations) - invitations.blank? ? true : send_invitation_emails(invitations) + invitations.blank? || send_invitation_emails(invitations) end private + def external_ids_taken_outside(excluded_course_user_ids, excluded_invitation_ids) + course_ext_ids = @current_course.course_users. + where.not(id: excluded_course_user_ids). + where.not(external_id: nil).pluck(:external_id) + invitation_ext_ids = Course::UserInvitation.unconfirmed. + where(course: @current_course). + where.not(id: excluded_invitation_ids). + where.not(external_id: nil).pluck(:external_id) + Set.new(course_ext_ids + invitation_ext_ids) + end + + def apply_external_id_update(updates, records_by_id, taken) + updates.each do |update| + record = records_by_id[update[:id]] + next unless record + + value = update[:external_id].presence + raise ExternalIdConflict, value if value && taken.include?(value) + + taken.add(value) if value + record.update!(external_id: value) + end + end + + def save_invitation_records!(result) + new_invitations, _, new_course_users, = result + all_records = new_invitations + new_course_users + raise ActiveRecord::Rollback unless all_records.all?(&:save) + end + # Invites the given users into the course. # # @param [Array|File|TempFile] users Invites the given users. @@ -75,8 +133,15 @@ def resend_invitation(invitations) # newly registered and already registered, and duplicate users respectively. # @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] + user_hashes = parse_invitations(users) + augment_user_objects(user_hashes) + existing_account_emails = user_hashes. + select { |u| u[:user].present? }. + to_set { |u| u[:email].downcase } + unique_users, parse_duplicates = partition_unique_users(user_hashes, existing_account_emails) + @failed_users = parse_duplicates + @pending_invitation_updates = [] + @pending_course_user_updates = [] + process_invitations(unique_users) + [@failed_users, @pending_invitation_updates, @pending_course_user_updates] end end diff --git a/app/views/course/user_invitations/_invitation_result_data.json.jbuilder b/app/views/course/user_invitations/_invitation_result_data.json.jbuilder index 421b05dc99d..a406fde4aba 100644 --- a/app/views/course/user_invitations/_invitation_result_data.json.jbuilder +++ b/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.pendingInvitationUpdates pending_invitation_updates do |item| + inv = item[:record] + json.id inv.id + json.name inv.name + json.email inv.email + json.externalId item[:new_external_id] + json.previousExternalId item[:previous_external_id] + json.role inv.role + json.phantom inv.phantom + json.timelineAlgorithm inv.timeline_algorithm +end + +json.pendingCourseUserUpdates pending_course_user_updates do |item| + cu = item[:record] + json.id cu.id + json.name cu.name.strip + json.email cu.user.email + json.externalId item[:new_external_id] + json.previousExternalId item[:previous_external_id] + json.role cu.role + json.phantom cu.phantom? + json.timelineAlgorithm cu.timeline_algorithm end diff --git a/app/views/course/user_invitations/index.json.jbuilder b/app/views/course/user_invitations/index.json.jbuilder index f62c23a563c..17ea9e6b4cb 100644 --- a/app/views/course/user_invitations/index.json.jbuilder +++ b/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 diff --git a/client/app/api/course/UserInvitations.ts b/client/app/api/course/UserInvitations.ts index 9d042ee905b..067ec3a75d1 100644 --- a/client/app/api/course/UserInvitations.ts +++ b/client/app/api/course/UserInvitations.ts @@ -4,6 +4,7 @@ import { ManageCourseUsersSharedData, } from 'types/course/courseUsers'; import { + ExternalIdUpdate, InvitationFileEntity, InvitationListData, } from 'types/course/userInvitations'; @@ -36,11 +37,10 @@ export default class UserInvitationsAPI extends BaseCourseAPI { * @return {Promise} * error response: { errors: [] } - An array of errors will be returned upon validation error. */ - invite(data: InvitationFileEntity | FormData): Promise< - AxiosResponse<{ - newInvitations: number; - invitationResult: string; // string which is JSON.parsed to type InvitationResult - }> + invite( + data: InvitationFileEntity | FormData, + ): Promise< + AxiosResponse<{ newInvitations: number; invitationResult: string }> > { const config = { headers: { @@ -52,9 +52,7 @@ export default class UserInvitationsAPI extends BaseCourseAPI { let formData = new FormData(); if ('file' in data) { - const temp = { - invitations_file: data.file, - }; + const temp = { invitations_file: data.file }; SubmissionsAPI.appendFormData(formData, temp, 'course'); } else { formData = data as FormData; @@ -67,6 +65,16 @@ export default class UserInvitationsAPI extends BaseCourseAPI { ); } + updateExternalIds(updates: ExternalIdUpdate[]): Promise> { + return this.client.post(`${this.#urlPrefix}/users/update_external_ids`, { + updates: updates.map((u) => ({ + type: u.type, + id: u.id, + external_id: u.externalId ?? '', + })), + }); + } + /** * Fetches course registration key. */ diff --git a/client/app/bundles/course/enrol-requests/store.ts b/client/app/bundles/course/enrol-requests/store.ts index c27fd35db95..d1fa07dbdb1 100644 --- a/client/app/bundles/course/enrol-requests/store.ts +++ b/client/app/bundles/course/enrol-requests/store.ts @@ -32,6 +32,7 @@ const initialState: EnrolRequestsState = { requestsCount: 0, invitationsCount: 0, defaultTimelineAlgorithm: 'fixed', + showPersonalizedTimelineFeatures: false, }, }; diff --git a/client/app/bundles/course/user-invitations/components/forms/IndividualInviteForm.tsx b/client/app/bundles/course/user-invitations/components/forms/IndividualInviteForm.tsx index 56f8a9b1330..0c52009e2ea 100644 --- a/client/app/bundles/course/user-invitations/components/forms/IndividualInviteForm.tsx +++ b/client/app/bundles/course/user-invitations/components/forms/IndividualInviteForm.tsx @@ -114,33 +114,34 @@ const IndividualInviteForm: FC = (props) => { } }, [invitationsFields.length === 0]); + const handleError = (error: unknown): void => { + const rawErrors = (error as { response?: { data?: { errors?: unknown } } }) + ?.response?.data?.errors; + let errorList: string[]; + if (Array.isArray(rawErrors)) errorList = rawErrors; + else if (typeof rawErrors === 'string') errorList = [rawErrors]; + else errorList = []; + const first = errorList[0]; + const overflow = + errorList.length > 1 ? ` (and ${errorList.length - 1} more)` : ''; + if (first) { + toast.error(t(translations.failure, { error: first + overflow }), { + autoClose: false, + }); + } else { + toast.error(t(translations.failureGeneric), { autoClose: false }); + } + }; + const onSubmit = (data: InvitationsPostData): Promise => { setIsLoading(true); return dispatch(inviteUsersFromForm(data)) - .then((response) => { + .then((result) => { reset(initialValues); - openResultDialog(response); - }) - .catch((error) => { - const rawErrors = error.response?.data?.errors; - let errorList: string[]; - if (Array.isArray(rawErrors)) errorList = rawErrors; - else if (typeof rawErrors === 'string') errorList = [rawErrors]; - else errorList = []; - const first = errorList[0]; - const overflow = - errorList.length > 1 ? ` (and ${errorList.length - 1} more)` : ''; - if (first) { - toast.error(t(translations.failure, { error: first + overflow }), { - autoClose: false, - }); - } else { - toast.error(t(translations.failureGeneric), { autoClose: false }); - } + openResultDialog(result); }) - .finally(() => { - setIsLoading(false); - }); + .catch(handleError) + .finally(() => setIsLoading(false)); }; return ( diff --git a/client/app/bundles/course/user-invitations/components/forms/InviteUsersFileUploadForm.tsx b/client/app/bundles/course/user-invitations/components/forms/InviteUsersFileUploadForm.tsx index 3f1dd85c539..1f36ff16cea 100644 --- a/client/app/bundles/course/user-invitations/components/forms/InviteUsersFileUploadForm.tsx +++ b/client/app/bundles/course/user-invitations/components/forms/InviteUsersFileUploadForm.tsx @@ -12,7 +12,7 @@ import useTranslation from 'lib/hooks/useTranslation'; interface Props { open: boolean; onSubmit: ( - data: InvitationFileEntity, + data: { file: InvitationFileEntity }, setError: UseFormSetError, ) => Promise; onClose: () => void; diff --git a/client/app/bundles/course/user-invitations/components/misc/InvitationResultDialog.tsx b/client/app/bundles/course/user-invitations/components/misc/InvitationResultDialog.tsx index 8c6260c4434..aa149cda180 100644 --- a/client/app/bundles/course/user-invitations/components/misc/InvitationResultDialog.tsx +++ b/client/app/bundles/course/user-invitations/components/misc/InvitationResultDialog.tsx @@ -1,7 +1,10 @@ -import { FC } from 'react'; -import { defineMessages, injectIntl, WrappedComponentProps } from 'react-intl'; +import { FC, useEffect, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import ErrorOutline from '@mui/icons-material/ErrorOutline'; import HelpIcon from '@mui/icons-material/Help'; +import WarningAmber from '@mui/icons-material/WarningAmber'; import { + Alert, Button, Dialog, DialogActions, @@ -10,30 +13,34 @@ import { Tooltip, Typography, } from '@mui/material'; -import { InvitationResult } from 'types/course/userInvitations'; +import { CourseUserData } from 'types/course/courseUsers'; +import { + ExternalIdUpdate, + FailedInvitationRowData, + InvitationListData, + InvitationResult, + InvitationSuccessRow, +} from 'types/course/userInvitations'; + +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; +import useTranslation from 'lib/hooks/useTranslation'; -import InvitationResultInvitationsTable from '../tables/InvitationResultInvitationsTable'; -import InvitationResultUsersTable from '../tables/InvitationResultUsersTable'; +import { updateExternalIds } from '../../operations'; +import { getManageCourseUsersSharedData } from '../../selectors'; +import ExternalIdUpdateTable from '../tables/ExternalIdUpdateTable'; +import InvitationResultExistingTable, { + ExistingRow, +} from '../tables/InvitationResultExistingTable'; +import InvitationResultFailedTable from '../tables/InvitationResultFailedTable'; +import InvitationResultPrimaryTable from '../tables/InvitationResultPrimaryTable'; -interface Props extends WrappedComponentProps { +interface Props { open: boolean; handleClose: () => void; invitationResult: InvitationResult; } -const styles = { - icon: { - fontSize: '16px', - marginRight: '4px', - }, - dialogStyle: { - top: 40, - '& .MuiDialog-paper': { - overflowY: 'hidden', - }, - }, -}; - const translations = defineMessages({ header: { id: 'course.userInvitations.InvitationResultDialog.header', @@ -43,69 +50,209 @@ const translations = defineMessages({ id: 'course.userInvitations.InvitationResultDialog.close', defaultMessage: 'Close', }, - body: { - id: 'course.userInvitations.InvitationResultDialog.body', + summary: { + id: 'course.userInvitations.InvitationResultDialog.summary', defaultMessage: - '{newInvitationsCount, plural, =0 {No new users were} one {# new user has been} other {# new users have been}} invited to Coursemology. ' + - '{newCourseUsersCount, plural, =0 {No user with Coursemology account has been} one {# new user with existing Coursemology account has been} other {# new users with existing Coursemology accounts have been}} added to this course.', + '{newInvitations} new {newInvitations, plural, one {invitation} other {invitations}} sent, {newEnrollments} directly enrolled, {alreadyInCourse} already in course.', + }, + summaryFailed: { + id: 'course.userInvitations.InvitationResultDialog.summaryFailed', + defaultMessage: '{count} failed.', + }, + actionableTitle: { + id: 'course.userInvitations.InvitationResultDialog.actionableTitle', + defaultMessage: 'Failed ({count})', + }, + failedRowsSubtitle: { + id: 'course.userInvitations.InvitationResultDialog.failedRowsSubtitle', + defaultMessage: + '{count} {count, plural, one {row} other {rows}} highlighted in red could not be sent', + }, + newInvitations: { + id: 'course.userInvitations.InvitationResultDialog.newInvitations', + defaultMessage: 'New Invitations ({count})', }, - duplicateInfo: { - id: 'course.userInvitations.InvitationResultDialog.duplicateInfo', + newCourseUsers: { + id: 'course.userInvitations.InvitationResultDialog.newCourseUsers', + defaultMessage: 'New Course Users ({count})', + }, + externalIdUpdates: { + id: 'course.userInvitations.InvitationResultDialog.externalIdUpdates', + defaultMessage: 'External IDs to update ({count})', + }, + externalIdUpdatesInfo: { + id: 'course.userInvitations.InvitationResultDialog.externalIdUpdatesInfo', defaultMessage: - 'Duplicate users were found in the invitation. Only the first instance of each user will be invited.', + 'These users are already invited or enrolled. Their External ID changes only if you apply the file values.', }, - duplicateUsers: { - id: 'course.userInvitations.InvitationResultDialog.duplicateUsers', - defaultMessage: 'Duplicate Users ({count})', + invitationsSubheading: { + id: 'course.userInvitations.InvitationResultDialog.invitationsSubheading', + defaultMessage: 'Invitations ({count})', }, - existingCourseUsersInfo: { - id: 'course.userInvitations.InvitationResultDialog.existingCourseUsersInfo', + enrolledMembersSubheading: { + id: 'course.userInvitations.InvitationResultDialog.enrolledMembersSubheading', + defaultMessage: 'Enrolled members ({count})', + }, + existingInvitations: { + id: 'course.userInvitations.InvitationResultDialog.existingInvitations', + defaultMessage: 'Existing Invitations ({count})', + }, + existingInvitationsInfo: { + id: 'course.userInvitations.InvitationResultDialog.existingInvitationsInfo', defaultMessage: - 'Existing course users with this email were found in the invitation. They were not invited.', + 'These users already have a pending invitation. They were not re-invited.', }, existingCourseUsers: { id: 'course.userInvitations.InvitationResultDialog.existingCourseUsers', defaultMessage: 'Existing Course Users ({count})', }, - existingInvitationsInfo: { - id: 'course.userInvitations.InvitationResultDialog.existingInvitationsInfo', + existingCourseUsersInfo: { + id: 'course.userInvitations.InvitationResultDialog.existingCourseUsersInfo', defaultMessage: - 'Existing invitations for these users with this email already exist. They were not invited.', + 'These users are already enrolled in this course. They were not re-enrolled.', }, - existingInvitations: { - id: 'course.userInvitations.InvitationResultDialog.existingInvitations', - defaultMessage: 'Existing Invitations ({count})', + pendingExternalIdInfo: { + id: 'course.userInvitations.InvitationResultDialog.pendingExternalIdInfo', + defaultMessage: + '{count, plural, one {# row has} other {# rows have}} a different External ID in your file. Current values were kept.', }, - newCourseUsers: { - id: 'course.userInvitations.InvitationResultDialog.newCourseUsers', - defaultMessage: 'New Course Users ({count})', + externalIdUpdatedDone: { + id: 'course.userInvitations.InvitationResultDialog.externalIdUpdatedDone', + defaultMessage: 'External IDs were updated from your file.', }, - newInvitations: { - id: 'course.userInvitations.InvitationResultDialog.newInvitations', - defaultMessage: 'New Invitations ({count})', + applyFileValues: { + id: 'course.userInvitations.InvitationResultDialog.applyFileValues', + defaultMessage: 'Apply changes ({count})', }, + changesApplied: { + id: 'course.userInvitations.InvitationResultDialog.changesApplied', + defaultMessage: 'Changes applied', + }, + applyFailed: { + id: 'course.userInvitations.InvitationResultDialog.applyFailed', + defaultMessage: + "Couldn''t apply — External ID ''{id}'' is now used by another member. No changes were made.", + }, +}); + +const toSuccessRow = ( + item: InvitationListData | CourseUserData, + prefix: string, +): InvitationSuccessRow => ({ + id: `${prefix}-${item.id}`, + name: item.name, + email: item.email, + externalId: item.externalId ?? null, + role: item.role ?? '', + phantom: item.phantom ?? false, + timelineAlgorithm: item.timelineAlgorithm, }); -const InvitationResultDialog: FC = (props) => { - const { open, handleClose, invitationResult, intl } = props; +const InvitationResultDialog: FC = ({ + open, + handleClose, + invitationResult, +}) => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const { showPersonalizedTimelineFeatures } = useAppSelector( + getManageCourseUsersSharedData, + ); + const [applied, setApplied] = useState(false); + const [busy, setBusy] = useState(false); + + useEffect(() => { + if (!open) { + setApplied(false); + setBusy(false); + } + }, [open]); + + if (!open) return null; + const { - duplicateUsers, - existingCourseUsers, - existingInvitations, - newCourseUsers, - newInvitations, + newInvitations = [], + newCourseUsers = [], + existingInvitations = [], + existingCourseUsers = [], + failedUsers = [], + pendingInvitationUpdates = [], + pendingCourseUserUpdates = [], } = invitationResult; - if (!open) { - return null; - } + const newInvitationRows = newInvitations.map((i) => toSuccessRow(i, 'inv')); + const newCourseUserRows = newCourseUsers.map((u) => toSuccessRow(u, 'cu')); - const handleDialogClose = (_event: object, reason: string): void => { - if (reason !== 'backdropClick') { - handleClose(); + const failedInvitations = existingInvitations.filter( + (i) => i.isRetryable === false, + ); + const normalExistingInvitations = existingInvitations.filter( + (i) => i.isRetryable !== false, + ); + + const failedToSendRows: FailedInvitationRowData[] = failedInvitations.map( + (inv) => ({ + id: inv.id, + name: inv.name, + email: inv.email, + externalId: inv.externalId ?? undefined, + role: inv.role, + phantom: inv.phantom, + reason: 'failed_to_send' as const, + timelineAlgorithm: inv.timelineAlgorithm, + }), + ); + + const allFailedUsers: FailedInvitationRowData[] = [ + ...failedToSendRows, + ...failedUsers, + ]; + + const pendingCount = + pendingInvitationUpdates.length + pendingCourseUserUpdates.length; + + const buildApplyUpdates = (): ExternalIdUpdate[] => [ + ...pendingInvitationUpdates.map((p) => ({ + type: 'invitation' as const, + id: p.id, + externalId: p.externalId, + })), + ...pendingCourseUserUpdates.map((p) => ({ + type: 'course_user' as const, + id: p.id, + externalId: p.externalId, + })), + ]; + + const applyChanges = async (): Promise => { + setBusy(true); + try { + await dispatch(updateExternalIds(buildApplyUpdates())); + setApplied(true); + } catch (error) { + const id = ( + error as { response?: { data?: { conflictingExternalId?: string } } } + )?.response?.data?.conflictingExternalId; + toast.error(t(translations.applyFailed, { id: id ?? '' })); + } finally { + setBusy(false); } }; + const existingInvitationRows: ExistingRow[] = normalExistingInvitations; + const existingCourseUserRows: ExistingRow[] = existingCourseUsers; + + const needsAttentionCount = failedUsers.length + failedInvitations.length; + const alreadyInCourseCount = + normalExistingInvitations.length + + existingCourseUsers.length + + pendingInvitationUpdates.length + + pendingCourseUserUpdates.length; + + const handleDialogClose = (_event: object, reason: string): void => { + if (reason !== 'backdropClick') handleClose(); + }; + return ( = (props) => { maxWidth="lg" onClose={handleDialogClose} open={open} - sx={styles.dialogStyle} + sx={{ + top: 40, + '& .MuiDialog-paper': { + display: 'flex', + flexDirection: 'column', + overflowY: 'hidden', + }, + }} > - {intl.formatMessage(translations.header)} - + {t(translations.header)} + - {intl.formatMessage(translations.body, { - newInvitationsCount: newInvitations?.length ?? 0, - newCourseUsersCount: newCourseUsers?.length ?? 0, + {needsAttentionCount > 0 && + `${t(translations.summaryFailed, { count: needsAttentionCount })} `} + {t(translations.summary, { + newInvitations: newInvitations.length, + newEnrollments: newCourseUsers.length, + alreadyInCourse: alreadyInCourseCount, })} - {duplicateUsers && duplicateUsers.length > 0 && ( -
- - - - - {intl.formatMessage(translations.duplicateUsers, { - count: duplicateUsers.length, - })} - + + {needsAttentionCount > 0 && ( +
+ + + {t(translations.actionableTitle, { + count: needsAttentionCount, + })} + + {failedInvitations.length > 0 && ( + + {t(translations.failedRowsSubtitle, { + count: failedInvitations.length, + })} + + )} + -
+ )} - {existingInvitations && existingInvitations.length > 0 && ( -
- - - - - {intl.formatMessage(translations.existingInvitations, { - count: existingInvitations.length, + + {pendingCount > 0 && ( +
+ + + + + {t(translations.externalIdUpdates, { count: pendingCount })} + + + + {applied + ? t(translations.externalIdUpdatedDone) + : t(translations.pendingExternalIdInfo, { + count: pendingCount, + })} + + + + {pendingInvitationUpdates.length > 0 && ( + <> + + {t(translations.invitationsSubheading, { + count: pendingInvitationUpdates.length, })} + + + )} + + {pendingCourseUserUpdates.length > 0 && ( + <> + + {t(translations.enrolledMembersSubheading, { + count: pendingCourseUserUpdates.length, + })} + + + + )} +
+ )} + + {newInvitationRows.length > 0 && ( +
+ + {t(translations.newInvitations, { + count: newInvitationRows.length, + })} + + -
+ )} - {existingCourseUsers && existingCourseUsers.length > 0 && ( -
- - - - - {intl.formatMessage(translations.existingCourseUsers, { - count: existingCourseUsers.length, - })} - + + {newCourseUserRows.length > 0 && ( +
+ + {t(translations.newCourseUsers, { + count: newCourseUserRows.length, + })} + + -
+ )} - {newInvitations && newInvitations.length > 0 && ( -
- - {intl.formatMessage(translations.newInvitations, { - count: newInvitations.length, - })} - + + {existingInvitationRows.length > 0 && ( +
+ + + + + {t(translations.existingInvitations, { + count: existingInvitationRows.length, + })} + + -
+ )} - {newCourseUsers && newCourseUsers.length > 0 && ( -
- - {intl.formatMessage(translations.newCourseUsers, { - count: newCourseUsers.length, - })} - + + {existingCourseUserRows.length > 0 && ( +
+ + + + + {t(translations.existingCourseUsers, { + count: existingCourseUserRows.length, + })} + + -
+ )} - ); }; -export default injectIntl(InvitationResultDialog); +export default InvitationResultDialog; diff --git a/client/app/bundles/course/user-invitations/components/misc/__test__/InvitationResultDialog.test.tsx b/client/app/bundles/course/user-invitations/components/misc/__test__/InvitationResultDialog.test.tsx new file mode 100644 index 00000000000..7d290a38a7e --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/misc/__test__/InvitationResultDialog.test.tsx @@ -0,0 +1,696 @@ +import React from 'react'; +import userEvent from '@testing-library/user-event'; +import { fireEvent, render, screen, waitFor, waitForElementToBeRemoved } from 'test-utils'; +import { CourseUserData } from 'types/course/courseUsers'; +import { + FailedInvitationRowData, + InvitationListData, + InvitationResult, + InvitationUpdatedItem, +} from 'types/course/userInvitations'; + +import * as operations from '../../../operations'; +import InvitationResultDialog from '../InvitationResultDialog'; + +jest.mock('../../../operations', () => ({ + ...jest.requireActual('../../../operations'), + updateExternalIds: jest.fn(), +})); + +const CAROL_EMAIL = 'carol@example.com'; +const EXT_ID_UPDATES_1 = 'External IDs to update (1)'; + +const carolDuplicateUser: FailedInvitationRowData = { + id: 3, + name: 'Carol', + email: CAROL_EMAIL, + role: 'student', + reason: 'duplicate_email_in_file', +}; + +const baseInvitation: InvitationListData = { + id: 1, + name: 'Alice', + email: 'alice@example.com', + externalId: null, + role: 'student', + phantom: false, + invitationKey: 'abc', + confirmed: false, + sentAt: null, + confirmedAt: null, + isRetryable: true, +}; + +const failedInvitation: InvitationListData = { + ...baseInvitation, + id: 99, + name: 'FailedUser', + email: 'failed@example.com', + isRetryable: false, +}; + +const baseCourseUser = { + id: 2, + name: 'Bob', + email: 'bob@example.com', + externalId: null, + role: 'student', + phantom: false, + level: 0, + exp: 0, + canReadStatistics: false, +} as CourseUserData; + +const noop = (): void => {}; + +const renderDialog = (result: InvitationResult): void => { + render( + , + ); +}; + +describe('InvitationResultDialog', () => { + // Lifecycle + it('renders nothing when closed', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('hides all optional sections when data is empty', async () => { + renderDialog({}); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/Failed/)).not.toBeInTheDocument(); + expect(screen.queryByText(/New Invitations/)).not.toBeInTheDocument(); + expect(screen.queryByText(/New Course Users/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Existing Invitations/)).not.toBeInTheDocument(); + expect(screen.queryByText(/Existing Course Users/)).not.toBeInTheDocument(); + }); + + it('does not close when backdrop is clicked', async () => { + const handleClose = jest.fn(); + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + + const backdrop = document.querySelector('.MuiBackdrop-root'); + if (backdrop) fireEvent.click(backdrop); + + expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(handleClose).not.toHaveBeenCalled(); + }); + + // Summary line + it('shows summary text with correct counts', async () => { + renderDialog({ + newInvitations: [baseInvitation], + newCourseUsers: [baseCourseUser], + existingInvitations: [{ ...baseInvitation, id: 3 }], + existingCourseUsers: [{ ...baseCourseUser, id: 4 }], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText( + /1 new invitation sent, 1 directly enrolled, 2 already in course/, + ), + ).toBeInTheDocument(); + }); + + it('prepends failed count to summary when failures present', async () => { + renderDialog({ failedUsers: [carolDuplicateUser] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText(/1 failed\./)).toBeInTheDocument(); + }); + + it('does not prepend failed count to summary when count is zero', async () => { + renderDialog({ newInvitations: [baseInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/failed\./)).not.toBeInTheDocument(); + }); + + // Failed section + it('shows Failed section when failedUsers is non-empty', async () => { + renderDialog({ failedUsers: [carolDuplicateUser] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Failed (1)')).toBeInTheDocument(); + expect(screen.getByText('Carol')).toBeInTheDocument(); + }); + + it('renders Failed before New Invitations in DOM', async () => { + renderDialog({ + newInvitations: [baseInvitation], + failedUsers: [carolDuplicateUser], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + const headings = screen + .getAllByRole('heading') + .map((h) => h.textContent ?? ''); + const needsIdx = headings.findIndex((h) => h.includes('Failed')); + const newInvIdx = headings.findIndex((h) => h.includes('New Invitations')); + expect(needsIdx).toBeGreaterThanOrEqual(0); + expect(newInvIdx).toBeGreaterThanOrEqual(0); + expect(needsIdx).toBeLessThan(newInvIdx); + }); + + it('shows failed invitation in Failed, not Existing Invitations', async () => { + renderDialog({ existingInvitations: [failedInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Failed (1)')).toBeInTheDocument(); + expect(screen.getByText('FailedUser')).toBeInTheDocument(); + expect( + screen.getByText( + 'Failed to send invitation email, please try again - if failures persist, contact us for assistance', + ), + ).toBeInTheDocument(); + expect(screen.queryByText(/Existing Invitations/)).not.toBeInTheDocument(); + }); + + it('shows failed invitation with non-null externalId in Failed, not Existing Invitations', async () => { + const failedWithExtId: InvitationListData = { + ...baseInvitation, + id: 100, + name: 'FailedWithExtId', + email: 'failedext@example.com', + externalId: 'old-id', + isRetryable: false, + }; + renderDialog({ existingInvitations: [failedWithExtId] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Failed (1)')).toBeInTheDocument(); + expect(screen.getByText('FailedWithExtId')).toBeInTheDocument(); + expect(screen.queryByText(/Existing Invitations/)).not.toBeInTheDocument(); + }); + + it('keeps retryable existing invitation in Existing Invitations section', async () => { + renderDialog({ + existingInvitations: [{ ...baseInvitation, id: 7, name: 'Retryable' }], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Existing Invitations (1)')).toBeInTheDocument(); + expect(screen.getByText('Retryable')).toBeInTheDocument(); + expect(screen.queryByText(/Failed/)).not.toBeInTheDocument(); + }); + + it('does not count failed invitations in alreadyInCourse summary', async () => { + renderDialog({ + existingInvitations: [failedInvitation], + existingCourseUsers: [baseCourseUser], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText( + /0 new invitations sent, 0 directly enrolled, 1 already in course/, + ), + ).toBeInTheDocument(); + }); + + it('shows failedRowsSubtitle when failed_to_send rows exist', async () => { + renderDialog({ existingInvitations: [failedInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText('1 row highlighted in red could not be sent'), + ).toBeInTheDocument(); + }); + + it('does not show failedRowsSubtitle when only duplicate users exist', async () => { + renderDialog({ failedUsers: [carolDuplicateUser] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/highlighted in red/)).not.toBeInTheDocument(); + }); + + it('shows failedRowsSubtitle with failed_to_send count only when mixed failures', async () => { + renderDialog({ + existingInvitations: [failedInvitation], + failedUsers: [carolDuplicateUser], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Failed (2)')).toBeInTheDocument(); + expect( + screen.getByText('1 row highlighted in red could not be sent'), + ).toBeInTheDocument(); + }); + + // New Invitations section + it('shows New Invitations section when newInvitations is non-empty', async () => { + renderDialog({ newInvitations: [baseInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('New Invitations (1)')).toBeInTheDocument(); + expect(screen.getByText('Alice')).toBeInTheDocument(); + }); + + it('hides New Invitations section when empty', async () => { + renderDialog({ newInvitations: [] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/New Invitations/)).not.toBeInTheDocument(); + }); + + // New Course Users section + it('shows New Course Users section when non-empty', async () => { + renderDialog({ newCourseUsers: [baseCourseUser] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('New Course Users (1)')).toBeInTheDocument(); + expect(screen.getByText('Bob')).toBeInTheDocument(); + }); + + it('hides New Course Users section when empty', async () => { + renderDialog({ newCourseUsers: [] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/New Course Users/)).not.toBeInTheDocument(); + }); + + // Existing Invitations section + it('shows Existing Invitations section with name visible', async () => { + renderDialog({ + existingInvitations: [{ ...baseInvitation, id: 5, name: 'Charlie' }], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Existing Invitations (1)')).toBeInTheDocument(); + expect(screen.getByText('Charlie')).toBeInTheDocument(); + }); + + it('hides Existing Invitations section when both existingInvitations and pendingInvitationUpdates are empty', async () => { + renderDialog({ newInvitations: [baseInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/Existing Invitations/)).not.toBeInTheDocument(); + }); + + it('shows pending invitation diffs in the dedicated section, not Existing Invitations', async () => { + const pendingItem: InvitationUpdatedItem = { + id: 10, + name: 'Carol', + email: CAROL_EMAIL, + externalId: 'NEW001', + previousExternalId: 'OLD001', + role: 'student', + phantom: false, + }; + renderDialog({ pendingInvitationUpdates: [pendingItem] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText(EXT_ID_UPDATES_1)).toBeInTheDocument(); + expect(screen.getByText('Invitations (1)')).toBeInTheDocument(); + expect(screen.queryByText(/Existing Invitations/)).not.toBeInTheDocument(); + expect(screen.getByText('Carol')).toBeInTheDocument(); + }); + + it('splits pending invitation diffs out of Existing Invitations but still counts them as already in course', async () => { + const pendingItem: InvitationUpdatedItem = { + id: 10, + name: 'UpdatedAlice', + email: 'updated@example.com', + externalId: 'NEW001', + previousExternalId: 'OLD001', + role: 'student', + phantom: false, + }; + renderDialog({ + existingInvitations: [ + { ...baseInvitation, id: 5, name: 'NormalCharlie' }, + ], + pendingInvitationUpdates: [pendingItem], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Existing Invitations (1)')).toBeInTheDocument(); + expect(screen.getByText(EXT_ID_UPDATES_1)).toBeInTheDocument(); + expect(screen.getByText('Invitations (1)')).toBeInTheDocument(); + expect( + screen.getByText( + /0 new invitations sent, 0 directly enrolled, 2 already in course/, + ), + ).toBeInTheDocument(); + }); + + // Existing Course Users section + it('shows Existing Course Users section with name visible', async () => { + renderDialog({ + existingCourseUsers: [{ ...baseCourseUser, id: 6, name: 'Diana' }], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Existing Course Users (1)')).toBeInTheDocument(); + expect(screen.getByText('Diana')).toBeInTheDocument(); + }); + + it('hides Existing Course Users section when empty', async () => { + renderDialog({ newInvitations: [baseInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/Existing Course Users/)).not.toBeInTheDocument(); + }); + + it('shows pending course-user diffs in the dedicated section, not Existing Course Users', async () => { + const pendingUser: InvitationUpdatedItem = { + id: 20, + name: 'Dana', + email: 'dana@example.com', + externalId: 'CU001', + previousExternalId: 'CU000', + role: 'student', + phantom: false, + }; + renderDialog({ pendingCourseUserUpdates: [pendingUser] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText(EXT_ID_UPDATES_1)).toBeInTheDocument(); + expect(screen.getByText('Enrolled members (1)')).toBeInTheDocument(); + expect(screen.queryByText(/Existing Course Users/)).not.toBeInTheDocument(); + expect(screen.getByText('Dana')).toBeInTheDocument(); + }); + + it('splits pending course-user diffs out of Existing Course Users but still counts them as already in course', async () => { + const pendingUser: InvitationUpdatedItem = { + id: 20, + name: 'UpdatedDana', + email: 'updated-dana@example.com', + externalId: 'CU001', + previousExternalId: 'CU000', + role: 'student', + phantom: false, + }; + renderDialog({ + existingCourseUsers: [{ ...baseCourseUser, id: 6, name: 'NormalEve' }], + pendingCourseUserUpdates: [pendingUser], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Existing Course Users (1)')).toBeInTheDocument(); + expect(screen.getByText(EXT_ID_UPDATES_1)).toBeInTheDocument(); + expect(screen.getByText('Enrolled members (1)')).toBeInTheDocument(); + expect( + screen.getByText( + /0 new invitations sent, 0 directly enrolled, 2 already in course/, + ), + ).toBeInTheDocument(); + }); + + describe('Personalized Timeline column (showPersonalizedTimelineFeatures)', () => { + const storeWithTimelines = { + invitations: { + invitations: { ids: [], entities: {}, byId: {} }, + permissions: { + canManageCourseUsers: false, + canManageEnrolRequests: false, + canManageReferenceTimelines: false, + canManagePersonalTimes: false, + canRegisterWithCode: false, + }, + manageCourseUsersData: { + requestsCount: 0, + invitationsCount: 0, + defaultTimelineAlgorithm: 'fixed' as const, + showPersonalizedTimelineFeatures: true, + }, + courseRegistrationKey: '', + }, + }; + + it('shows Personalized Timeline column in new invitations table when feature is on', async () => { + render( + , + { state: storeWithTimelines }, + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('Otot')).toBeInTheDocument(); + }); + + it('hides Personalized Timeline column when feature is off', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.queryByText('Personalized Timeline'), + ).not.toBeInTheDocument(); + }); + }); + + it('renders External IDs to update before New Invitations in DOM', async () => { + renderDialog({ + newInvitations: [baseInvitation], + pendingInvitationUpdates: [ + { + id: 10, + name: 'PendingPat', + email: 'pat@example.com', + externalId: 'NEW001', + previousExternalId: 'OLD001', + role: 'student', + phantom: false, + }, + ], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + const headings = screen + .getAllByRole('heading') + .map((h) => h.textContent ?? ''); + const updIdx = headings.findIndex((h) => h.includes('External IDs to update')); + const newInvIdx = headings.findIndex((h) => h.includes('New Invitations')); + expect(updIdx).toBeGreaterThanOrEqual(0); + expect(newInvIdx).toBeGreaterThanOrEqual(0); + expect(updIdx).toBeLessThan(newInvIdx); + }); + + it('shows a single combined count and both sub-tables when both arrays are non-empty', async () => { + renderDialog({ + pendingInvitationUpdates: [ + { + id: 10, + name: 'PendingPat', + email: 'pat@example.com', + externalId: 'NEW001', + previousExternalId: 'OLD001', + role: 'student', + phantom: false, + }, + ], + pendingCourseUserUpdates: [ + { + id: 20, + name: 'PendingDana', + email: 'dana@example.com', + externalId: 'CU001', + previousExternalId: 'CU000', + role: 'student', + phantom: false, + }, + ], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('External IDs to update (2)')).toBeInTheDocument(); + expect(screen.getByText('Invitations (1)')).toBeInTheDocument(); + expect(screen.getByText('Enrolled members (1)')).toBeInTheDocument(); + expect(screen.getByText('PendingPat')).toBeInTheDocument(); + expect(screen.getByText('PendingDana')).toBeInTheDocument(); + }); + + it('exposes the keep-by-default explanation as a tooltip on the section', async () => { + renderDialog({ + pendingInvitationUpdates: [ + { + id: 10, + name: 'PendingPat', + email: 'pat@example.com', + externalId: 'NEW001', + previousExternalId: 'OLD001', + role: 'student', + phantom: false, + }, + ], + }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + // Tooltip text is rendered (in the accessible tree) by MUI's Tooltip title prop. + expect( + screen.getByLabelText( + 'These users are already invited or enrolled. Their External ID changes only if you apply the file values.', + ), + ).toBeInTheDocument(); + }); + + describe('pending external-ID apply', () => { + const pendingResult: InvitationResult = { + pendingCourseUserUpdates: [ + { + id: 1, + name: 'Alice', + email: 'alice@example.com', + externalId: 'NEW001', + previousExternalId: 'OLD001', + role: 'student', + phantom: false, + }, + ], + }; + + beforeEach(() => { + (operations.updateExternalIds as jest.Mock).mockReturnValue( + async () => undefined, + ); + }); + + it('shows the flagged status line and Apply button when diffs exist', async () => { + renderDialog(pendingResult); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText(/1 row has a different External ID/), + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /Apply changes \(1\)/ }), + ).toBeInTheDocument(); + expect(screen.getByText(EXT_ID_UPDATES_1)).toBeInTheDocument(); + expect(screen.getByText('Enrolled members (1)')).toBeInTheDocument(); + }); + + it('calls backend with file values, shows Changes applied button disabled and updated status line', async () => { + const user = userEvent.setup(); + renderDialog(pendingResult); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + + await user.click( + screen.getByRole('button', { name: /Apply changes \(1\)/ }), + ); + + expect(operations.updateExternalIds).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + type: 'course_user', + id: 1, + externalId: 'NEW001', + }), + ]), + ); + expect( + screen.getByText(/External IDs were updated from your file/), + ).toBeInTheDocument(); + const changesAppliedBtn = screen.getByRole('button', { + name: /Changes applied/, + }); + expect(changesAppliedBtn).toBeInTheDocument(); + expect(changesAppliedBtn).toBeDisabled(); + }); + + it('shows no Apply affordance or pending-diff status text when there are no diffs', async () => { + renderDialog({ newInvitations: [baseInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.queryByRole('button', { name: /Apply changes/ }), + ).not.toBeInTheDocument(); + expect( + screen.queryByText(/different External ID/), + ).not.toBeInTheDocument(); + }); + + it('on apply failure shows error toast and leaves button at Apply', async () => { + (operations.updateExternalIds as jest.Mock).mockReturnValue(async () => { + throw Object.assign(new Error('conflict'), { + response: { data: { conflictingExternalId: 'TAKEN' } }, + }); + }); + + const user = userEvent.setup(); + renderDialog(pendingResult); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + + await user.click( + screen.getByRole('button', { name: /Apply changes \(1\)/ }), + ); + + expect( + screen.getByRole('button', { name: /Apply changes \(1\)/ }), + ).toBeInTheDocument(); + await screen.findByText( + /External ID 'TAKEN' is now used by another member/, + ); + }); + + it('disables Apply button while request is in flight', async () => { + let resolveFn!: () => void; + const pending = new Promise((res) => { + resolveFn = res; + }); + (operations.updateExternalIds as jest.Mock).mockReturnValue(() => pending); + + const user = userEvent.setup(); + renderDialog(pendingResult); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + + const applyBtn = screen.getByRole('button', { name: /Apply changes \(1\)/ }); + const clickPromise = user.click(applyBtn); + await waitFor(() => expect(applyBtn).toBeDisabled()); + + resolveFn(); + await clickPromise; + expect( + screen.getByRole('button', { name: /Changes applied/ }), + ).toBeDisabled(); + }); + + it('resets to Apply button when dialog closes and reopens', async () => { + const user = userEvent.setup(); + + const ToggleWrapper: React.FC = () => { + const [open, setOpen] = React.useState(true); + return ( + <> + + + + + ); + }; + + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + + await user.click(screen.getByRole('button', { name: /Apply changes \(1\)/ })); + expect( + screen.getByRole('button', { name: /Changes applied/ }), + ).toBeDisabled(); + + // These buttons are inside the aria-hidden backdrop; use fireEvent to bypass the a11y filter. + fireEvent.click(screen.getByRole('button', { name: 'close', hidden: true })); + fireEvent.click(screen.getByRole('button', { name: 'reopen', hidden: true })); + + await waitFor( + () => + expect( + screen.getByRole('button', { name: /Apply changes \(1\)/ }), + ).not.toBeDisabled(), + { timeout: 5000 }, + ); + }); + + it('renders no dedicated diff section when there are no diffs', async () => { + renderDialog({ newInvitations: [baseInvitation] }); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText(/External IDs to update/)).not.toBeInTheDocument(); + }); + }); +}); diff --git a/client/app/bundles/course/user-invitations/components/tables/ExternalIdUpdateTable.tsx b/client/app/bundles/course/user-invitations/components/tables/ExternalIdUpdateTable.tsx new file mode 100644 index 00000000000..3e0d86bbad9 --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/ExternalIdUpdateTable.tsx @@ -0,0 +1,147 @@ +import { FC, ReactNode } from 'react'; +import { defineMessages } from 'react-intl'; +import { InvitationUpdatedItem } from 'types/course/userInvitations'; + +import { ColumnTemplate } from 'lib/components/table'; +import Table from 'lib/components/table/Table'; +import { + DEFAULT_MINI_TABLE_ROWS_PER_PAGE, + TIMELINE_ALGORITHMS, +} from 'lib/constants/sharedConstants'; +import useTranslation from 'lib/hooks/useTranslation'; +import roleTranslations from 'lib/translations/course/users/roles'; +import tableTranslations from 'lib/translations/table'; + +const translations = defineMessages({ + yes: { + id: 'course.userInvitations.ExternalIdUpdateTable.yes', + defaultMessage: 'Yes', + }, + no: { + id: 'course.userInvitations.ExternalIdUpdateTable.no', + defaultMessage: 'No', + }, + existingColumn: { + id: 'course.userInvitations.ExternalIdUpdateTable.existingColumn', + defaultMessage: 'Existing', + }, + inFileColumn: { + id: 'course.userInvitations.ExternalIdUpdateTable.inFileColumn', + defaultMessage: 'In file', + }, + currentColumn: { + id: 'course.userInvitations.ExternalIdUpdateTable.currentColumn', + defaultMessage: '{label} (current)', + }, +}); + +interface Props { + rows: InvitationUpdatedItem[]; + applied: boolean; + showPersonalizedTimelineFeatures?: boolean; +} + +// The currently-live column (Existing before apply, In file after) gets a full-cell +// blue fill; the other column is muted. Column `className` lands on the MUI TableCell, +// so the whole cell fills cleanly (it also tints the column's header cell — intended: +// the live column reads as one highlighted column, with a "(current)" header label). +const LIVE_CELL_CLASS = 'bg-blue-100 font-semibold text-blue-900'; +const MUTED_CELL_CLASS = 'text-neutral-400'; + +const ExternalIdUpdateTable: FC = ({ + rows, + applied, + showPersonalizedTimelineFeatures, +}) => { + const { t } = useTranslation(); + + if (rows.length === 0) return null; + + const currentTitle = (label: string): string => + t(translations.currentColumn, { label }); + + const columns: ColumnTemplate[] = [ + { + of: 'name', + title: t(tableTranslations.name), + sortable: false, + cell: (row) => row.name, + csvDownloadable: true, + }, + { + of: 'email', + title: t(tableTranslations.email), + sortable: false, + cell: (row) => row.email, + csvDownloadable: true, + }, + { + of: 'previousExternalId', + title: applied + ? t(translations.existingColumn) + : currentTitle(t(translations.existingColumn)), + className: applied ? MUTED_CELL_CLASS : LIVE_CELL_CLASS, + sortable: false, + cell: (row): ReactNode => row.previousExternalId ?? '—', + csvDownloadable: true, + }, + { + of: 'externalId', + title: applied + ? currentTitle(t(translations.inFileColumn)) + : t(translations.inFileColumn), + className: applied ? LIVE_CELL_CLASS : MUTED_CELL_CLASS, + sortable: false, + cell: (row): ReactNode => row.externalId ?? '—', + csvDownloadable: true, + }, + { + of: 'role', + title: t(tableTranslations.role), + sortable: false, + cell: (row): ReactNode => { + const desc = + roleTranslations[row.role as keyof typeof roleTranslations]; + return desc ? t(desc) : row.role; + }, + csvDownloadable: true, + }, + { + of: 'phantom', + title: t(tableTranslations.phantom), + sortable: false, + cell: (row) => (row.phantom ? t(translations.yes) : t(translations.no)), + csvDownloadable: true, + }, + ...(showPersonalizedTimelineFeatures + ? ([ + { + of: 'timelineAlgorithm', + title: t(tableTranslations.personalizedTimeline), + sortable: false, + cell: (row): ReactNode => + TIMELINE_ALGORITHMS.find( + (tl) => tl.value === row.timelineAlgorithm, + )?.label ?? '-', + csvDownloadable: true, + }, + ] as ColumnTemplate[]) + : []), + ]; + + return ( + ({ ...row, applied })} + getRowId={(row) => String(row.id)} + pagination={{ + rowsPerPage: [DEFAULT_MINI_TABLE_ROWS_PER_PAGE], + showAllRows: true, + }} + toolbar={{ show: false }} + /> + ); +}; + +export default ExternalIdUpdateTable; diff --git a/client/app/bundles/course/user-invitations/components/tables/InvitationResultExistingTable.tsx b/client/app/bundles/course/user-invitations/components/tables/InvitationResultExistingTable.tsx new file mode 100644 index 00000000000..625b753ddc7 --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/InvitationResultExistingTable.tsx @@ -0,0 +1,127 @@ +import { FC, ReactNode } from 'react'; +import { defineMessages } from 'react-intl'; +import { TimelineAlgorithm } from 'types/course/personalTimes'; + +import { ColumnTemplate } from 'lib/components/table'; +import Table from 'lib/components/table/Table'; +import { + DEFAULT_TABLE_ROWS_PER_PAGE, + TIMELINE_ALGORITHMS, +} from 'lib/constants/sharedConstants'; +import useTranslation from 'lib/hooks/useTranslation'; +import roleTranslations from 'lib/translations/course/users/roles'; +import tableTranslations from 'lib/translations/table'; + +const translations = defineMessages({ + yes: { + id: 'course.userInvitations.InvitationResultExistingTable.yes', + defaultMessage: 'Yes', + }, + no: { + id: 'course.userInvitations.InvitationResultExistingTable.no', + defaultMessage: 'No', + }, +}); + +export interface ExistingRow { + id: number; + name: string; + email: string; + externalId?: string | null; + role?: string; + phantom?: boolean; + timelineAlgorithm?: TimelineAlgorithm; +} + +interface Props { + rows: ExistingRow[]; + showPersonalizedTimelineFeatures?: boolean; +} + +const InvitationResultExistingTable: FC = ({ + rows, + showPersonalizedTimelineFeatures, +}) => { + const { t } = useTranslation(); + + if (rows.length === 0) return null; + + const showExternalId = rows.some((r) => r.externalId != null); + + const columns: ColumnTemplate[] = [ + { + of: 'name', + title: t(tableTranslations.name), + sortable: false, + cell: (row) => row.name, + csvDownloadable: true, + }, + { + of: 'email', + title: t(tableTranslations.email), + sortable: false, + cell: (row) => row.email, + csvDownloadable: true, + }, + ...(showExternalId + ? ([ + { + of: 'externalId', + title: t(tableTranslations.externalId), + sortable: false, + cell: (row): ReactNode => row.externalId ?? '', + csvDownloadable: true, + }, + ] as ColumnTemplate[]) + : []), + { + of: 'role', + title: t(tableTranslations.role), + sortable: false, + cell: (row): ReactNode => { + if (!row.role) return ''; + const desc = + roleTranslations[row.role as keyof typeof roleTranslations]; + return desc ? t(desc) : row.role; + }, + csvDownloadable: true, + }, + { + of: 'phantom', + title: t(tableTranslations.phantom), + sortable: false, + cell: (row) => (row.phantom ? t(translations.yes) : t(translations.no)), + csvDownloadable: true, + }, + ...(showPersonalizedTimelineFeatures + ? ([ + { + of: 'timelineAlgorithm', + title: t(tableTranslations.personalizedTimeline), + sortable: false, + cell: (row): ReactNode => + TIMELINE_ALGORITHMS.find( + (tl) => tl.value === row.timelineAlgorithm, + )?.label ?? '-', + csvDownloadable: true, + }, + ] as ColumnTemplate[]) + : []), + ]; + + return ( +
row} + getRowId={(row) => String(row.id)} + pagination={{ + rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], + showAllRows: true, + }} + toolbar={{ show: false }} + /> + ); +}; + +export default InvitationResultExistingTable; diff --git a/client/app/bundles/course/user-invitations/components/tables/InvitationResultFailedTable.tsx b/client/app/bundles/course/user-invitations/components/tables/InvitationResultFailedTable.tsx new file mode 100644 index 00000000000..83ba8eae886 --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/InvitationResultFailedTable.tsx @@ -0,0 +1,163 @@ +import { FC, memo, ReactNode } from 'react'; +import { defineMessages } from 'react-intl'; +import equal from 'fast-deep-equal'; +import { + FailedInvitationRowData, + InvitationFailureReason, +} from 'types/course/userInvitations'; + +import { ColumnTemplate } from 'lib/components/table'; +import Table from 'lib/components/table/Table'; +import { TIMELINE_ALGORITHMS } from 'lib/constants/sharedConstants'; +import useTranslation from 'lib/hooks/useTranslation'; +import roleTranslations from 'lib/translations/course/users/roles'; +import tableTranslations from 'lib/translations/table'; + +const translations = defineMessages({ + duplicateEmailInFile: { + id: 'course.userInvitations.InvitationResultFailedTable.duplicateEmailInFile', + defaultMessage: 'Duplicate email in uploaded CSV', + }, + duplicateExternalIdInFile: { + id: 'course.userInvitations.InvitationResultFailedTable.duplicateExternalIdInFile', + defaultMessage: 'Duplicate external ID in uploaded CSV', + }, + externalIdTaken: { + id: 'course.userInvitations.InvitationResultFailedTable.externalIdTaken', + defaultMessage: 'External ID is taken by another course member', + }, + failedToSend: { + id: 'course.userInvitations.InvitationResultFailedTable.failedToSend', + defaultMessage: + 'Failed to send invitation email, please try again - if failures persist, contact us for assistance', + }, + yes: { + id: 'course.userInvitations.InvitationResultFailedTable.yes', + defaultMessage: 'Yes', + }, + no: { + id: 'course.userInvitations.InvitationResultFailedTable.no', + defaultMessage: 'No', + }, +}); + +interface Props { + users: FailedInvitationRowData[]; + showPersonalizedTimelineFeatures?: boolean; +} + +const REASON_MAP: Record = { + duplicate_email_in_file: 'duplicateEmailInFile', + duplicate_external_id_in_file: 'duplicateExternalIdInFile', + external_id_taken: 'externalIdTaken', + failed_to_send: 'failedToSend', +}; + +const InvitationResultFailedTable: FC = ({ + users, + showPersonalizedTimelineFeatures, +}) => { + const { t } = useTranslation(); + + if (users.length === 0) return null; + + const showExternalId = users.some((u) => u.externalId != null); + + const columns: ColumnTemplate[] = [ + { + of: 'name', + title: t(tableTranslations.name), + sortable: false, + searchable: true, + cell: (row) => row.name, + csvDownloadable: true, + }, + { + of: 'email', + title: t(tableTranslations.email), + sortable: false, + searchable: true, + cell: (row) => row.email, + csvDownloadable: true, + }, + ...(showExternalId + ? ([ + { + of: 'externalId', + title: t(tableTranslations.externalId), + sortable: false, + searchable: false, + cell: (row) => row.externalId ?? '', + csvDownloadable: true, + }, + ] as ColumnTemplate[]) + : []), + { + of: 'role', + title: t(tableTranslations.role), + sortable: false, + searchable: false, + cell: (row): ReactNode => { + if (!row.role) return ''; + const desc = + roleTranslations[row.role as keyof typeof roleTranslations]; + return desc ? t(desc) : row.role; + }, + csvDownloadable: true, + }, + { + of: 'phantom', + title: t(tableTranslations.phantom), + sortable: false, + searchable: false, + cell: (row) => (row.phantom ? t(translations.yes) : t(translations.no)), + csvDownloadable: true, + }, + ...(showPersonalizedTimelineFeatures + ? ([ + { + of: 'timelineAlgorithm', + title: t(tableTranslations.personalizedTimeline), + sortable: false, + searchable: false, + cell: (row): ReactNode => + TIMELINE_ALGORITHMS.find( + (tl) => tl.value === row.timelineAlgorithm, + )?.label ?? '-', + csvDownloadable: true, + }, + ] as ColumnTemplate[]) + : []), + { + of: 'reason', + title: t(tableTranslations.reason), + sortable: false, + searchable: false, + cell: (row): ReactNode => { + return t(translations[REASON_MAP[row.reason]]); + }, + csvDownloadable: true, + }, + ]; + + return ( +
+ row.reason === 'failed_to_send' ? 'bg-[#ffebee]' : '' + } + getRowEqualityData={(row) => row} + getRowId={(row) => String(row.id)} + toolbar={{ show: false }} + /> + ); +}; + +export default memo( + InvitationResultFailedTable, + (prev, next) => + equal(prev.users, next.users) && + prev.showPersonalizedTimelineFeatures === + next.showPersonalizedTimelineFeatures, +); diff --git a/client/app/bundles/course/user-invitations/components/tables/InvitationResultInvitationsTable.tsx b/client/app/bundles/course/user-invitations/components/tables/InvitationResultInvitationsTable.tsx deleted file mode 100644 index fb70f544196..00000000000 --- a/client/app/bundles/course/user-invitations/components/tables/InvitationResultInvitationsTable.tsx +++ /dev/null @@ -1,152 +0,0 @@ -import { FC, memo } from 'react'; -import { Typography } from '@mui/material'; -import equal from 'fast-deep-equal'; -import { TableColumns, TableOptions } from 'types/components/DataTable'; -import { InvitationListData } from 'types/course/userInvitations'; - -import DataTable from 'lib/components/core/layouts/DataTable'; -import { DEFAULT_TABLE_ROWS_PER_PAGE } from 'lib/constants/sharedConstants'; -import useTranslation from 'lib/hooks/useTranslation'; -import roleTranslations from 'lib/translations/course/users/roles'; -import tableTranslations from 'lib/translations/table'; - -interface Props { - title: JSX.Element; - invitations: InvitationListData[]; -} - -const InvitationResultInvitationsTable: FC = (props) => { - const { title, invitations } = props; - const { t } = useTranslation(); - - if (invitations && invitations.length === 0) return null; - - const showExternalId = invitations.some((i) => i.externalId != null); - - const options: TableOptions = { - download: true, - filter: false, - pagination: true, - print: false, - rowsPerPage: DEFAULT_TABLE_ROWS_PER_PAGE, - rowsPerPageOptions: [DEFAULT_TABLE_ROWS_PER_PAGE], - search: false, - selectableRows: 'none', - setTableProps: (): object => { - return { size: 'small' }; - }, - setRowProps: (_row, dataIndex, _rowIndex): Record => { - return { - key: `invitation_result_invitation_${invitations[dataIndex].id}`, - invitationid: `invitation_result_invitation_${invitations[dataIndex].id}`, - className: `invitation_result_invitation invitation_result_invitation_${invitations[dataIndex].id}`, - }; - }, - viewColumns: false, - }; - - const columns: TableColumns[] = [ - { - name: 'id', - label: t(tableTranslations.id), - options: { - display: false, - filter: false, - sort: false, - }, - }, - { - name: 'name', - label: t(tableTranslations.name), - options: { - alignCenter: false, - sort: false, - }, - }, - { - name: 'email', - label: t(tableTranslations.email), - options: { - alignCenter: false, - sort: false, - }, - }, - ...(showExternalId - ? [ - { - name: 'externalId', - label: t(tableTranslations.externalId), - options: { - alignCenter: true, - sort: false, - }, - }, - ] - : []), - { - name: 'phantom', - label: t(tableTranslations.phantom), - options: { - sort: false, - customBodyRenderLite: (dataIndex): JSX.Element => { - const invitation = invitations[dataIndex]; - return ( - - {invitation.phantom ? 'Yes' : 'No'} - - ); - }, - }, - }, - { - name: 'role', - label: t(tableTranslations.role), - options: { - alignCenter: false, - sort: false, - customBodyRenderLite: (dataIndex): JSX.Element => { - const invitation = invitations[dataIndex]; - return ( - - {t(roleTranslations[invitation.role])} - - ); - }, - }, - }, - { - name: 'sentAt', - label: t(tableTranslations.invitationSentAt), - options: { - alignCenter: false, - sort: false, - }, - }, - ]; - - return ( - - ); -}; - -export default memo( - InvitationResultInvitationsTable, - (prevProps, nextProps) => { - return equal(prevProps.invitations, nextProps.invitations); - }, -); diff --git a/client/app/bundles/course/user-invitations/components/tables/InvitationResultPrimaryTable.tsx b/client/app/bundles/course/user-invitations/components/tables/InvitationResultPrimaryTable.tsx new file mode 100644 index 00000000000..e1ec0e89acd --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/InvitationResultPrimaryTable.tsx @@ -0,0 +1,113 @@ +import { FC, ReactNode } from 'react'; +import { defineMessages } from 'react-intl'; +import { InvitationSuccessRow } from 'types/course/userInvitations'; + +import { ColumnTemplate } from 'lib/components/table'; +import Table from 'lib/components/table/Table'; +import { + DEFAULT_TABLE_ROWS_PER_PAGE, + TIMELINE_ALGORITHMS, +} from 'lib/constants/sharedConstants'; +import useTranslation from 'lib/hooks/useTranslation'; +import roleTranslations from 'lib/translations/course/users/roles'; +import tableTranslations from 'lib/translations/table'; + +const translations = defineMessages({ + yes: { + id: 'course.userInvitations.InvitationResultPrimaryTable.yes', + defaultMessage: 'Yes', + }, + no: { + id: 'course.userInvitations.InvitationResultPrimaryTable.no', + defaultMessage: 'No', + }, +}); + +interface Props { + rows: InvitationSuccessRow[]; + showPersonalizedTimelineFeatures?: boolean; +} + +const InvitationResultPrimaryTable: FC = ({ + rows, + showPersonalizedTimelineFeatures, +}) => { + const { t } = useTranslation(); + const showExternalId = rows.some((r) => r.externalId != null); + + const columns: ColumnTemplate[] = [ + { + of: 'name', + title: t(tableTranslations.name), + sortable: false, + cell: (row) => row.name, + csvDownloadable: true, + }, + { + of: 'email', + title: t(tableTranslations.email), + sortable: false, + cell: (row) => row.email, + csvDownloadable: true, + }, + ...(showExternalId + ? ([ + { + of: 'externalId', + title: t(tableTranslations.externalId), + sortable: false, + cell: (row) => row.externalId ?? '', + csvDownloadable: true, + }, + ] as ColumnTemplate[]) + : []), + { + of: 'role', + title: t(tableTranslations.role), + sortable: false, + cell: (row): ReactNode => { + const desc = + roleTranslations[row.role as keyof typeof roleTranslations]; + return desc ? t(desc) : row.role; + }, + csvDownloadable: true, + }, + { + of: 'phantom', + title: t(tableTranslations.phantom), + sortable: false, + cell: (row) => (row.phantom ? t(translations.yes) : t(translations.no)), + csvDownloadable: true, + }, + ...(showPersonalizedTimelineFeatures + ? ([ + { + of: 'timelineAlgorithm', + title: t(tableTranslations.personalizedTimeline), + sortable: false, + cell: (row): ReactNode => + TIMELINE_ALGORITHMS.find( + (tl) => tl.value === row.timelineAlgorithm, + )?.label ?? '-', + csvDownloadable: true, + }, + ] as ColumnTemplate[]) + : []), + ]; + + return ( +
row} + getRowId={(row) => row.id} + pagination={{ + rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], + showAllRows: false, + }} + toolbar={{ show: false }} + /> + ); +}; + +export default InvitationResultPrimaryTable; diff --git a/client/app/bundles/course/user-invitations/components/tables/InvitationResultUsersTable.tsx b/client/app/bundles/course/user-invitations/components/tables/InvitationResultUsersTable.tsx deleted file mode 100644 index c8b0728fd6c..00000000000 --- a/client/app/bundles/course/user-invitations/components/tables/InvitationResultUsersTable.tsx +++ /dev/null @@ -1,190 +0,0 @@ -import { FC, memo } from 'react'; -import { defineMessages } from 'react-intl'; -import { Typography } from '@mui/material'; -import equal from 'fast-deep-equal'; -import { TableColumns, TableOptions } from 'types/components/DataTable'; -import { CourseUserListData } from 'types/course/courseUsers'; -import { DuplicateReason } from 'types/course/userInvitations'; - -import DataTable from 'lib/components/core/layouts/DataTable'; -import useTranslation from 'lib/hooks/useTranslation'; -import roleTranslations from 'lib/translations/course/users/roles'; -import tableTranslations from 'lib/translations/table'; - -const translations = defineMessages({ - duplicateEmailInFile: { - id: 'course.userInvitations.InvitationResultUsersTable.duplicateEmailInFile', - defaultMessage: 'Duplicate email in upload', - }, - duplicateExternalIdInFile: { - id: 'course.userInvitations.InvitationResultUsersTable.duplicateExternalIdInFile', - defaultMessage: 'Duplicate external ID in upload', - }, - externalIdTaken: { - id: 'course.userInvitations.InvitationResultUsersTable.externalIdTaken', - defaultMessage: 'External ID is already assigned to another course member', - }, -}); - -interface Props { - title: JSX.Element; - users: Array; -} - -const InvitationResultUsersTable: FC = (props) => { - const { title, users } = props; - const { t } = useTranslation(); - - if (users && users.length === 0) return null; - - const showExternalId = users.some((u) => u.externalId != null); - const showReason = users.some((u) => u.reason != null); - - const options: TableOptions = { - download: true, - filter: false, - pagination: false, - print: false, - search: false, - selectableRows: 'none', - setTableProps: (): object => { - return { size: 'small' }; - }, - setRowProps: (_row, dataIndex, _rowIndex): Record => { - return { - key: `invitation_result_user_${users[dataIndex].id}`, - userid: `invitation_result_user_${users[dataIndex].id}`, - className: `invitation_result_user invitation_result_user_${users[dataIndex].id}`, - }; - }, - viewColumns: false, - }; - - const columns: TableColumns[] = [ - { - name: 'id', - label: t(tableTranslations.id), - options: { - display: false, - filter: false, - sort: false, - }, - }, - { - name: 'name', - label: t(tableTranslations.name), - options: { - alignCenter: false, - sort: false, - }, - }, - { - name: 'email', - label: t(tableTranslations.email), - options: { - alignCenter: false, - sort: false, - }, - }, - ...(showExternalId - ? [ - { - name: 'externalId', - label: t(tableTranslations.externalId), - options: { - alignCenter: true, - sort: false, - }, - }, - ] - : []), - ...(showReason - ? [ - { - name: 'reason', - label: t(tableTranslations.reason), - options: { - alignCenter: false, - sort: false, - customBodyRenderLite: (dataIndex: number): JSX.Element => { - const user = users[dataIndex]; - const reasonText = - { - duplicate_email_in_file: t( - translations.duplicateEmailInFile, - ), - duplicate_external_id_in_file: t( - translations.duplicateExternalIdInFile, - ), - external_id_taken: t(translations.externalIdTaken), - }[user.reason ?? ''] ?? ''; - return ( - - {reasonText} - - ); - }, - }, - }, - ] - : []), - { - name: 'phantom', - label: t(tableTranslations.phantom), - options: { - sort: false, - customBodyRenderLite: (dataIndex): JSX.Element => { - const user = users[dataIndex]; - return ( - - {user.phantom ? 'Yes' : 'No'} - - ); - }, - }, - }, - { - name: 'role', - label: t(tableTranslations.role), - options: { - alignCenter: false, - sort: false, - customBodyRenderLite: (dataIndex): JSX.Element => { - const user = users[dataIndex]; - return ( - - {t(roleTranslations[user.role])} - - ); - }, - }, - }, - ]; - - return ( - - ); -}; - -export default memo(InvitationResultUsersTable, (prevProps, nextProps) => { - return equal(prevProps.users, nextProps.users); -}); diff --git a/client/app/bundles/course/user-invitations/components/tables/UserInvitationsTable.tsx b/client/app/bundles/course/user-invitations/components/tables/UserInvitationsTable.tsx index 86d2f9f69f2..c24b976f1f8 100644 --- a/client/app/bundles/course/user-invitations/components/tables/UserInvitationsTable.tsx +++ b/client/app/bundles/course/user-invitations/components/tables/UserInvitationsTable.tsx @@ -6,7 +6,6 @@ import { InvitationStatus, } from 'types/course/userInvitations'; -import { getManageCourseUserPermissions } from 'course/users/selectors'; import Note from 'lib/components/core/Note'; import GhostIcon from 'lib/components/icons/GhostIcon'; import { ColumnTemplate } from 'lib/components/table'; @@ -18,6 +17,7 @@ import { formatMiniDateTime } from 'lib/moment'; import roleTranslations from 'lib/translations/course/users/roles'; import tableTranslations from 'lib/translations/table'; +import { getManageCourseUsersSharedData } from '../../selectors'; import translations from '../../translations'; import InvitationActionButtons from '../buttons/InvitationActionButtons'; import ResendAllInvitationsButton from '../buttons/ResendAllInvitationsButton'; @@ -140,7 +140,9 @@ const UserInvitationsTable: FC = (props) => { ); const { t } = useTranslation(); - const permissions = useAppSelector(getManageCourseUserPermissions); + const { showPersonalizedTimelineFeatures } = useAppSelector( + getManageCourseUsersSharedData, + ); const columns: ColumnTemplate[] = [ { @@ -169,7 +171,7 @@ const UserInvitationsTable: FC = (props) => { title: t(tableTranslations.externalId), sortable: false, searchable: false, - cell: (datum) => datum.externalId ?? null, + cell: (datum) => datum.externalId ?? '', } satisfies ColumnTemplate, ] : []), @@ -186,7 +188,7 @@ const UserInvitationsTable: FC = (props) => { TIMELINE_ALGORITHMS.find( (timeline) => timeline.value === datum.timelineAlgorithm, )?.label ?? '-', - unless: !permissions.canManagePersonalTimes, + unless: !showPersonalizedTimelineFeatures, }, { id: 'status', diff --git a/client/app/bundles/course/user-invitations/components/tables/__test__/ExternalIdUpdateTable.test.tsx b/client/app/bundles/course/user-invitations/components/tables/__test__/ExternalIdUpdateTable.test.tsx new file mode 100644 index 00000000000..3f2cf024911 --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/__test__/ExternalIdUpdateTable.test.tsx @@ -0,0 +1,82 @@ +import { render, screen, waitForElementToBeRemoved } from 'test-utils'; +import { InvitationUpdatedItem } from 'types/course/userInvitations'; + +import ExternalIdUpdateTable from '../ExternalIdUpdateTable'; + +const baseRow: InvitationUpdatedItem = { + id: 1, + name: 'Alice Tan', + email: 'alice@example.com', + externalId: 'NEW001', + previousExternalId: 'OLD001', + role: 'student', + phantom: false, +}; + +const settle = async (): Promise => { + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); +}; + +describe('ExternalIdUpdateTable', () => { + it('renders nothing when rows is empty', async () => { + render(); + await settle(); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('shows Name, Email, both external IDs, Role and Phantom', async () => { + render(); + await settle(); + expect(screen.getByText('Alice Tan')).toBeInTheDocument(); + expect(screen.getByText('alice@example.com')).toBeInTheDocument(); + expect(screen.getByText('In file')).toBeInTheDocument(); + expect(screen.getByText('OLD001')).toBeInTheDocument(); + expect(screen.getByText('NEW001')).toBeInTheDocument(); + expect(screen.getByText('Student')).toBeInTheDocument(); + expect(screen.getByText('No')).toBeInTheDocument(); + }); + + it('marks Existing as the current column and fills its cells when not applied', async () => { + render(); + await settle(); + expect(screen.getByText('Existing (current)')).toBeInTheDocument(); + expect(screen.getByText('OLD001').closest('td')).toHaveClass('bg-blue-100'); + expect(screen.getByText('NEW001').closest('td')).toHaveClass( + 'text-neutral-400', + ); + }); + + it('moves the current marker and fill to In file when applied', async () => { + render(); + await settle(); + expect(screen.getByText('In file (current)')).toBeInTheDocument(); + expect(screen.getByText('NEW001').closest('td')).toHaveClass('bg-blue-100'); + expect(screen.getByText('OLD001').closest('td')).toHaveClass( + 'text-neutral-400', + ); + }); + + it('renders a null previousExternalId as an em dash in the Existing column', async () => { + render( + , + ); + await settle(); + expect(screen.getByText('—')).toBeInTheDocument(); + }); + + it('shows the Personalized Timeline column when the feature is on', async () => { + render( + , + ); + await settle(); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('Fomo')).toBeInTheDocument(); + }); +}); diff --git a/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultExistingTable.test.tsx b/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultExistingTable.test.tsx new file mode 100644 index 00000000000..f9accbe2620 --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultExistingTable.test.tsx @@ -0,0 +1,99 @@ +import { render, screen, waitForElementToBeRemoved } from 'test-utils'; + +import InvitationResultExistingTable from '../InvitationResultExistingTable'; + +const baseRow = { + id: 1, + name: 'Alice Tan', + email: 'alice@example.com', + externalId: 'aliceExt', + role: 'student', + phantom: false, +}; + +describe('InvitationResultExistingTable', () => { + it('renders nothing when rows is empty', async () => { + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('shows Name, Email, Ext ID, Role, Phantom columns', async () => { + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Alice Tan')).toBeInTheDocument(); + expect(screen.getByText('alice@example.com')).toBeInTheDocument(); + expect(screen.getByText('aliceExt')).toBeInTheDocument(); + expect(screen.getByText('Student')).toBeInTheDocument(); + expect(screen.getByText('No')).toBeInTheDocument(); + }); + + it('hides Ext ID column when no rows have externalId', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + expect(screen.getByText('Alice Tan')).toBeInTheDocument(); + expect(screen.getByText('Student')).toBeInTheDocument(); + }); + + it('localizes role via roleTranslations', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Teaching Assistant')).toBeInTheDocument(); + }); + + it('renders Yes for phantom user', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Yes')).toBeInTheDocument(); + }); + + describe('Personalized Timeline column', () => { + it('shows column header and algorithm label when showPersonalizedTimelineFeatures is true', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('Fomo')).toBeInTheDocument(); + }); + + it('hides column when showPersonalizedTimelineFeatures is false', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.queryByText('Personalized Timeline'), + ).not.toBeInTheDocument(); + }); + + it('shows dash when timelineAlgorithm is undefined', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('-')).toBeInTheDocument(); + }); + }); +}); diff --git a/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultFailedTable.test.tsx b/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultFailedTable.test.tsx new file mode 100644 index 00000000000..f28d3f31393 --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultFailedTable.test.tsx @@ -0,0 +1,228 @@ +import { render, screen, waitForElementToBeRemoved } from 'test-utils'; +import { FailedInvitationRowData } from 'types/course/userInvitations'; + +import InvitationResultFailedTable from '../InvitationResultFailedTable'; + +const baseUser: FailedInvitationRowData = { + id: 1, + name: 'Alice', + email: 'alice@example.com', + role: 'student', + reason: 'failed_to_send', +}; + +describe('InvitationResultFailedTable', () => { + it('renders nothing when users is empty', async () => { + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('renders name and email for each row', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Alice')).toBeInTheDocument(); + expect(screen.getByText('alice@example.com')).toBeInTheDocument(); + }); + + it('renders reason label for duplicate_email_in_file', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText('Duplicate email in uploaded CSV'), + ).toBeInTheDocument(); + }); + + it('renders reason label for external_id_taken', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText('External ID is taken by another course member'), + ).toBeInTheDocument(); + }); + + it('renders reason label for duplicate_external_id_in_file', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText('Duplicate external ID in uploaded CSV'), + ).toBeInTheDocument(); + }); + + it('renders reason label for failed_to_send', async () => { + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.getByText( + 'Failed to send invitation email, please try again - if failures persist, contact us for assistance', + ), + ).toBeInTheDocument(); + }); + + it('shows Role and Phantom columns', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Student')).toBeInTheDocument(); + expect(screen.getByText('No')).toBeInTheDocument(); + }); + + it('renders Yes for phantom user', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Yes')).toBeInTheDocument(); + }); + + it('shows External ID column when any user has a non-null externalId', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('External ID')).toBeInTheDocument(); + expect(screen.getByText('ext123')).toBeInTheDocument(); + }); + + it('renders empty cell for null externalId when column is shown by another row', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('External ID')).toBeInTheDocument(); + expect(screen.getByText('ext123')).toBeInTheDocument(); + expect(screen.getByText('Bob')).toBeInTheDocument(); + }); + + it('hides External ID column when all users have no externalId', async () => { + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + }); + + it('applies red highlight class to the failed_to_send row', async () => { + const { container } = render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + const rows = Array.from(container.querySelectorAll('tr')); + const aliceRow = rows.find((tr) => tr.textContent?.includes('Alice')); + const bobRow = rows.find((tr) => tr.textContent?.includes('Bob')); + expect(aliceRow?.className).toContain('bg-[#ffebee]'); + expect(bobRow?.className).not.toContain('bg-[#ffebee]'); + }); + + it('does not apply red highlight class to non-failed_to_send rows', async () => { + const { container } = render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + const highlighted = Array.from(container.querySelectorAll('tr')).find( + (tr) => tr.className.includes('bg-[#ffebee]'), + ); + expect(highlighted).toBeUndefined(); + }); + + describe('Personalized Timeline column', () => { + it('shows column header and algorithm label when showPersonalizedTimelineFeatures is true', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('Stragglers')).toBeInTheDocument(); + }); + + it('hides column when showPersonalizedTimelineFeatures is false', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.queryByText('Personalized Timeline'), + ).not.toBeInTheDocument(); + }); + + it('shows dash when timelineAlgorithm is undefined', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('-')).toBeInTheDocument(); + }); + }); +}); diff --git a/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultPrimaryTable.test.tsx b/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultPrimaryTable.test.tsx new file mode 100644 index 00000000000..f20af531f1f --- /dev/null +++ b/client/app/bundles/course/user-invitations/components/tables/__test__/InvitationResultPrimaryTable.test.tsx @@ -0,0 +1,112 @@ +import { render, screen, waitForElementToBeRemoved } from 'test-utils'; + +import InvitationResultPrimaryTable from '../InvitationResultPrimaryTable'; + +const baseRow = { + id: 'inv-1', + name: 'Alice', + email: 'alice@example.com', + externalId: null, + role: 'student', + phantom: false, +}; + +describe('InvitationResultPrimaryTable', () => { + it('renders Name, Email, Role, and Phantom columns', async () => { + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Alice')).toBeInTheDocument(); + expect(screen.getByText('alice@example.com')).toBeInTheDocument(); + expect(screen.getByText('Student')).toBeInTheDocument(); + expect(screen.getByText('No')).toBeInTheDocument(); + }); + + it('renders Yes for phantom user', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Yes')).toBeInTheDocument(); + }); + + it('localizes role via roleTranslations', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Teaching Assistant')).toBeInTheDocument(); + }); + + it('shows External ID column when any row has a non-null externalId', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('External ID')).toBeInTheDocument(); + expect(screen.getByText('ext123')).toBeInTheDocument(); + }); + + it('hides External ID column when all rows have null externalId', async () => { + render(); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + }); + + it('renders empty cell for null externalId when column is shown by another row', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('External ID')).toBeInTheDocument(); + expect(screen.getByText('ext123')).toBeInTheDocument(); + expect(screen.getByText('Bob')).toBeInTheDocument(); + }); + + describe('Personalized Timeline column', () => { + it('shows column header and algorithm label when showPersonalizedTimelineFeatures is true', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('Otot')).toBeInTheDocument(); + }); + + it('hides column when showPersonalizedTimelineFeatures is false', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect( + screen.queryByText('Personalized Timeline'), + ).not.toBeInTheDocument(); + }); + + it('shows dash when timelineAlgorithm is undefined', async () => { + render( + , + ); + await waitForElementToBeRemoved(() => screen.queryByRole('progressbar')); + expect(screen.getByText('Personalized Timeline')).toBeInTheDocument(); + expect(screen.getByText('-')).toBeInTheDocument(); + }); + }); +}); diff --git a/client/app/bundles/course/user-invitations/operations.ts b/client/app/bundles/course/user-invitations/operations.ts index d9a0c409114..59a95738bfb 100644 --- a/client/app/bundles/course/user-invitations/operations.ts +++ b/client/app/bundles/course/user-invitations/operations.ts @@ -1,5 +1,6 @@ import { Operation } from 'store'; import { + ExternalIdUpdate, InvitationFileEntity, InvitationPostData, InvitationResult, @@ -80,10 +81,15 @@ export function inviteUsersFromFile( CourseAPI.userInvitations.invite(fileEntity).then((response) => { const data = response.data; dispatch(actions.updateInvitationCounts(data.newInvitations)); - return JSON.parse(data.invitationResult); + return JSON.parse(data.invitationResult) as InvitationResult; }); } +export function updateExternalIds(updates: ExternalIdUpdate[]): Operation { + return async () => + CourseAPI.userInvitations.updateExternalIds(updates).then(() => undefined); +} + export function inviteUsersFromForm( postData: InvitationsPostData, ): Operation { @@ -92,7 +98,7 @@ export function inviteUsersFromForm( CourseAPI.userInvitations.invite(formattedData).then((response) => { const data = response.data; dispatch(actions.updateInvitationCounts(data.newInvitations)); - return JSON.parse(data.invitationResult); + return JSON.parse(data.invitationResult) as InvitationResult; }); } diff --git a/client/app/bundles/course/user-invitations/pages/InvitationsIndex/__test__/index.test.tsx b/client/app/bundles/course/user-invitations/pages/InvitationsIndex/__test__/index.test.tsx new file mode 100644 index 00000000000..5b601ebf4a6 --- /dev/null +++ b/client/app/bundles/course/user-invitations/pages/InvitationsIndex/__test__/index.test.tsx @@ -0,0 +1,80 @@ +import { createMockAdapter } from 'mocks/axiosMock'; +import { render, waitFor } from 'test-utils'; + +import CourseAPI from 'api/course'; + +import InvitationsIndex from '../index'; + +const mock = createMockAdapter(CourseAPI.userInvitations.client); + +const STUB_INVITATION = { + id: 1, + name: 'Test User', + email: 'test@example.com', + externalId: null, + role: 'student', + phantom: false, + timelineAlgorithm: 'fixed', + invitationKey: 'ABC123', + confirmed: false, + sentAt: '2024-01-01T00:00:00Z', + confirmedAt: null, + isRetryable: true, +}; + +const BASE_PERMISSIONS = { + canManageCourseUsers: true, + canManageEnrolRequests: true, + canManageReferenceTimelines: false, + canManagePersonalTimes: false, + canRegisterWithCode: false, +}; + +const BASE_MANAGE_COURSE_USERS_DATA = { + requestsCount: 0, + invitationsCount: 0, + defaultTimelineAlgorithm: 'fixed', + showPersonalizedTimelineFeatures: false, +}; + +const mockIndexResponse = ( + showPersonalizedTimelineFeatures: boolean, +): object => ({ + invitations: [STUB_INVITATION], + permissions: { + ...BASE_PERMISSIONS, + canManagePersonalTimes: showPersonalizedTimelineFeatures, + }, + manageCourseUsersData: { + ...BASE_MANAGE_COURSE_USERS_DATA, + showPersonalizedTimelineFeatures, + }, +}); + +describe('', () => { + it('shows the Personalized Timeline column when showPersonalizedTimelineFeatures is true', async () => { + mock + .onGet(`/courses/${global.courseId}/user_invitations`) + .reply(200, mockIndexResponse(true)); + + const page = render(); + + // Column only appears once invitations load. state.users is NOT pre-populated — + // the column must come from state.invitations (the correct store). + await waitFor(() => { + expect(page.queryByText('Personalized Timeline')).not.toBeNull(); + }); + }); + + it('hides the Personalized Timeline column when showPersonalizedTimelineFeatures is false', async () => { + mock + .onGet(`/courses/${global.courseId}/user_invitations`) + .reply(200, mockIndexResponse(false)); + + const page = render(); + + await waitFor(() => { + expect(page.queryByText('Personalized Timeline')).toBeNull(); + }); + }); +}); diff --git a/client/app/bundles/course/user-invitations/pages/InviteUsersFileUpload/__test__/index.test.tsx b/client/app/bundles/course/user-invitations/pages/InviteUsersFileUpload/__test__/index.test.tsx new file mode 100644 index 00000000000..4dedfc45ee7 --- /dev/null +++ b/client/app/bundles/course/user-invitations/pages/InviteUsersFileUpload/__test__/index.test.tsx @@ -0,0 +1,97 @@ +import { render, screen, waitFor } from 'test-utils'; +import { InvitationFileEntity } from 'types/course/userInvitations'; + +import InviteUsersFileUpload from '../index'; + +// Capture the onSubmit prop so we can call it directly with IFormInputs-shaped data, +// reproducing exactly what FormDialog passes at runtime. +let capturedOnSubmit: + | ((data: { file: InvitationFileEntity }) => Promise) + | null = null; + +// jest.mock is hoisted before variable declarations, so MockFileUploadForm must be +// defined inside the factory (not as a module-level variable referenced in the factory). +jest.mock('../../../components/forms/InviteUsersFileUploadForm', () => { + const MockFileUploadForm = ({ + open, + onSubmit, + }: { + open: boolean; + onSubmit: (data: { file: InvitationFileEntity }) => Promise; + }): JSX.Element | null => { + capturedOnSubmit = onSubmit; + return open ? ( +