Skip to content

chore: Allow for soft deleting the person#667

Open
sashko9807 wants to merge 5 commits intopodkrepi-bg:masterfrom
sashko9807:person-softdelete
Open

chore: Allow for soft deleting the person#667
sashko9807 wants to merge 5 commits intopodkrepi-bg:masterfrom
sashko9807:person-softdelete

Conversation

@sashko9807
Copy link
Copy Markdown
Member

Closes #641

Person might have relation to other entities, thus it is better to soft-delete that person, instead of cascading all person relations

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 2, 2024

✅ Tests will run for this PR. Once they succeed it can be merged.

@sashko9807 sashko9807 changed the title Person softdelete chore: Allow for soft deleting the person Sep 2, 2024
@sashko9807 sashko9807 added the run tests Allows running the tests workflows for forked repos label Sep 2, 2024
@github-actions github-actions bot removed the run tests Allows running the tests workflows for forked repos label Sep 2, 2024
@sashko9807 sashko9807 requested a review from Copilot April 12, 2026 09:59
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds soft-delete support for Person records to avoid foreign key constraint failures when users delete accounts, aligning with issue #641.

Changes:

  • Added deletedAt to the Person model plus a DB migration to persist soft-delete state.
  • Switched account deletion flow to anonymize/soft-delete the person record instead of hard-deleting it, and added a guard against deletion with active recurring donations.
  • Updated mocks/seed data and module wiring to support the new behavior.

Reviewed changes

Copilot reviewed 9 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
schema.prisma Adds deletedAt field to Person Prisma model
podkrepi.dbml Mirrors schema changes in DBML; adds a missing reference
migrations/20240902223329_add_deleted_at_field_to_person/migration.sql Adds deleted_at column to people table
db/seed/person/factory.ts Seeds deletedAt: null
apps/api/src/person/person.service.ts Adds softDelete() and includes recurring donations in findOneByKeycloakId
apps/api/src/person/mock/personMock.ts Adds deletedAt to mock
apps/api/src/domain/generated/person/entities/person.entity.ts Adds deletedAt to generated entity
apps/api/src/domain/generated/person/dto/create-person.dto.ts Adds deletedAt to generated create DTO
apps/api/src/domain/generated/person/dto/update-person.dto.ts Adds deletedAt to generated update DTO
apps/api/src/auth/auth.service.ts Uses softDelete() during user deletion; adds not-found + recurring donation checks
apps/api/src/auth/auth.service.spec.ts Updates deleteUser tests to spy on softDelete(); adjusts mock typing
apps/api/src/account/account.module.ts Imports PersonModule and removes direct PersonService provider

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +141 to +155
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(),
},
@@ -1,4 +1,4 @@
import { Beneficiary, Person } from '@prisma/client'
import { Beneficiary, Person, Prisma } from '@prisma/client'
Comment on lines 494 to 498
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
Comment on lines +505 to +509
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`,
)
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: User not being able to delete his account if it is associated with other parts of the platform.

2 participants