Skip to content

feat(deleter): project delete soft-deletes its resources - #2013

Merged
AmanGIT07 merged 2 commits into
mainfrom
feat/project-delete-soft-resources
Oct 23, 2026
Merged

AmanGIT07 merged 2 commits into
mainfrom
feat/project-delete-soft-resources

Conversation

@AmanGIT07

@AmanGIT07 AmanGIT07 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The project delete now soft-deletes the project's live resources with resource.Service.Delete, which keeps the row and removes its SpiceDB tuples, instead of purging every row. A resource that is already gone is skipped, like the org's own resources. Purge and resource.Filter.IncludeDeleted only existed so the old hard project delete could clear every row that pointed at the project. The project row is now kept, so both are removed. Stacked on #2008.

Changes

  • core/deleter/service.go: the resource step lists live resources only and calls Delete, skipping resource.ErrNotExist. The order stays: policies, then resources, then the project row. Each removed resource gets the resource.deleted audit record that the resource delete API also writes.
  • core/resource/service.go, core/resource/resource.go, internal/store/postgres/resource_repository.go: Purge removed from the service, the repository interface, and the repository. The deleter's ResourceService interface drops it too; both mocks are regenerated.
  • core/resource/filter.go, internal/store/postgres/resource_repository.go: IncludeDeleted removed; List always skips deleted rows.
  • test/e2e/regression/api_test.go: the org delete test with a disabled project now gives that project a resource. The suite allows compute/volume for both deletes and creates its permission in the setup, and the project in TestProjectDeleteKeepsTheRowAndRemovesAccess gets a compute/volume resource.

Test Plan

  • core/deleter/service_test.go: the project and org delete cases expect a live-only list and Delete; a resource that is already gone does not stop the project delete.
  • TestPurge removed from core/resource/service_test.go and internal/store/postgres/resource_repository_test.go, with the IncludeDeleted assertion in TestSkipsSoftDeletedResources.
  • e2e TestOrganizationDeleteAlsoDeletesDisabledProjectsAndGroups: after the org delete, the disabled project's row and its resource's row are kept with deleted_at set. With the resource delete reading the project through the lookup that hides disabled projects, the org delete fails with failed to get project, so the test covers fix(resource): a resource in a disabled project can be deleted #2006's lookup.
  • e2e TestProjectDeleteKeepsTheRowAndRemovesAccess: after DeleteProject, the project's compute/volume resource row is kept with deleted_at set. With the resource step skipped, this check fails.
  • go test ./core/... ./internal/api/... and the resource and project repository suites pass.
  • e2e TestEndToEndAPIRegressionTestSuite, TestEndToEndServiceUsersRegressionTestSuite and TestEndToEndPATRegressionTestSuite pass.
  • golangci-lint run ./core/... ./internal/store/postgres/... ./test/e2e/regression/... reports no issues.

SQL Safety

  • No new values in queries; List now always starts from fromLive(TABLE_RESOURCES), and the removed Purge was the only DELETE on resources.
  • ToSQL() callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Never query, _, err := ….
  • No ? placeholders inside single-quoted SQL literals in goqu.L.
  • No new //nolint:forbidigo or // #nosec G20x annotations.

@vercel

vercel Bot commented Oct 21, 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 Oct 23, 2026 3:25am UTC

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 487ee5d and 46950f2.

📒 Files selected for processing: 11
  • core/deleter/mocks/resource_service.go
  • core/deleter/service.go
  • core/deleter/service_test.go
  • core/resource/filter.go
  • core/resource/mocks/repository.go
  • core/resource/resource.go
  • core/resource/service.go
  • core/resource/service_test.go
  • internal/store/postgres/resource_repository.go
  • internal/store/postgres/resource_repository_test.go
  • test/e2e/regression/api_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 49d6736 and 487ee5d.

📒 Files selected for processing: 14
  • core/deleter/deleter.go
  • core/deleter/mocks/resource_service.go
  • core/deleter/service.go
  • core/deleter/service_test.go
  • core/resource/filter.go
  • core/resource/mocks/repository.go
  • core/resource/resource.go
  • core/resource/service.go
  • core/resource/service_test.go
  • internal/api/v1beta1connect/deleter.go
  • internal/api/v1beta1connect/deleter_test.go
  • internal/store/postgres/resource_repository.go
  • internal/store/postgres/resource_repository_test.go
  • test/e2e/regression/api_test.go
💤 Files with no reviewable changes: 7
  • core/resource/mocks/repository.go
  • core/resource/service.go
  • core/resource/resource.go
  • core/resource/filter.go
  • core/resource/service_test.go
  • core/deleter/mocks/resource_service.go
  • internal/store/postgres/resource_repository_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Project deletion now validates project IDs and returns a not-found response for invalid or missing projects.
    • Projects with blocking resources cannot be deleted; the response identifies the resource preventing deletion. Once blockers are removed, deletion can proceed.
    • Deletion now handles already-missing project data and resources without stopping the cleanup process.
    • Deleting a project or organization leaves its resources marked as deleted rather than permanently removing them.

Project deletion now validates the project ID, checks for blockers, and tears down the loaded project. Resource purge APIs were removed. Connect responses and regression tests now cover project deletion errors and resource deletion behavior.

Changes: Project and organization deletion

Layer / File(s) Summary
Resource deletion contracts and behavior
core/resource/*, internal/store/postgres/resource_repository.go, internal/store/postgres/resource_repository_test.go, core/deleter/service.go, core/deleter/mocks/resource_service.go
The Purge methods and IncludeDeleted filter were removed. Resource listing now uses live resources, and related mocks and tests were updated.
Project and organization teardown
core/deleter/deleter.go, core/deleter/service.go, core/deleter/service_test.go
Project deletion validates UUIDs, checks for disabled projects and resource blockers, then tears down the loaded project. Organization deletion calls project teardown directly. Tests cover blockers, missing items, and cascades.
Connect deletion responses
internal/api/v1beta1connect/deleter.go, internal/api/v1beta1connect/deleter_test.go
Project deletion maps blockers to failed-precondition and missing or invalid-UUID projects to not-found. Blocked-delete logging includes organization and project IDs.
Deletion regression coverage
test/e2e/regression/api_test.go
Regression tests cover project blockers, deletion after resource removal, organization deletion with remaining resources, and soft-deleted database rows.

Priority: ⚪ Pending latest changes

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

Change: Feature

Suggested reviewers: rohilsurana, whoabhisheksah

Merge Risk: ⚪ Minimal · up to 487ee

No actionable merge-blocking issue was identified in the project and organization deletion changes. The change is ready for normal merge checks.

Pre-merge checks: Passed 2
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.
  • 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.

❤️ Share

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

@coveralls

coveralls commented Oct 21, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 39894875643

Coverage decreased (-0.02%) to 55.697%

Details

  • Coverage decreased (-0.02%) from the base build.
  • Patch coverage: 5 of 5 lines across 2 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: 41884
Covered Lines: 23328
Line Coverage: 55.7%
Coverage Strength: 19.35 hits per line

💛 - Coveralls

@AmanGIT07

Copy link
Copy Markdown
Contributor Author

Sandbox run: a local Frontier server with Postgres and SpiceDB in Docker, called over the Connect protocol. The binary was built from #2014 with #2011 applied, so it has main plus #2008, #2011, #2013 and #2014.

Deletion config used:

app:
  deletion:
    enabled: true
    resources:
      resource/aoi: { project: block, organization: allow }
      resource/workflow: { project: block, organization: allow }
      resource/orderitem: { project: block_unless_deletable, organization: block_unless_deletable }
      resource/insight: { project: allow, organization: allow }

resource/report has no rule.

Resources of a deleted project

Check Expected Observed
Delete a project after 4 of its 5 resources were deleted: all 5 resource rows, incl. the finished order item that was still live 5 kept with deleted_at, 0 live 5 kept with deleted_at, 0 live
Owner can get the finished order item, before → after true → false true → false
Delete a project holding only an insight ok ok
Its insight row 1 kept with deleted_at, 0 live 1 kept with deleted_at, 0 live
Owner can get the insight after the delete false false
Delete a disabled project holding an insight ok ok
Its insight row 1 kept with deleted_at, 0 live 1 kept with deleted_at, 0 live
Delete an org with a project holding an AOI and a disabled project holding an insight: both project rows kept with deleted_at 2 2
That org's 2 resource rows 2 kept with deleted_at, 0 live 2 kept with deleted_at, 0 live
With enabled: false, its 2 resource rows 2 kept with deleted_at, 0 live 2 kept with deleted_at, 0 live

Project row and access after the delete (from #2007, together with this PR)

Check Expected Observed
Project row kept with deleted_at 1 1
Policies on the project: live before → live after, marked deleted 1 → 0, 1 1 → 0, 1
Owner can get the project, before → after true → false true → false
Platform superuser can get the project, before → after true → false true → false
GetProject on the deleted project (owner) permission_denied permission_denied
Second DeleteProject on the same project permission_denied permission_denied
A new project can take the deleted project's name ok ok

Comment thread core/deleter/service.go
Comment thread core/deleter/service.go
Comment thread test/e2e/regression/api_test.go
Base automatically changed from feat/project-delete-blockers to main October 23, 2026 03:21
The project delete now deletes the project's live resources with Delete,
which keeps the row and removes its SpiceDB tuples, instead of purging
every row. A resource that is already gone is skipped. Purge and
resource.Filter.IncludeDeleted have no users left and are removed.
The API suite allows compute/volume for both deletes and creates its
permission in the suite setup. The project in
TestProjectDeleteKeepsTheRowAndRemovesAccess gets a compute/volume
resource, and its row is kept with deleted_at set after the delete.
@AmanGIT07
AmanGIT07 force-pushed the feat/project-delete-soft-resources branch from 487ee5d to 46950f2 Compare October 23, 2026 03:24
@AmanGIT07
AmanGIT07 merged commit 7338205 into main Oct 23, 2026
8 checks passed
@AmanGIT07
AmanGIT07 deleted the feat/project-delete-soft-resources branch October 23, 2026 03:29

This branch was successfully deployed

1 active deployment
Preview — 46950f2a Deployed Oct 23, 2026 by vercel[bot]
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