Skip to content

feat(store): user, organization, and project reads skip soft-deleted rows - #1931

Merged
whoAbhishekSah merged 3 commits into
mainfrom
soft-delete-live-read-helpers
Sep 18, 2026
Merged

whoAbhishekSah merged 3 commits into
mainfrom
soft-delete-live-read-helpers

Conversation

@whoAbhishekSah

@whoAbhishekSah whoAbhishekSah commented Sep 17, 2026

Copy link
Copy Markdown
Member

Soft delete keeps a row and sets deleted_at. Reads have to skip those rows. This is the first of three PRs that make the identity repositories do that.

What

  • Two helpers in postgres.go: live(table) is the deleted_at IS NULL filter, fromLive(table) is dialect.From with it applied.
  • Every read in the user, organization, and project repositories goes through fromLive.
  • Every update in those repositories adds the live filter too, so a deleted row is never written to.

Behaviour change

None today. No row has deleted_at set yet. This must reach every environment before any release that writes deleted_at.

Tested

One new test per repository: soft-delete a seeded row, then check every get, list, and update skips it. Store package passes with -race. Verified live against the sandbox twice.

@vercel

vercel Bot commented Sep 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
frontier Ready Ready Preview Sep 18, 2026 7:49am UTC

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8d857b19-c69e-4ec2-8e87-ace7295d1bbd

📥 Commits

Reviewing files that changed from the base of the PR and between ef7ff66 and 9096e51.

📒 Files selected for processing (2)
  • internal/store/postgres/project_repository.go
  • internal/store/postgres/project_repository_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Soft-deleted organizations, projects, and users are now excluded from lookups and listings.
    • Updates by name, email, or ID no longer modify soft-deleted records.
    • Operations targeting soft-deleted records now correctly report that the record does not exist.
    • State changes no longer affect soft-deleted organizations, projects, or users.
    • Project state updates now return the correct project-not-found result when the record has been soft-deleted.

Walkthrough

Changes

PostgreSQL repositories now exclude soft-deleted organizations, projects, and users from reads and updates. Shared helpers add deleted_at IS NULL predicates. Tests cover repository behavior and generated SQL.

Soft-delete filtering

Layer / File(s) Summary
Shared live-row query helpers
internal/store/postgres/postgres.go
Adds live and fromLive helpers that filter rows with deleted_at IS NULL.
Organization and project repository filtering
internal/store/postgres/organization_repository.go, internal/store/postgres/organization_repository_test.go, internal/store/postgres/project_repository.go, internal/store/postgres/project_repository_test.go
Applies live-row filtering to reads and updates. project.SetState now returns project.ErrNotExist when no live row matches. Tests verify soft-deleted records are excluded.
User repository filtering and validation
internal/store/postgres/user_repository.go, internal/store/postgres/user_repository_test.go
Applies live-row filtering to user reads, prepared queries, and updates. SQL expectations and soft-delete tests are updated.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: amangit07

Merge Risk: ⚪ Minimal · up to 9096e

Supported reads and updates consistently exclude soft-deleted records, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coveralls

coveralls commented Sep 17, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 35321254518

Coverage increased (+0.07%) to 50.306%

Details

  • Coverage increased (+0.07%) from the base build.
  • Patch coverage: 36 of 36 lines across 4 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 41122
Covered Lines: 20687
Line Coverage: 50.31%
Coverage Strength: 16.14 hits per line

💛 - Coveralls

@whoAbhishekSah

Copy link
Copy Markdown
Member Author

Added a second commit after a live test run against the branch found that UpdateProject and UpdateUser by uuid still wrote into a soft-deleted row.

UpdateByID and SetState on users, organizations, and projects now add the same live filter as the name-keyed updates. The three tests cover them. Project SetState runs a plain exec and does not report a miss, so its test checks the row's state directly.

@whoAbhishekSah

Copy link
Copy Markdown
Member Author

Tested live on a local server built from this branch. Fixtures: 3 users, 2 orgs, 2 projects. Ran every affected RPC before and after setting deleted_at directly in the DB, then diffed.

Path RPCs Result
Get by id / name / email GetOrganization, GetProject, GetUser not found
Lists ListAllOrganizations, ListOrganizationsByCurrentUser, ListOrganizationProjects, ListProjects, ListProjectsByCurrentUser, ListAllUsers, SearchUsers (incl. group_by) soft-deleted row dropped
Member lists (GetByIDs) ListOrganizationUsers, ListProjectUsers, ListOrganizationsByUser soft-deleted user dropped
Updates by id / name / email UpdateOrganization, UpdateProject, UpdateUser not found, row untouched (by-id fixed in the second commit)
Org gate ListOrganizationUsers, ListOrganizationProjects, CreateProject, CheckOrganizationDelete on a soft-deleted org not found
Name reuse CreateOrganization / CreateProject with a soft-deleted name succeeds (live-only unique index)
Auth interceptor any RPC with a soft-deleted user's live session not found

Seen but out of scope here, lands in later PRs:

  • CheckResourcePermission still returns true for a soft-deleted org or project. The SpiceDB tuples are removed by the delete cascade work.
  • A soft-deleted user's live session gets 404 instead of 401. Session teardown on delete handles this.
  • Login or UpdateUser with a soft-deleted email creates a new account with no memberships. Intended.
  • Policies, relations, resources, domains, groups, and service users are the next two PRs.

Comment thread internal/store/postgres/project_repository.go Outdated
…rows

Adds two helpers next to the shared goqu dialect. live(table) is the
deleted_at IS NULL filter, qualified with the table name so it stays correct
inside joins. fromLive(table) is dialect.From(table) with that filter applied.

Every read in the user, organization, and project repositories now goes
through fromLive. The four updates keyed by name or email instead of id also
add the filter, so a deleted row that shares a name with a live one is never
updated by mistake.

No row has deleted_at set today, so nothing changes for callers. This must be
deployed everywhere before any soft-delete write ships.
UpdateByID and SetState on users, organizations, and projects filtered only
on the id. A soft-deleted row could still be written to. Each now adds the
live filter, so the update matches nothing and reports not found, the same
way a read does.

Project SetState runs a plain exec and does not report a miss, so its test
checks the row's state directly instead of expecting an error.
…versions

Project SetState ran a plain exec and returned nil whether or not a row
matched. Its no-rows branch could never fire and returned the user sentinel.
It now returns the updated row and maps a miss to project.ErrNotExist, the
same shape as the user and organization SetState.
@whoAbhishekSah
whoAbhishekSah force-pushed the soft-delete-live-read-helpers branch from 9096e51 to 121181c Compare September 18, 2026 07:48
@whoAbhishekSah
whoAbhishekSah merged commit f840f22 into main Sep 18, 2026
8 checks passed
@whoAbhishekSah
whoAbhishekSah deleted the soft-delete-live-read-helpers branch September 18, 2026 08:06
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.

3 participants