Uh oh!
There was an error while loading. Please reload this page.
feat: add delete reference functionality with confirmation modal - #908
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a delete eReference feature: protected DELETE API route and controller, a service method that validates ownership and cascades deletion of related signatures, and a frontend dashboard flow with confirmation modal, mutation, and error handling. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Frontend as Client (Dashboard)
participant API as Backend API
participant Service as ReferenceService
participant DB as Database
User->>Frontend: Click Delete on Reference
Frontend->>Frontend: Open confirmation modal
User->>Frontend: Confirm delete
Frontend->>API: DELETE /api/references/:referenceId (auth)
API->>Service: deleteReference(referenceId, userId)
Service->>DB: Find reference by id & author
alt found & authorized
Service->>DB: Delete ReferenceSignature records
Service->>DB: Delete Reference record
DB-->>Service: success
Service-->>API: return true
API-->>Frontend: 200 OK
Frontend->>Frontend: invalidate queries, show success toast
else not found or not owner
DB-->>Service: not found
Service-->>API: return false
API-->>Frontend: 404 Not Found
Frontend->>Frontend: show error toast
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/ereputation/api/src/services/ReferenceService.ts`:
- Around line 98-112: The deleteReference method currently deletes signatures
then removes the reference separately, risking partial deletes; wrap both
operations in a single DB transaction using AppDataSource.transaction (or a
QueryRunner) so they succeed or roll back together: inside the transaction use
the transactional EntityManager (from AppDataSource.manager.transaction or
queryRunner.manager) to delete ReferenceSignature entries (signatureRepository
via manager.delete) and then remove the Reference entity via manager.remove (or
manager.delete by id) instead of calling AppDataSource.getRepository(...) and
this.referenceRepository.remove outside the transaction; ensure you throw/return
appropriately so failures cause the transaction to rollback.
In `@platforms/ereputation/client/client/src/pages/dashboard.tsx`:
- Around line 85-92: The confirmation dialog is being closed immediately when
the delete is triggered, which prevents the mutation's pending/error UI from
rendering; remove the immediate dialog-close call that runs when the mutation is
invoked (e.g., any closeDialog() or setIsConfirmOpen(false) in the delete
handler) and instead call that same close function inside the mutation onSuccess
block (where queryClient.invalidateQueries and toast are used), leaving the
dialog open on pending and onError so the retry UI (the pending state rendered
around the confirmation dialog) remains visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d47f2177-e1c0-4bc7-8d1e-c96d8483ff0d
📒 Files selected for processing (4)
platforms/ereputation/api/src/controllers/ReferenceController.tsplatforms/ereputation/api/src/index.tsplatforms/ereputation/api/src/services/ReferenceService.tsplatforms/ereputation/client/client/src/pages/dashboard.tsx
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
platforms/ereputation/client/client/src/pages/dashboard.tsx (1)
989-1016:⚠️ Potential issue | 🟡 MinorPrevent closing the confirmation modal while the delete is in flight.
onOpenChangeand the Cancel button can still dismiss the dialog duringdeleteMutation.isPending, so the pending/error state disappears again before the request finishes.Suggested fix
-<Dialog open={!!deleteModalOpen} onOpenChange={(open) => !open && setDeleteModalOpen(null)}>+<Dialog+ open={!!deleteModalOpen}+ onOpenChange={(open) => {+ if (!open && !deleteMutation.isPending) {+ setDeleteModalOpen(null);+ }+ }}+> @@ - <Button- variant="outline"- onClick={() => setDeleteModalOpen(null)}+ <Button+ variant="outline"+ onClick={() => !deleteMutation.isPending && setDeleteModalOpen(null)}+ disabled={deleteMutation.isPending} className="flex-1 border-2 border-fig/30 text-fig/70 hover:bg-fig-10 hover:border-fig/40 font-bold h-11" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/client/client/src/pages/dashboard.tsx` around lines 989 - 1016, The confirmation dialog can be dismissed while a delete is in progress; prevent that by blocking both the Dialog's onOpenChange handler and the Cancel button when deleteMutation.isPending. Update the onOpenChange callback used on the Dialog so it only calls setDeleteModalOpen(null) when open is false AND deleteMutation.isPending is false, and set the Cancel <Button> to disabled={deleteMutation.isPending} (and no-op its onClick when disabled) so users cannot close the modal while confirmDeleteActivity is in flight.platforms/ereputation/api/src/services/ReferenceService.ts (1)
98-109:⚠️ Potential issue | 🔴 CriticalMove the ownership lookup into the transaction.
Lines 99-105 still run outside the transactional delete, so this path keeps a TOCTOU window: a concurrent update/delete can invalidate the ownership check, and Line 109 will then remove a stale entity without re-checking
authorId.Suggested fix
async deleteReference(referenceId: string, authorId: string): Promise<boolean> { - const reference = await this.referenceRepository.findOne({- where: { id: referenceId, authorId }- });-- if (!reference) {- return false;- }-- await AppDataSource.manager.transaction(async (manager) => {- await manager.delete("ReferenceSignature", { referenceId });- await manager.remove(reference);- });-- return true;+ return AppDataSource.manager.transaction(async (manager) => {+ const reference = await manager.findOne(Reference, {+ where: { id: referenceId, authorId },+ });++ if (!reference) {+ return false;+ }++ await manager.delete("ReferenceSignature", { referenceId });+ await manager.delete(Reference, { id: referenceId, authorId });+ return true;+ }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/ereputation/api/src/services/ReferenceService.ts` around lines 98 - 109, Move the ownership lookup into the same transaction used for deletion: inside deleteReference, instead of calling this.referenceRepository.findOne(...) before AppDataSource.manager.transaction(...), perform the find/select for the Reference by id with authorId within the transaction callback (use manager.findOne or manager.getRepository('Reference').findOne), and if not found return false; only then call manager.delete("ReferenceSignature", { referenceId }) and manager.remove(reference) so the existence/ownership check and removals occur atomically within the same transaction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@platforms/ereputation/api/src/services/ReferenceService.ts`:
- Around line 98-109: Move the ownership lookup into the same transaction used
for deletion: inside deleteReference, instead of calling
this.referenceRepository.findOne(...) before
AppDataSource.manager.transaction(...), perform the find/select for the
Reference by id with authorId within the transaction callback (use
manager.findOne or manager.getRepository('Reference').findOne), and if not found
return false; only then call manager.delete("ReferenceSignature", { referenceId
}) and manager.remove(reference) so the existence/ownership check and removals
occur atomically within the same transaction.
In `@platforms/ereputation/client/client/src/pages/dashboard.tsx`:
- Around line 989-1016: The confirmation dialog can be dismissed while a delete
is in progress; prevent that by blocking both the Dialog's onOpenChange handler
and the Cancel button when deleteMutation.isPending. Update the onOpenChange
callback used on the Dialog so it only calls setDeleteModalOpen(null) when open
is false AND deleteMutation.isPending is false, and set the Cancel <Button> to
disabled={deleteMutation.isPending} (and no-op its onClick when disabled) so
users cannot close the modal while confirmDeleteActivity is in flight.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d58b7d9-ad1a-4e91-8371-a1465c227a33
📒 Files selected for processing (2)
platforms/ereputation/api/src/services/ReferenceService.tsplatforms/ereputation/client/client/src/pages/dashboard.tsx
coodos
approved these changes
Mar 10, 2026
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of change
API
api/src/services/ReferenceService.ts): AddeddeleteReference()— verifies author ownership, deletes related signatures, then removes the reference.api/src/controllers/ReferenceController.ts): AddeddeleteReferencehandler with 404 for not found/unauthorized.api/src/index.ts): RegisteredDELETE /api/references/:referenceIdwith auth guard.Client
client/src/pages/dashboard.tsx): Added delete button in activity dropdown (visible for "Reference Provided" entries), confirmation modal, and delete mutation viaapiClient. Stripsref-sent-prefix from activity IDs to get the actual reference UUID.Issue Number
Closes#874
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes / UX