Repository navigation
Conversation
Delete soft-deletes the user's sessions before it removes the relations and the user row. Today the sessions go only through the sessions.user_id ON DELETE CASCADE, which stops firing once the user row is kept.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Delete sessions before user records core/user/service.go, core/user/service_test.go |
Service.Delete removes sessions before relations and the user record. Unit tests check the order and confirm that a session-deletion error stops the operation. |
Check session after user deletion test/e2e/regression/api_test.go |
The regression test verifies that a request using the deleted user’s session fails with Unauthenticated. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: kushsharma
Merge Risk: 
c60c0
-
Session cleanup now runs before user removal, and the deletion stops if cleanup fails. No established issue prevents merging after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Coverage Report for CI Build 40175772677Coverage increased (+0.003%) to 55.732%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Summary
user.Service.Deletenow ends the user's sessions before it removes the user's relations and the user row. Today the sessions go only becausesessions.user_idreferencesuserswithON DELETE CASCADE, which fires on the hard delete. #2023 makes the user delete keep the row, so that cascade stops firing. Without this change, a request with a deleted user's session cookie would get not found instead of unauthenticated. This PR should be merged before #2023.Changes
core/user/service.go:Deletefirst callssessionService.DeleteByUserID, the same callDisablemakes. A failure stops the delete before the relations and the row.Technical Details
DeleteByUserIDsetsdeleted_aton each live session of the user. A session withdeleted_atset failsSession.IsValid, so the session authenticator skips it and the request is unauthenticated.session.revokedaudit record, as withDisableUser. No record is written when the actor is the session owner.deleted_at.Test Plan
core/user/service_test.goTestService_Delete: the sessions end before the relations and the user row; a session failure stops the delete before the relations and the user; a relation failure stops it before the user. The cases fail without the session step, with the step after the relations, or with its error ignored.TestUserAPIcase 13: a logged-in user is deleted by an admin, and the same session cookie then gets unauthenticated. Onmainthis passes without the change too, because the hard delete still removes the sessions. With feat(store): user delete keeps the row #2023's soft delete applied locally, it passes with this change and fails without it: the cookie gets not found instead of unauthenticated.go test ./core/user/...passes.go test ./test/e2e/...passes.golangci-lint run ./core/user/... ./test/e2e/regression/...reports no issues.