Skip to content

Commit d6e1808

Browse files
committed
feat(users): add optional external_id field to course members and invitations
Allows institutions to link Coursemology enrolments to their own student records or LMS identifiers. The field is stored on CourseUser and Course::UserInvitation and validated unique per course across both tables via a cross-table concern and partial DB index - a pending invitation holds its ext_id until confirmed, preventing collisions with direct enrolments. Surfaces: - Individual invite form: ext_id input field - Bulk CSV invite: ext_id column in both template variants (with and without Timeline column); set on new records only - existing pending invitations and enrolled users are read-only in this flow - Manage users table: inline editable ext_id column (always visible) - Score summary export: ext_id column included when any student has one - Statistics > Students table: ext_id column, sortable and searchable, shown only when at least one student has a non-null ext_id View-only tables suppress the ext_id column when no course members have one set, consistent with how group manager, gamification, and video columns are conditionally shown. Edit tables always show it. Also changes error responses from the invitations controller from a single concatenated string to an array of per-record strings, enabling the frontend to render overflow counts without truncating meaningful error detail.
1 parent 958e986 commit d6e1808

69 files changed

Lines changed: 1833 additions & 271 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

app/controllers/concerns/course/users_controller_management_concern.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ def should_update_personalized_timeline
123123

124124
def course_user_params
125125
@course_user_params ||= params.require(:course_user).permit(
126-
:user_id, :name, :timeline_algorithm, :role, :phantom, :reference_timeline_id
126+
:user_id, :name, :timeline_algorithm, :role, :phantom, :reference_timeline_id, :external_id
127127
)
128128
end
129129

app/controllers/course/user_invitations_controller.rb

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ def create
1919
create_invitation_success(result)
2020
else
2121
propagate_errors
22-
render json: { errors: current_course.errors.full_messages.to_sentence }, status: :bad_request
22+
errors = current_course.errors[:base]
23+
render json: { errors: errors }, status: :bad_request
2324
end
2425
end
2526

@@ -59,7 +60,8 @@ def course_user_invitation_params
5960
@course_user_invitation_params ||= begin
6061
params[:course] = { invitations_attributes: {} } unless params.key?(:course)
6162
params.require(:course).permit(:invitations_file, :registration_key,
62-
invitations_attributes: [:name, :email, :role, :phantom, :timeline_algorithm])
63+
invitations_attributes: [:name, :email, :role, :phantom,
64+
:timeline_algorithm, :external_id])
6365
end
6466
end
6567

@@ -120,7 +122,7 @@ def invite_by_file?
120122
def invite
121123
invitation_service.invite(invitation_params)
122124
rescue CSV::MalformedCSVError => e
123-
current_course.errors.add(:invitations_file, e.message)
125+
current_course.errors.add(:base, e.message)
124126
false
125127
end
126128

@@ -135,15 +137,7 @@ def invitation_service
135137
#
136138
# @return [void]
137139
def propagate_errors
138-
propagate_errors_to_file if invite_by_file?
139-
end
140-
141-
# Propagates errors from the generated records to the file.
142-
#
143-
# @return [void]
144-
def propagate_errors_to_file
145-
errors = aggregate_errors
146-
current_course.errors.add(:invitations_file, errors.to_sentence) unless errors.empty?
140+
aggregate_errors.each { |msg| current_course.errors.add(:base, msg) }
147141
end
148142

149143
# Aggregates errors from all the known sources of failure.
@@ -157,9 +151,24 @@ def aggregate_errors
157151
#
158152
# @return [Array<String>]
159153
def invalid_course_user_errors
160-
invalid_course_users.map do |course_user|
161-
user = self.class.helpers.display_course_user(course_user)
162-
t('errors.course.user_invitations.duplicate_user', user: user)
154+
invalid_course_users.flat_map do |course_user|
155+
email = course_user.user&.email || course_user.name
156+
errors = []
157+
if course_user.errors.of_kind?(:external_id, :taken)
158+
errors << t('errors.course.user_invitations.duplicate_external_id',
159+
email: email, external_id: course_user.external_id)
160+
end
161+
if course_user.errors.of_kind?(:user, :taken) || course_user.errors.of_kind?(:course, :taken)
162+
errors << t('errors.course.user_invitations.already_enrolled', email: email)
163+
end
164+
other_errors = course_user.errors.reject { |e| %w[external_id user course].include?(e.attribute.to_s) }
165+
# .any? is intentional: this is a catch-all for unrecognised errors; all messages are forwarded verbatim
166+
if other_errors.any?
167+
message = other_errors.map { |e| course_user.errors.full_message(e.attribute, e.message) }.
168+
to_sentence
169+
errors << t('errors.course.user_invitations.other_error', email: email, message: message)
170+
end
171+
errors
163172
end
164173
end
165174

@@ -174,9 +183,29 @@ def invalid_course_users
174183
#
175184
# @return [Array<String>]
176185
def invalid_invitation_email_errors
177-
invalid_invitations.map do |invitation|
178-
message = invitation.errors.full_messages.to_sentence
179-
t('errors.course.user_invitations.invalid_email', email: invitation.email, message: message)
186+
invalid_invitations.flat_map do |invitation|
187+
email = invitation.email
188+
errors = []
189+
if invitation.errors.of_kind?(:external_id, :taken)
190+
errors << t('errors.course.user_invitations.duplicate_external_id',
191+
email: email, external_id: invitation.external_id)
192+
end
193+
# .any? is intentional: invalid_email is a broad wrapper; the actual messages are forwarded in `message`
194+
if invitation.errors[:email].any?
195+
message = invitation.errors[:email].to_sentence
196+
errors << t('errors.course.user_invitations.invalid_email', email: email, message: message)
197+
end
198+
if invitation.errors.of_kind?(:base, :existing_invitation)
199+
errors << t('errors.course.user_invitations.duplicate_invitation', email: email)
200+
end
201+
other_errors = invitation.errors.reject { |e| %w[external_id email base].include?(e.attribute.to_s) }
202+
# .any? is intentional: this is a catch-all for unrecognised errors; all messages are forwarded verbatim
203+
if other_errors.any?
204+
message = other_errors.map { |e| invitation.errors.full_message(e.attribute, e.message) }.
205+
to_sentence
206+
errors << t('errors.course.user_invitations.other_error', email: email, message: message)
207+
end
208+
errors
180209
end
181210
end
182211

@@ -254,7 +283,7 @@ def destroy_invitation_success
254283

255284
def destroy_invitation_failure
256285
respond_to do |format|
257-
format.json { render json: { errors: @invitation.errors.full_messages.to_sentence }, status: :bad_request }
286+
format.json { render json: { errors: @invitation.errors.full_messages }, status: :bad_request }
258287
end
259288
end
260289

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# frozen_string_literal: true
2+
# This concern validates that external IDs are unique within a course,
3+
# across both course users and pending invitations.
4+
#
5+
# Nil and blank external IDs are allowed.
6+
module Course::UniqueExternalIdConcern
7+
extend ActiveSupport::Concern
8+
9+
included do
10+
before_validation :normalize_external_id
11+
12+
validate :validate_unique_external_id_within_course, if: -> { new_record? || external_id_changed? }
13+
end
14+
15+
private
16+
17+
# Normalizes blank external IDs to nil.
18+
#
19+
# @return [void]
20+
def normalize_external_id
21+
self.external_id = nil if external_id.blank?
22+
end
23+
24+
# Validates that the external ID is unique within the course,
25+
# across both course users and invitations.
26+
#
27+
# @return [void]
28+
def validate_unique_external_id_within_course
29+
return if external_id.blank?
30+
31+
invitation_exists = Course::UserInvitation.
32+
unconfirmed.
33+
where(course_id: course_id, external_id: external_id).
34+
where.not(id: id).
35+
exists?
36+
37+
course_user_exists = CourseUser.
38+
where(course_id: course_id, external_id: external_id).
39+
where.not(id: id).
40+
exists?
41+
42+
return unless invitation_exists || course_user_exists
43+
44+
errors.add(:external_id, :taken)
45+
end
46+
end

app/models/course/user_invitation.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ class Course::UserInvitation < ApplicationRecord
44
after_initialize :set_defaults, if: :new_record?
55
before_validation :set_defaults, if: :new_record?
66

7+
include Course::UniqueExternalIdConcern
8+
79
validates :email, format: { with: Devise.email_regexp }, if: :email_changed?
810
validates :name, presence: true
911
validates :role, presence: true

app/models/course_user.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ class CourseUser < ApplicationRecord
33
include CourseUser::StaffConcern
44
include CourseUser::LevelProgressConcern
55
include CourseUser::TodoConcern
6+
include Course::UniqueExternalIdConcern
67

78
after_initialize :set_defaults, if: :new_record?
89
before_validation :set_defaults, if: :new_record?

app/models/user.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ def build_course_user_from_invitation(invitation)
129129
phantom: invitation.phantom,
130130
timeline_algorithm: invitation.timeline_algorithm ||
131131
invitation.course&.default_timeline_algorithm,
132+
external_id: invitation.external_id,
132133
creator: self,
133134
updater: self)
134135
end

app/models/user/email.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,14 @@ def accept_all_pending_invitations
3535
unconfirmed_invitation.confirm!(confirmer: user)
3636
next
3737
end
38-
user.build_course_user_from_invitation(unconfirmed_invitation)
39-
unconfirmed_invitation.confirm!(confirmer: user) if user.save && user.persisted?
38+
# Confirm the invitation before saving the CourseUser so that the
39+
# UniqueExternalIdConcern validation doesn't reject the new CourseUser
40+
# for sharing an external_id with what is now a confirmed invitation.
41+
CourseUser.transaction(requires_new: true) do
42+
unconfirmed_invitation.confirm!(confirmer: user)
43+
user.build_course_user_from_invitation(unconfirmed_invitation)
44+
raise ActiveRecord::Rollback unless user.save && user.persisted?
45+
end
4046
end
4147
end
4248
end

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

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,23 @@ def parse_invitations(users)
4747
# ]
4848
def partition_unique_users(users)
4949
users.each { |user| user[:email] = user[:email].downcase }
50-
unique_users = {}
50+
seen_emails = Set.new
51+
seen_external_ids = Set.new
52+
unique_users = []
5153
duplicate_users = []
5254
users.each do |user|
53-
if unique_users.key?(user[:email])
54-
duplicate_users.push(user)
55+
ext_id = user[:external_id].presence
56+
if seen_emails.include?(user[:email])
57+
duplicate_users.push(user.merge(reason: :duplicate_email_in_file))
58+
elsif ext_id && seen_external_ids.include?(ext_id)
59+
duplicate_users.push(user.merge(reason: :duplicate_external_id_in_file))
5560
else
56-
unique_users[user[:email]] = user
61+
seen_emails.add(user[:email])
62+
seen_external_ids.add(ext_id) if ext_id
63+
unique_users << user
5764
end
5865
end
59-
[unique_users.values, duplicate_users]
66+
[unique_users, duplicate_users]
6067
end
6168

6269
# Change all invitees' roles to :student if inviter is a teaching_assistant.
@@ -86,7 +93,8 @@ def parse_from_form(users)
8693
email: value[:email],
8794
role: value[:role],
8895
phantom: phantom,
89-
timeline_algorithm: value[:timeline_algorithm] }
96+
timeline_algorithm: value[:timeline_algorithm],
97+
external_id: value[:external_id] }
9098
end
9199
end
92100

@@ -144,11 +152,17 @@ def parse_file_row(row)
144152
return nil if row[1].blank?
145153

146154
row[0] = row[1] if row[0].blank?
147-
148155
role = parse_file_role(row[2])
149156
phantom = parse_file_phantom(row[3])
150-
timeline_algorithm = parse_file_timeline_algorithm(row[4])
151-
{ name: row[0], email: row[1], role: role, phantom: phantom, timeline_algorithm: timeline_algorithm }
157+
if @current_course.show_personalized_timeline_features?
158+
timeline_algorithm = parse_file_timeline_algorithm(row[4])
159+
external_id = parse_file_external_id(row[5])
160+
else
161+
external_id = parse_file_external_id(row[4])
162+
timeline_algorithm = parse_file_timeline_algorithm(nil)
163+
end
164+
{ name: row[0], email: row[1], role: role, phantom: phantom,
165+
timeline_algorithm: timeline_algorithm, external_id: external_id }
152166
end
153167

154168
# Parses the role column from the CSV file.
@@ -188,6 +202,17 @@ def parse_file_timeline_algorithm(timeline_algorithm)
188202
symbol || @current_course.default_timeline_algorithm
189203
end
190204

205+
# Parses file value for an invitation's external ID.
206+
# Returns nil if value is not specified.
207+
#
208+
# @param [String|nil] external_id External ID as specified in the CSV file.
209+
# @return [String|nil] The external ID string, or nil if blank.
210+
def parse_file_external_id(external_id)
211+
return nil if external_id.blank?
212+
213+
external_id
214+
end
215+
191216
# Removes the UTF-8 byte order mark (BOM) from the string.
192217
# The BOM exists at the start of in CSVs (optionally) to indicate the
193218
# encoding of the file.

0 commit comments

Comments
 (0)