From 7dc3a9065b2002163e406604e3a75457d64c5c2a Mon Sep 17 00:00:00 2001 From: Aleksandar Date: Tue, 3 Sep 2024 01:36:27 +0300 Subject: [PATCH 1/5] chore: Allow for soft deleting the person --- apps/api/src/account/account.module.ts | 5 +++-- apps/api/src/auth/auth.service.ts | 2 +- .../generated/person/dto/create-person.dto.ts | 1 + .../generated/person/dto/update-person.dto.ts | 1 + .../generated/person/entities/person.entity.ts | 1 + apps/api/src/person/person.service.ts | 17 +++++++++++++++++ db/seed/person/factory.ts | 1 + .../migration.sql | 2 ++ podkrepi.dbml | 5 ++++- schema.prisma | 1 + 10 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 migrations/20240902223329_add_deleted_at_field_to_person/migration.sql diff --git a/apps/api/src/account/account.module.ts b/apps/api/src/account/account.module.ts index 0f1bef50d..ff120ca5b 100644 --- a/apps/api/src/account/account.module.ts +++ b/apps/api/src/account/account.module.ts @@ -4,10 +4,11 @@ import { PrismaService } from '../prisma/prisma.service' import { AccountController } from './account.controller' import { AccountService } from './account.service' import { AuthModule } from '../auth/auth.module' +import { PersonModule } from '../person/person.module' @Module({ - imports:[AuthModule], + imports: [AuthModule, PersonModule], controllers: [AccountController], - providers: [AccountService, PersonService, PrismaService], + providers: [AccountService, PrismaService], }) export class AccountModule {} diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index fb876b54d..cb20cb190 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -503,7 +503,7 @@ export class AuthService { return this.authenticateAdmin() .then(() => this.admin.users.del({ id: keycloakId })) - .then(() => this.prismaService.person.delete({ where: { keycloakId } })) + .then(() => this.personService.softDelete(keycloakId)) .then(() => Logger.log(`User with keycloak id ${keycloakId} was successfully deleted!`)) .catch((err) => { const errorMessage = `Deleting user fails with reason: ${err.message ?? 'server error!'}` diff --git a/apps/api/src/domain/generated/person/dto/create-person.dto.ts b/apps/api/src/domain/generated/person/dto/create-person.dto.ts index 2e7032ad4..9cd0ec57a 100644 --- a/apps/api/src/domain/generated/person/dto/create-person.dto.ts +++ b/apps/api/src/domain/generated/person/dto/create-person.dto.ts @@ -3,6 +3,7 @@ export class CreatePersonDto { lastName: string email?: string phone?: string + deletedAt?: Date newsletter?: boolean helpUsImprove?: boolean address?: string diff --git a/apps/api/src/domain/generated/person/dto/update-person.dto.ts b/apps/api/src/domain/generated/person/dto/update-person.dto.ts index 75f4c44fb..54312a66d 100644 --- a/apps/api/src/domain/generated/person/dto/update-person.dto.ts +++ b/apps/api/src/domain/generated/person/dto/update-person.dto.ts @@ -3,6 +3,7 @@ export class UpdatePersonDto { lastName?: string email?: string phone?: string + deletedAt?: Date newsletter?: boolean helpUsImprove?: boolean address?: string diff --git a/apps/api/src/domain/generated/person/entities/person.entity.ts b/apps/api/src/domain/generated/person/entities/person.entity.ts index 4ee8cc896..2f49b09ea 100644 --- a/apps/api/src/domain/generated/person/entities/person.entity.ts +++ b/apps/api/src/domain/generated/person/entities/person.entity.ts @@ -28,6 +28,7 @@ export class Person { phone: string | null createdAt: Date updatedAt: Date | null + deletedAt: Date | null newsletter: boolean | null helpUsImprove: boolean | null address: string | null diff --git a/apps/api/src/person/person.service.ts b/apps/api/src/person/person.service.ts index 952fdc90e..98bf05ad2 100644 --- a/apps/api/src/person/person.service.ts +++ b/apps/api/src/person/person.service.ts @@ -137,6 +137,23 @@ export class PersonService { return await this.prisma.person.delete({ where: { id } }) } + async softDelete(id: string) { + return await this.prisma.person.update({ + where: { keycloakId: id }, + data: { + firstName: '', + lastName: '', + address: '', + email: '', + birthday: '', + personalNumber: '', + phone: '', + helpUsImprove: false, + profileEnabled: false, + deletedAt: new Date(), + }, + }) + } private async addToContactList(createPersonDto: CreatePersonDto) { const data = { contacts: [ diff --git a/db/seed/person/factory.ts b/db/seed/person/factory.ts index f59acadbd..c67d5b1cb 100644 --- a/db/seed/person/factory.ts +++ b/db/seed/person/factory.ts @@ -22,4 +22,5 @@ export const personFactory = Factory.define(() => ({ updatedAt: faker.date.recent(), profileEnabled: true, helpUsImprove: false, + deletedAt: null, })) diff --git a/migrations/20240902223329_add_deleted_at_field_to_person/migration.sql b/migrations/20240902223329_add_deleted_at_field_to_person/migration.sql new file mode 100644 index 000000000..c83d59f75 --- /dev/null +++ b/migrations/20240902223329_add_deleted_at_field_to_person/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "people" ADD COLUMN "deleted_at" TIMESTAMPTZ(6); diff --git a/podkrepi.dbml b/podkrepi.dbml index f1b8bcdbc..24f50eb7e 100644 --- a/podkrepi.dbml +++ b/podkrepi.dbml @@ -15,6 +15,7 @@ Table people { phone String createdAt DateTime [default: `now()`, not null] updatedAt DateTime + deletedAt DateTime newsletter Boolean [default: false] helpUsImprove Boolean [default: false] address String @@ -988,4 +989,6 @@ Ref: expense_files.uploaderId > people.id Ref: documents.ownerId > people.id -Ref: campaign_applications.organizerId > organizers.id \ No newline at end of file +Ref: campaign_applications.organizerId > organizers.id + +Ref: campaign_application_files.campaignApplicationId > campaign_applications.id \ No newline at end of file diff --git a/schema.prisma b/schema.prisma index d5bb2c695..08051e000 100644 --- a/schema.prisma +++ b/schema.prisma @@ -38,6 +38,7 @@ model Person { phone String? @db.VarChar(50) createdAt DateTime @default(now()) @map("created_at") @db.Timestamptz(6) updatedAt DateTime? @updatedAt @map("updated_at") @db.Timestamptz(6) + deletedAt DateTime? @map("deleted_at") @db.Timestamptz(6) // Receive marketing notifications newsletter Boolean? @default(false) helpUsImprove Boolean? @default(false) @map("help_us_improve") From 2d24a81626f36646fab868c785a7267ec339d692 Mon Sep 17 00:00:00 2001 From: Aleksandar Date: Tue, 3 Sep 2024 01:41:05 +0300 Subject: [PATCH 2/5] chore: Soft delete by person id instead of keycloakid --- apps/api/src/auth/auth.service.ts | 3 ++- apps/api/src/person/person.service.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index cb20cb190..6d413f506 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -494,6 +494,7 @@ export class AuthService { async deleteUser(keycloakId: string) { const user = await this.personService.findOneByKeycloakId(keycloakId) + if (!user) throw new NotFoundException('User not found') //Check and throw if user is a beneficiary, organizer or corporate profile if ((!!user && user.beneficiaries.length > 0) || user?.organizer || user?.companyId) { throw new InternalServerErrorException( @@ -503,7 +504,7 @@ export class AuthService { return this.authenticateAdmin() .then(() => this.admin.users.del({ id: keycloakId })) - .then(() => this.personService.softDelete(keycloakId)) + .then(() => this.personService.softDelete(user.id)) .then(() => Logger.log(`User with keycloak id ${keycloakId} was successfully deleted!`)) .catch((err) => { const errorMessage = `Deleting user fails with reason: ${err.message ?? 'server error!'}` diff --git a/apps/api/src/person/person.service.ts b/apps/api/src/person/person.service.ts index 98bf05ad2..17aee7de3 100644 --- a/apps/api/src/person/person.service.ts +++ b/apps/api/src/person/person.service.ts @@ -139,7 +139,7 @@ export class PersonService { async softDelete(id: string) { return await this.prisma.person.update({ - where: { keycloakId: id }, + where: { id }, data: { firstName: '', lastName: '', From 6e9b55dfac5d61f2dcc35bab516931b8940dd840 Mon Sep 17 00:00:00 2001 From: Aleksandar Date: Tue, 3 Sep 2024 01:44:56 +0300 Subject: [PATCH 3/5] chore: Disallow account deletion if there are active recurring payments --- apps/api/src/auth/auth.service.ts | 6 ++++++ apps/api/src/person/person.service.ts | 1 + 2 files changed, 7 insertions(+) diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index 6d413f506..412f52437 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -502,6 +502,12 @@ export class AuthService { ) } + if (user.recurringDonations.length) { + throw new ForbiddenException( + `Account cannot be deleted due to active recurring payments. Please cancel all recurring payments before deleting this account`, + ) + } + return this.authenticateAdmin() .then(() => this.admin.users.del({ id: keycloakId })) .then(() => this.personService.softDelete(user.id)) diff --git a/apps/api/src/person/person.service.ts b/apps/api/src/person/person.service.ts index 17aee7de3..8d8197a2f 100644 --- a/apps/api/src/person/person.service.ts +++ b/apps/api/src/person/person.service.ts @@ -125,6 +125,7 @@ export class PersonService { company: true, beneficiaries: { select: { id: true } }, organizer: { select: { id: true } }, + recurringDonations: { select: { id: true } }, }, }) } From dbc25b5f0b93665b3f0a06f1dab313b266533509 Mon Sep 17 00:00:00 2001 From: Aleksandar Date: Tue, 3 Sep 2024 01:49:21 +0300 Subject: [PATCH 4/5] fix: Build error --- apps/api/src/person/__mock__/personMock.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/api/src/person/__mock__/personMock.ts b/apps/api/src/person/__mock__/personMock.ts index 69b8bce14..55b516b79 100644 --- a/apps/api/src/person/__mock__/personMock.ts +++ b/apps/api/src/person/__mock__/personMock.ts @@ -19,4 +19,5 @@ export const personMock: Person = { stripeCustomerId: null, profileEnabled: true, helpUsImprove: false, + deletedAt: null, } From 28e144e78886b104bb768fe04d3070c418c1a2c2 Mon Sep 17 00:00:00 2001 From: Aleksandar Date: Tue, 3 Sep 2024 02:27:17 +0300 Subject: [PATCH 5/5] chore: Adjust tests --- apps/api/src/auth/auth.service.spec.ts | 20 +++++++++++++------- apps/api/src/auth/auth.service.ts | 2 +- apps/api/src/person/person.service.ts | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/apps/api/src/auth/auth.service.spec.ts b/apps/api/src/auth/auth.service.spec.ts index b7ad7cb58..5ce13fc34 100644 --- a/apps/api/src/auth/auth.service.spec.ts +++ b/apps/api/src/auth/auth.service.spec.ts @@ -1,4 +1,4 @@ -import { Beneficiary, Person } from '@prisma/client' +import { Beneficiary, Person, Prisma } from '@prisma/client' import { mockDeep } from 'jest-mock-extended' import { ConfigService } from '@nestjs/config' import { HttpService } from '@nestjs/axios' @@ -448,7 +448,7 @@ describe('AuthService', () => { // Don't subscribe to marketing list newsletter: false, }) - + jest.spyOn(prismaMock.person, 'upsert').mockResolvedValue(person) jest.spyOn(admin.users, 'create').mockResolvedValue({ id: keycloakId }) const marketingSpy = jest @@ -463,7 +463,7 @@ describe('AuthService', () => { }) describe('deleteUser', () => { - const corporatePerson: any = { + const corporatePerson: Awaited> = { id: 'e43348aa-be33-4c12-80bf-2adfbf8736cd', firstName: 'Admin', lastName: 'Dev', @@ -483,6 +483,10 @@ describe('AuthService', () => { profileEnabled: false, beneficiaries: [], organizer: null, + deletedAt: null, + helpUsImprove: true, + company: null, + recurringDonations: [], } it('should delete user successfully', async () => { @@ -497,7 +501,9 @@ describe('AuthService', () => { .mockResolvedValueOnce('') const adminDeleteSpy = jest.spyOn(admin.users, 'del').mockResolvedValueOnce() - const prismaDeleteSpy = jest.spyOn(prismaMock.person, 'delete').mockResolvedValueOnce(person) + const prismaDeleteSpy = jest + .spyOn(personService, 'softDelete') + .mockResolvedValueOnce(corporatePerson) const loggerLogSpy = jest.spyOn(Logger, 'log') await expect(service.deleteUser(keycloakId)).resolves.not.toThrow() @@ -505,7 +511,7 @@ describe('AuthService', () => { expect(personSpy).toHaveBeenCalledOnce() expect(authenticateAdminSpy).toHaveBeenCalledTimes(1) expect(adminDeleteSpy).toHaveBeenCalledWith({ id: keycloakId }) - expect(prismaDeleteSpy).toHaveBeenCalledWith({ where: { keycloakId } }) + expect(prismaDeleteSpy).toHaveBeenCalledWith(corporatePerson.id) expect(loggerLogSpy).toHaveBeenCalledWith( `User with keycloak id ${keycloakId} was successfully deleted!`, ) @@ -551,7 +557,7 @@ describe('AuthService', () => { const adminDeleteSpy = jest.spyOn(admin.users, 'del').mockResolvedValueOnce() const prismaDeleteSpy = jest - .spyOn(prismaMock.person, 'delete') + .spyOn(personService, 'softDelete') .mockRejectedValueOnce(new Error('Prisma Rejection!')) const loggerLogSpy = jest.spyOn(Logger, 'error') @@ -561,7 +567,7 @@ describe('AuthService', () => { expect(personSpy).toHaveBeenCalledOnce() expect(authenticateAdminSpy).toHaveBeenCalledTimes(1) expect(adminDeleteSpy).toHaveBeenCalledWith({ id: keycloakId }) - expect(prismaDeleteSpy).toHaveBeenCalledWith({ where: { keycloakId } }) + expect(prismaDeleteSpy).toHaveBeenCalledWith(corporatePerson.id) expect(loggerLogSpy).toHaveBeenCalledWith( `Deleting user fails with reason: Prisma Rejection!`, ) diff --git a/apps/api/src/auth/auth.service.ts b/apps/api/src/auth/auth.service.ts index 412f52437..50f55cb11 100644 --- a/apps/api/src/auth/auth.service.ts +++ b/apps/api/src/auth/auth.service.ts @@ -502,7 +502,7 @@ export class AuthService { ) } - if (user.recurringDonations.length) { + if (user.recurringDonations?.length) { throw new ForbiddenException( `Account cannot be deleted due to active recurring payments. Please cancel all recurring payments before deleting this account`, ) diff --git a/apps/api/src/person/person.service.ts b/apps/api/src/person/person.service.ts index 8d8197a2f..c55e0e938 100644 --- a/apps/api/src/person/person.service.ts +++ b/apps/api/src/person/person.service.ts @@ -146,7 +146,7 @@ export class PersonService { lastName: '', address: '', email: '', - birthday: '', + birthday: null, personalNumber: '', phone: '', helpUsImprove: false,