fix(users): unblock sign up when the invitation carries an external ID

app/models/concerns/course/unique_external_id_concern.rb

@@ -7,6 +7,12 @@ module Course::UniqueExternalIdConcern
   extend ActiveSupport::Concern
 
   included do
+    # The invitation this record is being created from, if any. A record inheriting its external ID
+    # from the invitation that created it is not a conflict, so that invitation is excluded from the
+    # uniqueness check. Callers that set this must confirm or destroy the invitation in the same
+    # transaction, otherwise the two records are left sharing an external ID.
+    attr_accessor :source_invitation
+
     before_validation :normalize_external_id
 
     validate :validate_unique_external_id_within_course, if: -> { new_record? || external_id_changed? }
@@ -35,6 +41,7 @@ def validate_unique_external_id_within_course
   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 = query.where.not(id: source_invitation.id) if source_invitation&.persisted?
     query.exists?
   end
 

app/models/user.rb

@@ -166,6 +166,7 @@ def build_course_user_from_invitation(invitation)
                        timeline_algorithm: invitation.timeline_algorithm ||
                           invitation.course&.default_timeline_algorithm,
                        external_id: invitation.external_id.presence,
+                       source_invitation: invitation,
                        creator: self,
                        updater: self)
   end

spec/controllers/user/registration_controller_spec.rb

@@ -154,6 +154,51 @@
           end
         end
       end
+
+      context 'when signing up through a course invitation' do
+        requires_login
+
+        let(:course) { create(:course) }
+        let(:email) { generate(:email) }
+        let!(:invitation) do
+          create(:course_user_invitation, course: course, email: email, external_id: external_id)
+        end
+
+        before { allow(controller).to receive(:verify_recaptcha).and_return(true) }
+
+        subject do
+          post :create, params: {
+            invitation: invitation.invitation_key,
+            user: { name: 'New Student', email: email,
+                    password: 'lolololol', password_confirmation: 'lolololol' }
+          }
+        end
+
+        shared_examples 'a successful enrolment' do
+          it 'creates the user, enrols them once and confirms the invitation' do
+            expect { subject }.to change(User, :count).by(1).and change(CourseUser, :count).by(1)
+
+            new_user = User.order(:created_at).last
+            course_user = CourseUser.find_by(course: course, user: new_user)
+            expect(course_user).to be_present
+            expect(course_user.external_id).to eq(external_id)
+            expect(invitation.reload).to be_confirmed
+            expect(invitation.confirmer).to eq(new_user)
+          end
+        end
+
+        context 'when the invitation has no external_id' do
+          let(:external_id) { nil }
+
+          it_behaves_like 'a successful enrolment'
+        end
+
+        context 'when the invitation has an external_id' do
+          let(:external_id) { 'A0123456X' }
+
+          it_behaves_like 'a successful enrolment'
+        end
+      end
     end
   end
 end

spec/models/course_user_spec.rb

@@ -137,6 +137,36 @@
           expect(student).to be_valid
         end
       end
+
+      context 'when the course user is created from an invitation' do
+        let!(:invitation) { create(:course_user_invitation, course: course, external_id: 'invited-id') }
+
+        # The invitation is confirmed in the same transaction that saves the course user, so a course
+        # user inheriting its external_id is not a conflict with the invitation it came from.
+        it 'is valid despite the still-unconfirmed invitation holding the same external_id' do
+          student = build(:course_student, course: course, external_id: 'invited-id',
+                                           source_invitation: invitation)
+          expect(student).to be_valid
+        end
+
+        it 'is still invalid when a different pending invitation holds the external_id' do
+          other_invitation = create(:course_user_invitation, course: course, external_id: 'other-id')
+          student = build(:course_student, course: course, external_id: other_invitation.external_id,
+                                           source_invitation: invitation)
+          expect(student).not_to be_valid
+          expect(student.errors[:external_id]).
+            to include(I18n.t('activerecord.errors.models.course_user.attributes.external_id.taken'))
+        end
+
+        it 'is still invalid when an enrolled course user holds the external_id' do
+          existing = create(:course_student, course: course, external_id: 'enrolled-id')
+          student = build(:course_student, course: course, external_id: existing.external_id,
+                                           source_invitation: invitation)
+          expect(student).not_to be_valid
+          expect(student.errors[:external_id]).
+            to include(I18n.t('activerecord.errors.models.course_user.attributes.external_id.taken'))
+        end
+      end
     end
 
     describe '.staff' do