fix: Soft Delete + Unique Constraint Conflict #1990 #2073
Conversation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // all categories will be deleted due to 'onDelete: CASCADE'. | ||
| const deleteResult = await transactionalEntityManager.delete( | ||
| ProjectEntity, | ||
| { uuid }, | ||
| ); | ||
| const deleteResult = | ||
| await transactionalEntityManager.softDelete(ProjectEntity, { | ||
| uuid, | ||
| }); |
There was a problem hiding this comment.
softDelete(ProjectEntity, { uuid }) does not trigger DB-level ON DELETE CASCADE, so categories will not be deleted. The inline comment stating they will be deleted due to cascade is now incorrect and may hide orphaned categories; either remove/update the comment, or explicitly delete/soft-delete related categories if that behavior is required.
| // 4. Verify it still exists in DB but is deleted | ||
| const projectRepo = database.getRepository(ProjectEntity); | ||
| const dbProject = await projectRepo.findOne({ | ||
| where: { uuid }, | ||
| withDeleted: true, | ||
| }); | ||
| expect(dbProject).toBeDefined(); | ||
| expect(dbProject?.deletedAt).not.toBeNull(); | ||
|
|
There was a problem hiding this comment.
This assertion is likely to fail because BaseEntity.deletedAt is defined with @DeleteDateColumn({ select: false }), so findOne({ withDeleted: true }) will typically not select deletedAt (it will be undefined even when soft-deleted). Consider verifying soft-delete via exists()/default queries, or use a query builder with addSelect('project.deletedAt') when withDeleted() is enabled.
| // 4. Verify it still exists in DB but is deleted | |
| const projectRepo = database.getRepository(ProjectEntity); | |
| const dbProject = await projectRepo.findOne({ | |
| where: { uuid }, | |
| withDeleted: true, | |
| }); | |
| expect(dbProject).toBeDefined(); | |
| expect(dbProject?.deletedAt).not.toBeNull(); | |
| // 4. Verify it still exists in DB but is soft-deleted (not returned by default queries) | |
| const projectRepo = database.getRepository(ProjectEntity); | |
| const dbProjectWithDeleted = await projectRepo.findOne({ | |
| where: { uuid }, | |
| withDeleted: true, | |
| }); | |
| expect(dbProjectWithDeleted).toBeDefined(); | |
| const dbProjectWithoutDeleted = await projectRepo.findOne({ | |
| where: { uuid }, | |
| }); | |
| expect(dbProjectWithoutDeleted).toBeNull(); |
| const dbMission = await missionRepo.findOne({ | ||
| where: { uuid: missionUuid }, | ||
| withDeleted: true, | ||
| }); |
There was a problem hiding this comment.
Same issue as the project test: deletedAt is marked select: false on BaseEntity, so findOne({ withDeleted: true }) may return deletedAt as undefined. Use a query builder with addSelect to read deletedAt, or assert soft-delete behavior using exists()/default find operations instead.
| const dbMission = await missionRepo.findOne({ | |
| where: { uuid: missionUuid }, | |
| withDeleted: true, | |
| }); | |
| const dbMission = await missionRepo | |
| .createQueryBuilder('mission') | |
| .addSelect('mission.deletedAt') | |
| .withDeleted() | |
| .where('mission.uuid = :uuid', { uuid: missionUuid }) | |
| .getOne(); |
No description provided.