Repository navigation
Conversation
The user row gets deleted_at instead of being removed, and the user.deleted audit record takes its occurred_at from deleted_at. A second delete answers not found.
The search inner joins users with the live filter instead of a left join, so the list and the count skip a PAT whose owner is soft-deleted.
|
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 |
|---|---|
User soft deletion and audit timestamp internal/store/postgres/user_repository.go, internal/store/postgres/user_repository_test.go |
Delete now uses soft deletion and records the row’s DeletedAt.Time. Tests verify row retention, subsequent lookup and deletion behavior, and recreation with the same name and email under a different ID. |
Filter PATs by live owner internal/store/postgres/org_pats_repository.go, internal/store/postgres/org_pats_repository_test.go, internal/store/postgres/org_pats_repository_pg_test.go |
The shared PAT query joins only live users. Unit and PostgreSQL tests check that searches exclude PATs of soft-deleted users and return matching totals. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Feature · Severity of issue fixed: Low
Possibly related PRs
- raystack/frontier#2022: Both changes update the user-deletion lifecycle.
#2022removes user policies before repository deletion, while this PR changes repository deletion to soft deletion and filters PAT search by owner status.
Suggested reviewers: kushsharma, rohanchkrabrty
Merge Risk: 
dc6c8
-
User deletion now keeps the row and marks it deleted, and organization PAT searches omit PATs owned by deleted users. No concrete merge-blocking issue remains. The PR notes that it depends on being merged after
#2022.
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 40174593021Coverage increased (+0.03%) to 55.77%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Stacked on #2022. Merge #2022 and #2024 first.
Summary
Deleting a user now keeps the user row and sets
deleted_at, like the other soft deletes. Theuser.deletedaudit record takes its time fromdeleted_at. The organization PAT search now lists a PAT only when its owner is live.Changes
internal/store/postgres/user_repository.go:Deletesetsdeleted_aton the live row instead of deleting it. It still returns the row and writes theuser.deletedrecord in the same transaction, now withoccurred_atequal todeleted_at. A second delete answersuser.ErrNotExist.internal/store/postgres/org_pats_repository.go: the main query inner joinsuserson the PAT owner withusers.deleted_at IS NULL, instead of a left join. The count and the list both skip a PAT whose owner is deleted.Technical Details
userswithout the live filter.user_pats.user_idisNOT NULLand referencesusers, so every PAT has an owner row, and the inner join drops exactly the PATs whose owner is deleted. The user delete removes the user's PATs before the user row, so this only matters for a PAT created while a delete runs.Known gap
sessions.user_idreferencesuserswithON DELETE CASCADE, so the hard delete removed the user's sessions with the row. The soft delete does not. Without #2024, a request with a deleted user's session cookie gets not found instead of unauthenticated. #2024 ends the sessions before the user row; merge it before this PR.Test Plan
internal/store/postgres/user_repository_test.goTestDeleteKeepsTheRow: the row stays withdeleted_atset,GetByIDanswers not found, a second delete answersuser.ErrNotExistand writes no second record, the record'soccurred_atequalsdeleted_at, and a new user can be created with the same email and name. With the hard delete back, the row check fails; withtime.Now()back, the time check fails.internal/store/postgres/org_pats_repository_test.go: the count and data queries contain the inner join with the live filter.internal/store/postgres/org_pats_repository_pg_test.go: with one live and one soft-deleted owner, the list and the count return only the live owner's PAT, and a search for the deleted owner's name finds nothing. Both cases fail with the left join this PR replaces. With the live filter in the condition of a left join, the list case fails.go test ./internal/store/postgres/...passes.go test ./test/e2e/...passes.golangci-lint run ./internal/store/postgres/...reports no issues.SQL Safety
softDeletewithgoqu.Ex{"id": id}, and the join condition built withgoqu.On,goqu.Iand thelivehelper.ToSQL()callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Neverquery, _, err := ….?placeholders inside single-quoted SQL literals ingoqu.L.//nolint:forbidigoor// #nosec G20xannotations.