Skip to content

Commit 395b96d

Browse files
fix(api): fix order dependent learner reconciliation
1 parent 3bf94ab commit 395b96d

3 files changed

Lines changed: 58 additions & 1 deletion

File tree

api/src/prescription/learner-management/infrastructure/repositories/organization-learner-repository.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,17 @@ const _reconcileOrganizationLearners = async function (studentsToImport, allOrga
104104
const organizationLearnersWithSameNationalStudentIdsAsImported =
105105
await studentRepository.findReconciledStudentsByNationalStudentId(nationalStudentIdsFromFile);
106106

107+
const userIdsWithMultipleNationalStudentIds = new Set(
108+
organizationLearnersWithSameNationalStudentIdsAsImported
109+
.filter((learner) =>
110+
organizationLearnersWithSameNationalStudentIdsAsImported.some(
111+
(other) =>
112+
other.account.userId === learner.account.userId && other.nationalStudentId !== learner.nationalStudentId,
113+
),
114+
)
115+
.map((learner) => learner.account.userId),
116+
);
117+
107118
organizationLearnersWithSameNationalStudentIdsAsImported.forEach((organizationLearner) => {
108119
const alreadyReconciledStudentToImport = studentsToImport.find(
109120
(studentToImport) => studentToImport.userId === organizationLearner.account.userId,
@@ -114,6 +125,10 @@ const _reconcileOrganizationLearners = async function (studentsToImport, allOrga
114125
return;
115126
}
116127

128+
if (userIdsWithMultipleNationalStudentIds.has(organizationLearner.account.userId)) {
129+
return;
130+
}
131+
117132
const studentToImport = studentsToImport.find(
118133
(studentToImport) => studentToImport.nationalStudentId === organizationLearner.nationalStudentId,
119134
);

api/tests/prescription/learner-management/integration/infrastructure/repositories/organization-learner-repository_test.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,48 @@ describe('Integration | Repository | Organization Learner Management | Organizat
826826
expect(expected1.userId).to.be.null;
827827
expect(expected2.userId).to.be.null;
828828
});
829+
830+
it('should save both organization learners with userId as null when one is already reconciled in the target organization', async function () {
831+
// - Org A already has INE1 reconciled with userId (existing reconciliation)
832+
// - Org B has INE2 reconciled with the same userId
833+
// - Importing both INE1 and INE2 into Org A should remove reconciliation from both
834+
const { id: orgAId } = databaseBuilder.factory.buildOrganization();
835+
const { id: orgBId } = databaseBuilder.factory.buildOrganization();
836+
const { id: userId } = databaseBuilder.factory.buildUser();
837+
databaseBuilder.factory.buildOrganizationLearner({
838+
nationalStudentId: 'INE2',
839+
organizationId: orgBId,
840+
userId,
841+
});
842+
databaseBuilder.factory.buildOrganizationLearner({
843+
nationalStudentId: 'INE1',
844+
organizationId: orgAId,
845+
userId,
846+
});
847+
await databaseBuilder.commit();
848+
849+
// when
850+
const organizationLearner1 = domainBuilder.buildOrganizationLearner({ nationalStudentId: 'INE1' });
851+
const organizationLearner2 = domainBuilder.buildOrganizationLearner({ nationalStudentId: 'INE2' });
852+
await DomainTransaction.execute((domainTransaction) => {
853+
return addOrUpdateOrganizationOfOrganizationLearners(
854+
[organizationLearner1, organizationLearner2],
855+
orgAId,
856+
domainTransaction,
857+
);
858+
});
859+
860+
// then
861+
const expected1 = await knex('organization-learners')
862+
.where({ nationalStudentId: 'INE1', organizationId: orgAId })
863+
.first();
864+
const expected2 = await knex('organization-learners')
865+
.where({ nationalStudentId: 'INE2', organizationId: orgAId })
866+
.first();
867+
868+
expect(expected1.userId).to.be.null;
869+
expect(expected2.userId).to.be.null;
870+
});
829871
});
830872

831873
context('when there are organizationLearners in another organization', function () {

high-level-tests/e2e-playwright/tests/pix-orga/sco-students-management.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ test.describe('SCO import does not auto-reconcile a learner when the user alread
399399
);
400400
});
401401

402-
test("Remove user reconciliation when the user is already in the target organization with a different INE, (is it ok, i don't know, but it's here since the beginning of the time)", async ({
402+
test("Remove user reconciliation when the user is already in the target organization with a different INE, (is it ok, i don't know, but it's here since the beginning of the time)", async ({
403403
page,
404404
}) => {
405405
test.slow();

0 commit comments

Comments
 (0)