Skip to content

Commit 66f35aa

Browse files
authored
Merge branch 'main' into BC-9204
2 parents 31ea8c2 + 20e3e29 commit 66f35aa

File tree

5 files changed

+96
-13
lines changed

5 files changed

+96
-13
lines changed

apps/server/src/modules/provisioning/service/tsp-provisioning.service.spec.ts

+28
Original file line numberDiff line numberDiff line change
@@ -610,5 +610,33 @@ describe('TspProvisioningService', () => {
610610
await expect(() => sut.provisionUser(data, school)).rejects.toThrow(BadDataLoggableException);
611611
});
612612
});
613+
614+
describe('when an error occurs while saving account', () => {
615+
const setup = () => {
616+
const school = schoolFactory.build();
617+
const data = oauthDataDtoFactory.build({
618+
system: provisioningSystemDtoFactory.build(),
619+
externalUser: externalUserDtoFactory.build(),
620+
externalSchool: externalSchoolDtoFactory.build(),
621+
});
622+
const user = userDoFactory.build({ id: faker.string.uuid(), roles: [] });
623+
624+
userServiceMock.findByExternalId.mockResolvedValueOnce(null);
625+
userServiceMock.save.mockResolvedValueOnce(user);
626+
schoolServiceMock.getSchools.mockResolvedValueOnce([school]);
627+
roleServiceMock.findByNames.mockResolvedValueOnce([]);
628+
629+
accountServiceMock.findByUserId.mockRejectedValueOnce(new Error('Test error'));
630+
631+
return { data, school, user };
632+
};
633+
634+
it('should delete the user and throw BadDataLoggableException', async () => {
635+
const { data, school, user } = setup();
636+
637+
await expect(sut.provisionUser(data, school)).rejects.toThrow(BadDataLoggableException);
638+
expect(userServiceMock.deleteUser).toHaveBeenCalledWith(user.id);
639+
});
640+
});
613641
});
614642
});

apps/server/src/modules/provisioning/service/tsp-provisioning.service.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,16 @@ export class TspProvisioningService {
199199
}
200200
const savedUser = await this.userService.save(user);
201201

202-
const account = await this.accountService.findByUserId(savedUser.id ?? '');
203-
const updated = this.createOrUpdateAccount(data.system.systemId, savedUser, account);
204-
await this.accountService.save(updated);
202+
try {
203+
const account = await this.accountService.findByUserId(savedUser.id ?? '');
204+
const updated = this.createOrUpdateAccount(data.system.systemId, savedUser, account);
205+
await this.accountService.save(updated);
206+
} catch (error) {
207+
if (existingUser === null) {
208+
await this.userService.deleteUser(savedUser.id ?? '');
209+
}
210+
throw new BadDataLoggableException('Error while saving account', { externalId: data.externalUser.externalId });
211+
}
205212

206213
return user;
207214
}

apps/server/src/modules/user/repo/mikro-orm/user.repo.ts

+10-10
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ import { UserScope } from './user.scope';
1111

1212
@Injectable()
1313
export class UserRepo extends BaseRepo<User> {
14-
get entityName() {
14+
get entityName(): typeof User {
1515
return User;
1616
}
1717

18-
async findById(id: EntityId, populate = false): Promise<User> {
18+
public async findById(id: EntityId, populate = false): Promise<User> {
1919
const user = await super.findById(id);
2020

2121
if (populate) {
@@ -26,7 +26,7 @@ export class UserRepo extends BaseRepo<User> {
2626
return user;
2727
}
2828

29-
async findByIdOrNull(id: EntityId, populate = false): Promise<User | null> {
29+
public async findByIdOrNull(id: EntityId, populate = false): Promise<User | null> {
3030
const user: User | null = await this._em.findOne(User, { id });
3131

3232
if (!user) {
@@ -41,7 +41,7 @@ export class UserRepo extends BaseRepo<User> {
4141
return user;
4242
}
4343

44-
async findByExternalIdOrFail(externalId: string, systemId: string): Promise<User> {
44+
public async findByExternalIdOrFail(externalId: string, systemId: string): Promise<User> {
4545
const [users] = await this._em.findAndCount(User, { externalId }, { populate: ['school.systems'] });
4646
const resultUser = users.find((user) => {
4747
const { systems } = user.school;
@@ -50,7 +50,7 @@ export class UserRepo extends BaseRepo<User> {
5050
return resultUser ?? Promise.reject();
5151
}
5252

53-
async findForImportUser(
53+
public async findForImportUser(
5454
school: SchoolEntity,
5555
filters?: ImportUserNameMatchFilter,
5656
options?: IFindOptions<User>
@@ -71,23 +71,23 @@ export class UserRepo extends BaseRepo<User> {
7171
return countedUsers;
7272
}
7373

74-
async findByEmail(email: string): Promise<User[]> {
74+
public findByEmail(email: string): Promise<User[]> {
7575
// find mail case-insensitive by regex
7676
const promise: Promise<User[]> = this._em.find(User, {
7777
email: new RegExp(`^${email.replace(/\W/g, '\\$&')}$`, 'i'),
7878
});
7979
return promise;
8080
}
8181

82-
async deleteUser(userId: EntityId): Promise<number> {
82+
public async deleteUser(userId: EntityId): Promise<number> {
8383
const deletedUserNumber = await this._em.nativeDelete(User, {
8484
id: userId,
8585
});
8686

8787
return deletedUserNumber;
8888
}
8989

90-
async getParentEmailsFromUser(userId: EntityId): Promise<string[]> {
90+
public async getParentEmailsFromUser(userId: EntityId): Promise<string[]> {
9191
const user: User | null = await this._em.findOne(User, { id: userId });
9292
let parentsEmails: string[] = [];
9393
if (user !== null) {
@@ -109,11 +109,11 @@ export class UserRepo extends BaseRepo<User> {
109109
}
110110
}
111111

112-
saveWithoutFlush(user: User): void {
112+
public saveWithoutFlush(user: User): void {
113113
this._em.persist(user);
114114
}
115115

116-
async flush(): Promise<void> {
116+
public async flush(): Promise<void> {
117117
await this._em.flush();
118118
}
119119

apps/server/src/modules/user/service/user.service.spec.ts

+40
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,46 @@ describe('UserService', () => {
847847
});
848848
});
849849

850+
describe('deleteUser', () => {
851+
describe('when user is successfully deleted', () => {
852+
const setup = () => {
853+
const userId = new ObjectId().toHexString();
854+
855+
userRepo.deleteUser.mockResolvedValueOnce(1);
856+
857+
return { userId };
858+
};
859+
860+
it('should return true', async () => {
861+
const { userId } = setup();
862+
863+
const result = await service.deleteUser(userId);
864+
865+
expect(result).toBe(true);
866+
expect(userRepo.deleteUser).toHaveBeenCalledWith(userId);
867+
});
868+
});
869+
870+
describe(`when user was not deleted`, () => {
871+
const setup = () => {
872+
const userId = new ObjectId().toHexString();
873+
874+
userRepo.deleteUser.mockResolvedValueOnce(0);
875+
876+
return { userId };
877+
};
878+
879+
it('should return false', async () => {
880+
const { userId } = setup();
881+
882+
const result = await service.deleteUser(userId);
883+
884+
expect(result).toBe(false);
885+
expect(userRepo.deleteUser).toHaveBeenCalledWith(userId);
886+
});
887+
});
888+
});
889+
850890
describe('deleteUserData', () => {
851891
describe('when user is missing', () => {
852892
const setup = () => {

apps/server/src/modules/user/service/user.service.ts

+8
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,14 @@ export class UserService implements DeletionService, IEventHandler<UserDeletedEv
228228
}
229229
}
230230

231+
public async deleteUser(userId: EntityId): Promise<boolean> {
232+
const deletionCount = await this.userRepo.deleteUser(userId);
233+
234+
const userDeleted = deletionCount === 1;
235+
236+
return userDeleted;
237+
}
238+
231239
public async deleteUserData(userId: EntityId): Promise<DomainDeletionReport> {
232240
this.logger.info(
233241
new DataDeletionDomainOperationLoggable('Deleting user', DomainName.USER, userId, StatusModel.PENDING)

0 commit comments

Comments
 (0)