Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TECH] Permettre de supprimer une liste de learners de l'orga (PIX-15244) #10521

Merged
merged 4 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ async function deleteOrganizationLearnersFromOrganization(organizationId, date)
await usecases.deleteOrganizationLearners({
organizationLearnerIds: organizationLearnerToDeleteIds,
userId: engineeringUserId,
organizationId,
});

await _anonymizeOrganizationLearners({ organizationId });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { HttpErrors } from '../../../shared/application/http-errors.js';
import { AggregateImportError } from '../domain/errors.js';
import { AggregateImportError, CouldNotDeleteLearnersError } from '../domain/errors.js';

const learnerManagementDomainErrorMappingConfiguration = [
{
Expand All @@ -8,6 +8,12 @@ const learnerManagementDomainErrorMappingConfiguration = [
return new HttpErrors.PreconditionFailedError(error.message, error.code, error.meta);
},
},
{
name: CouldNotDeleteLearnersError.name,
httpErrorFn: (error) => {
return new HttpErrors.PreconditionFailedError(error.message);
},
},
];

export { learnerManagementDomainErrorMappingConfiguration };
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import { usecases } from '../domain/usecases/index.js';
const deleteOrganizationLearners = async function (request, h) {
const authenticatedUserId = request.auth.credentials.userId;
const listLearners = request.payload.listLearners;
const organizationId = request.params.organizationId;

await DomainTransaction.execute(async () => {
await usecases.deleteOrganizationLearners({
organizationLearnerIds: listLearners,
userId: authenticatedUserId,
organizationId,
});
});
return h.response().code(200);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const register = async (server) => {
server.route([
{
method: 'DELETE',
path: '/api/organizations/{id}/organization-learners',
path: '/api/organizations/{organizationId}/organization-learners',
config: {
pre: [
{
Expand All @@ -27,7 +27,7 @@ const register = async (server) => {
],
validate: {
params: Joi.object({
id: identifiersType.organizationId,
organizationId: identifiersType.organizationId,
}),
payload: Joi.object({
listLearners: Joi.array().required().items(Joi.number().required()),
Expand Down
7 changes: 7 additions & 0 deletions api/src/prescription/learner-management/domain/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,15 @@ class ReconcileCommonOrganizationLearnerError extends DomainError {
}
}

class CouldNotDeleteLearnersError extends DomainError {
constructor() {
super(`Could not delete the following organization learners.`);
}
}

export {
AggregateImportError,
CouldNotDeleteLearnersError,
OrganizationDoesNotHaveFeatureEnabledError,
OrganizationLearnerImportFormatNotFoundError,
OrganizationLearnersCouldNotBeSavedError,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { logger } from '../../../../shared/infrastructure/utils/logger.js';
import { CouldNotDeleteLearnersError } from '../errors.js';

class OrganizationLearnerList {
constructor({ organizationId, organizationLearnerIds } = {}) {
this.organizationId = organizationId;
this.organizationLearnerIds = organizationLearnerIds;
}
canDeleteOrganizationLearners(organizationLearnerIdsToValidate, userId) {
const result = organizationLearnerIdsToValidate.filter((organizationLearnerId) => {
return !this.organizationLearnerIds.includes(organizationLearnerId);
});
if (result.length !== 0) {
logger.error(
`User id ${userId} could not delete organization learners because learner id ${result.join(',')} don't belong to organization id ${this.organizationId} "`,
);
throw new CouldNotDeleteLearnersError();
}
}
}

export { OrganizationLearnerList };
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
import { OrganizationLearnerList } from '../models/OrganizationLearnerList.js';

const deleteOrganizationLearners = async function ({
organizationLearnerIds,
userId,
organizationId,
organizationLearnerRepository,
campaignParticipationRepository,
}) {
const organizationLearnerIdsFromOrganization =
await organizationLearnerRepository.findOrganizationLearnerIdsByOrganizationId({
organizationId,
});

const organizationLearnerList = new OrganizationLearnerList({
organizationId,
organizationLearnerIds: organizationLearnerIdsFromOrganization,
});

organizationLearnerList.canDeleteOrganizationLearners(organizationLearnerIds, userId);
await campaignParticipationRepository.removeByOrganizationLearnerIds({
organizationLearnerIds,
userId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ const findByUserId = async function ({ userId }) {
return rawOrganizationLearners.map((rawOrganizationLearner) => new OrganizationLearner(rawOrganizationLearner));
};

const findOrganizationLearnerIdsByOrganizationId = function ({ organizationId }) {
const knexConnection = DomainTransaction.getConnection();
return knexConnection('view-active-organization-learners').where({ organizationId }).select('id').pluck('id');
};

export {
addOrUpdateOrganizationOfOrganizationLearners,
disableAllOrganizationLearnersInOrganization,
Expand All @@ -225,6 +230,7 @@ export {
findAllCommonLearnersFromOrganizationId,
findAllCommonOrganizationLearnerByReconciliationInfos,
findByUserId,
findOrganizationLearnerIdsByOrganizationId,
getOrganizationLearnerForAdmin,
reconcileUserByNationalStudentIdAndOrganizationId,
removeByIds,
Expand Down
2 changes: 1 addition & 1 deletion api/src/shared/application/security-pre-handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ async function checkUserDoesNotBelongsToScoOrganizationManagingStudents(
return _replyForbiddenError(h);
}

const organizationId = request.params.id;
const organizationId = request.params.organizationId || request.params.id;

const isOrganizationScoManagingStudent = await dependencies.checkOrganizationIsScoAndManagingStudentUsecase.execute({
organizationId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '../../../../test-helper.js';

describe('Integration | Application | Organization Learners Management | Routes', function () {
describe('DELETE /organizations/{id}/organization-learners', function () {
describe('DELETE /organizations/{organizationId}/organization-learners', function () {
const method = 'DELETE';

let headers, httpTestServer, organizationId, url, payload;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
disableCommonOrganizationLearnersFromOrganizationId,
findAllCommonLearnersFromOrganizationId,
findAllCommonOrganizationLearnerByReconciliationInfos,
findOrganizationLearnerIdsByOrganizationId,
getOrganizationLearnerForAdmin,
reconcileUserByNationalStudentIdAndOrganizationId,
removeByIds,
Expand Down Expand Up @@ -1708,4 +1709,43 @@ describe('Integration | Repository | Organization Learner Management | Organizat
]);
});
});

describe('#findOrganizationLearnerIdsByOrganizationId', function () {
let myOrganizationId;
let otherOrganizationId;
let organizationLearnerId;

beforeEach(async function () {
myOrganizationId = databaseBuilder.factory.buildOrganization().id;
otherOrganizationId = databaseBuilder.factory.buildOrganization().id;
organizationLearnerId = databaseBuilder.factory.buildOrganizationLearner({
organizationId: myOrganizationId,
}).id;
databaseBuilder.factory.buildOrganizationLearner({
organizationId: otherOrganizationId,
}).id;
await databaseBuilder.commit();
});

it('should return one learner', async function () {
const results = await findOrganizationLearnerIdsByOrganizationId({
organizationId: myOrganizationId,
});
expect(results).to.deep.equal([organizationLearnerId]);
});

it('should not return deleted learner', async function () {
databaseBuilder.factory.buildOrganizationLearner({
organizationId: myOrganizationId,
deletedAt: new Date('2020-04-05'),
}).id;

await databaseBuilder.commit();

const results = await findOrganizationLearnerIdsByOrganizationId({
organizationId: myOrganizationId,
});
expect(results).to.deep.equal([organizationLearnerId]);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { CouldNotDeleteLearnersError } from '../../../../../../src/prescription/learner-management/domain/errors.js';
import { OrganizationLearnerList } from '../../../../../../src/prescription/learner-management/domain/models/OrganizationLearnerList.js';
import { logger } from '../../../../../../src/shared/infrastructure/utils/logger.js';
import { catchErrSync, expect, sinon } from '../../../../../test-helper.js';

describe('Unit | Models | OrganizationLearnerListFormat', function () {
describe('#constructor', function () {
it('should initialize valid object', function () {
//when
const payload = {
organizationId: Symbol('organizationId'),
organizationLearnerIds: Symbol('organizationLearnerList'),
};
const organizationLearnerList = new OrganizationLearnerList(payload);
// then
expect(organizationLearnerList).to.deep.equal(payload);
});
});

describe('#can delete organization learners ', function () {
it('should throw when lists are different', function () {
sinon.stub(logger, 'error');
//when
const payload = {
organizationId: 777,
organizationLearnerIds: [123, 345],
};

const organizationLearnerList = new OrganizationLearnerList(payload);

const result = catchErrSync(organizationLearnerList.canDeleteOrganizationLearners, organizationLearnerList)(
[456, 123],
'userIdSample',
);

expect(result).to.be.instanceof(CouldNotDeleteLearnersError);
expect(
logger.error.calledWithExactly(
`User id userIdSample could not delete organization learners because learner id 345 don't belong to organization id 777`,
),
);
});

it('should not throw when lists are identical', function () {
const userId = Symbol('123');

const payload = {
organizationId: Symbol('organizationId'),
organizationLearnerIds: [123, 345],
};

expect(() => {
const organizationLearnerList = new OrganizationLearnerList(payload);
organizationLearnerList.canDeleteOrganizationLearners([123, 345], userId);
}).to.not.throw();
});
});
});
Original file line number Diff line number Diff line change
@@ -1,32 +1,49 @@
import { OrganizationLearnerList } from '../../../../../../src/prescription/learner-management/domain/models/OrganizationLearnerList.js';
import { deleteOrganizationLearners } from '../../../../../../src/prescription/learner-management/domain/usecases/delete-organization-learners.js';
import { expect, sinon } from '../../../../../test-helper.js';
import { catchErr, expect, sinon } from '../../../../../test-helper.js';

describe('Unit | UseCase | Organization Learners Management | Delete Organization Learners', function () {
let campaignParticipationRepository;
let organizationLearnerRepository;
let organizationLearnerIds;
let organizationId;
let userId;
let canDeleteStub;

beforeEach(function () {
userId = 777;
organizationId = 123;
organizationLearnerIds = [123, 456, 789];
canDeleteStub = sinon.stub(OrganizationLearnerList.prototype, 'canDeleteOrganizationLearners');
campaignParticipationRepository = {
removeByOrganizationLearnerIds: sinon.stub(),
};
organizationLearnerRepository = {
removeByIds: sinon.stub(),
findOrganizationLearnerIdsByOrganizationId: sinon.stub().returns(organizationLearnerIds),
};
organizationLearnerRepository.findOrganizationLearnerIdsByOrganizationId.resolves(organizationLearnerIds);
});

it('should delete organization learners and their participations', async function () {
it('should delete organization learners and their participations when all learners belong to organization', async function () {
// given
const userId = 777;
const organizationLearnerIds = [123, 456, 789];
canDeleteStub.withArgs(organizationLearnerIds);

// when
await deleteOrganizationLearners({
organizationLearnerIds,
userId,
organizationId,
campaignParticipationRepository,
organizationLearnerRepository,
});

expect(canDeleteStub).to.have.been.calledWith(organizationLearnerIds, userId);

expect(organizationLearnerRepository.findOrganizationLearnerIdsByOrganizationId).to.have.been.calledWithExactly({
organizationId,
});

// then
expect(campaignParticipationRepository.removeByOrganizationLearnerIds).to.have.been.calledWithExactly({
organizationLearnerIds,
Expand All @@ -38,4 +55,29 @@ describe('Unit | UseCase | Organization Learners Management | Delete Organizatio
userId,
});
});

it('should not delete organization learners and their participations when all learners do not belong to organization', async function () {
// given
const organizationLearnerIdsPayload = [123, 456, 789, 101];
canDeleteStub.withArgs(organizationLearnerIdsPayload).throws();

// when
await catchErr(deleteOrganizationLearners)({
organizationLearnerIds: organizationLearnerIdsPayload,
userId,
organizationId,
campaignParticipationRepository,
organizationLearnerRepository,
});

expect(canDeleteStub).to.have.been.calledWith(organizationLearnerIdsPayload, userId);

expect(organizationLearnerRepository.findOrganizationLearnerIdsByOrganizationId).to.have.been.calledWithExactly({
organizationId,
});

expect(campaignParticipationRepository.removeByOrganizationLearnerIds).to.not.have.been.called;

expect(organizationLearnerRepository.removeByIds).to.not.have.been.called;
});
});