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

chore: Allow for soft deleting the person #667

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
5 changes: 3 additions & 2 deletions apps/api/src/account/account.module.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { Module } from '@nestjs/common'
import { PersonService } from '../person/person.service'

Check warning on line 2 in apps/api/src/account/account.module.ts

View workflow job for this annotation

GitHub Actions / Run API tests

'PersonService' is defined but never used
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 {}
20 changes: 13 additions & 7 deletions apps/api/src/auth/auth.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Beneficiary, Person } from '@prisma/client'
import { Beneficiary, Person, Prisma } from '@prisma/client'

Check warning on line 1 in apps/api/src/auth/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Run API tests

'Prisma' is defined but never used
import { mockDeep } from 'jest-mock-extended'
import { ConfigService } from '@nestjs/config'
import { HttpService } from '@nestjs/axios'
Expand Down Expand Up @@ -448,7 +448,7 @@
// 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
Expand All @@ -463,7 +463,7 @@
})

describe('deleteUser', () => {
const corporatePerson: any = {
const corporatePerson: Awaited<ReturnType<PersonService['findOneByKeycloakId']>> = {
id: 'e43348aa-be33-4c12-80bf-2adfbf8736cd',
firstName: 'Admin',
lastName: 'Dev',
Expand All @@ -483,6 +483,10 @@
profileEnabled: false,
beneficiaries: [],
organizer: null,
deletedAt: null,
helpUsImprove: true,
company: null,
recurringDonations: [],
}

it('should delete user successfully', async () => {
Expand All @@ -493,19 +497,21 @@
.mockResolvedValue(corporatePerson)

const authenticateAdminSpy = jest
.spyOn(service as any, 'authenticateAdmin')

Check warning on line 500 in apps/api/src/auth/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Run API tests

Unexpected any. Specify a different type
.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()

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!`,
)
Expand All @@ -519,7 +525,7 @@
.mockResolvedValue(corporatePerson)

const authenticateAdminSpy = jest
.spyOn(service as any, 'authenticateAdmin')

Check warning on line 528 in apps/api/src/auth/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Run API tests

Unexpected any. Specify a different type
.mockResolvedValueOnce('')

const adminDeleteSpy = jest
Expand All @@ -545,13 +551,13 @@
.mockResolvedValue(corporatePerson)

const authenticateAdminSpy = jest
.spyOn(service as any, 'authenticateAdmin')

Check warning on line 554 in apps/api/src/auth/auth.service.spec.ts

View workflow job for this annotation

GitHub Actions / Run API tests

Unexpected any. Specify a different type
.mockResolvedValueOnce('')

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')
Expand All @@ -561,7 +567,7 @@
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!`,
)
Expand Down
9 changes: 8 additions & 1 deletion apps/api/src/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,16 +494,23 @@ 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(
'Cannot delete a beneficiary, organizer or corporate profile',
)
}

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.prismaService.person.delete({ where: { 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!'}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export class CreatePersonDto {
lastName: string
email?: string
phone?: string
deletedAt?: Date
newsletter?: boolean
helpUsImprove?: boolean
address?: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export class UpdatePersonDto {
lastName?: string
email?: string
phone?: string
deletedAt?: Date
newsletter?: boolean
helpUsImprove?: boolean
address?: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions apps/api/src/person/__mock__/personMock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ export const personMock: Person = {
stripeCustomerId: null,
profileEnabled: true,
helpUsImprove: false,
deletedAt: null,
}
18 changes: 18 additions & 0 deletions apps/api/src/person/person.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export class PersonService {
company: true,
beneficiaries: { select: { id: true } },
organizer: { select: { id: true } },
recurringDonations: { select: { id: true } },
},
})
}
Expand All @@ -137,6 +138,23 @@ export class PersonService {
return await this.prisma.person.delete({ where: { id } })
}

async softDelete(id: string) {
return await this.prisma.person.update({
where: { id },
data: {
firstName: '',
lastName: '',
address: '',
email: '',
birthday: null,
personalNumber: '',
phone: '',
helpUsImprove: false,
profileEnabled: false,
deletedAt: new Date(),
},
})
}
private async addToContactList(createPersonDto: CreatePersonDto) {
const data = {
contacts: [
Expand Down
1 change: 1 addition & 0 deletions db/seed/person/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export const personFactory = Factory.define<Person>(() => ({
updatedAt: faker.date.recent(),
profileEnabled: true,
helpUsImprove: false,
deletedAt: null,
}))
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- AlterTable
ALTER TABLE "people" ADD COLUMN "deleted_at" TIMESTAMPTZ(6);
5 changes: 4 additions & 1 deletion podkrepi.dbml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -988,4 +989,6 @@ Ref: expense_files.uploaderId > people.id

Ref: documents.ownerId > people.id

Ref: campaign_applications.organizerId > organizers.id
Ref: campaign_applications.organizerId > organizers.id

Ref: campaign_application_files.campaignApplicationId > campaign_applications.id
1 change: 1 addition & 0 deletions schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading