ENG-3507: Unlink StagedResources before system deletion by dsill-ethyca · Pull Request #8032 · ethyca/fides

dsill-ethyca · 2026-04-27T13:57:34Z

Summary

Fix IntegrityError when deleting a System that has linked StagedResources
Affects both IDP monitor resources (app promoted to system) and website monitor resources (cookie/JS tag linked to system via vendor_id matching)
Use a SQLAlchemy before_delete event listener on System to null out system_id and reset diff_status to ADDITION on any referencing StagedResources
This approach catches all delete paths (single, bulk, cascade) without modifying endpoint code

Problem

The stagedresource.system_id FK to ctl_systems.id was created without an ondelete clause, defaulting to PostgreSQL's RESTRICT. When a user tries to delete a System that is referenced by any StagedResource, the FK constraint blocks the delete with an IntegrityError.

Steps to Reproduce

IDP Monitor path

Configure an IDP monitor (Okta or Entra) and run a scan
Promote a discovered app (StagedResource) to a System — this sets stagedresource.system_id and diff_status to MONITORED
Attempt to delete the promoted System (single delete or bulk delete)
Expected: System is deleted successfully
Actual (before fix): IntegrityError — the FK constraint on stagedresource.system_id blocks the delete

Website Monitor path

Run a website monitor scan — resources (Cookies, JS tags, browser requests) are discovered
A resource with a vendor_id matches an existing System → system_id is set via associate_resource_with_system_or_vendor() and resource is promoted to MONITORED
Attempt to delete that System
Expected: System is deleted successfully
Actual (before fix): Same IntegrityError

Approach

SQLAlchemy before_delete event listener on System (_unlink_staged_resources_on_system_delete). Before the DELETE is issued, the listener executes an UPDATE to null system_id and reset diff_status to ADDITION on any referencing StagedResources. This keeps the dependency direction correct — the discovery module knows about System (via the FK), not the other way around — and requires no migration.

Test plan

test_single_delete_unlinks_staged_resource — single system delete unlinks IDP monitor StagedResource
test_single_delete_unlinks_web_monitor_staged_resource — single system delete unlinks website monitor StagedResource (Cookie)
test_bulk_delete_systems_with_staged_resources — bulk delete unlinks StagedResources
test_delete_system_without_staged_resources — delete succeeds when no StagedResources reference the system
Existing system delete tests still pass

🤖 Generated with Claude Code

When deleting a System that was promoted from an IDP monitor StagedResource, the FK constraint on stagedresource.system_id blocks the delete. Fix by nulling system_id and resetting diff_status to ADDITION before deletion, within the same transaction. This preserves the StagedResource for re-promotion while allowing the System to be removed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…e-staged-resource

vercel · 2026-04-27T13:57:40Z

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

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Apr 29, 2026 7:09pm
fides-privacy-center	Ignored		Apr 29, 2026 7:09pm

codecov · 2026-04-27T14:07:21Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.97%. Comparing base (353115b) to head (25b6120).
⚠️ Report is 26 commits behind head on main.

❌ Your project status has failed because the head coverage (84.97%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #8032    +/-   ##
========================================
  Coverage   84.96%   84.97%            
========================================
  Files         632      633     +1     
  Lines       41471    41663   +192     
  Branches     4836     4879    +43     
========================================
+ Hits        35235    35402   +167     
- Misses       5136     5151    +15     
- Partials     1100     1110    +10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The DB FK has no ondelete clause; annotating the model with ondelete="SET NULL" is misleading since it doesn't take effect without a migration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Move the StagedResource unlinking from the system delete endpoints into a SQLAlchemy before_delete event listener on the System model, registered from the discovery module. This keeps the dependency direction correct (discovery → system) and ensures cleanup happens for any code path that deletes a System. Follows the same pattern as fidesplus/jira/jira_credential_sync.py which listens for ConnectionConfig before_delete. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dsill-ethyca · 2026-04-28T14:38:31Z

+) -> None:
+    """Null out system_id and reset diff_status on StagedResources before a System is deleted.
+
+    This keeps the dependency direction correct: the discovery module knows about


Follows the same pattern as
fidesplus/jira/jira_credential_sync.py which listens for ConnectionConfig deletion.

- Capture system.id and system.fides_key into locals before the delete API call to avoid ObjectDeletedError on the stale ORM instance - Add changelog entry for PR #8032 - Add debug logging to the before_delete event listener - Refactor tests: yield fixture for cleanup safety, add no-op test case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vcruces

Looks great to me, thanks for making this change! It was needed, and blocking users when trying to delete a system was the wrong behavior

The before_delete listener already handles all StagedResources generically, but the tests, docstring, changelog, and PR description only referenced IDP monitors. Website monitor resources also get system_id set (via vendor_id matching) and hit the same IntegrityError on system deletion. - Add test for web monitor resource type (Cookie) unlinking on system delete - Update docstring to document both IDP and website monitor paths - Update changelog to mention website monitor discovery Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dsill-ethyca and others added 2 commits April 27, 2026 09:10

Merge remote-tracking branch 'origin/main' into ENG-3507-system-delet…

9c4e0b0

…e-staged-resource

vercel Bot deployed to Preview – fides-plus-nightly April 27, 2026 13:58 View deployment

dsill-ethyca and others added 3 commits April 27, 2026 15:55

Revert ondelete annotation — model should match actual DB constraint

a7f1397

The DB FK has no ondelete clause; annotating the model with ondelete="SET NULL" is misleading since it doesn't take effect without a migration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Merge branch 'main' into ENG-3507-system-delete-staged-resource

ed6df8c

dsill-ethyca commented Apr 28, 2026

View reviewed changes

dsill-ethyca marked this pull request as ready for review April 28, 2026 17:36

dsill-ethyca requested a review from a team as a code owner April 28, 2026 17:36

dsill-ethyca requested review from nreyes-dev and removed request for a team April 28, 2026 17:36

vcruces approved these changes Apr 29, 2026

View reviewed changes

dsill-ethyca removed the request for review from nreyes-dev April 29, 2026 19:11

dsill-ethyca added this pull request to the merge queue Apr 30, 2026

Merged via the queue into main with commit 5496449 Apr 30, 2026
70 of 71 checks passed

dsill-ethyca deleted the ENG-3507-system-delete-staged-resource branch April 30, 2026 13:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENG-3507: Unlink StagedResources before system deletion#8032

ENG-3507: Unlink StagedResources before system deletion#8032
dsill-ethyca merged 7 commits into
mainfrom
ENG-3507-system-delete-staged-resource

dsill-ethyca commented Apr 27, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

dsill-ethyca Apr 28, 2026

Uh oh!

vcruces left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

dsill-ethyca commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Steps to Reproduce

IDP Monitor path

Website Monitor path

Approach

Test plan

Uh oh!

vercel Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dsill-ethyca Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

vcruces left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dsill-ethyca commented Apr 27, 2026 •

edited

Loading

vercel Bot commented Apr 27, 2026 •

edited

Loading

codecov Bot commented Apr 27, 2026 •

edited

Loading