Skip to content

Fix SaaS connections missing dataset when created with system#7978

Merged
Linker44 merged 4 commits intomainfrom
fix/saas-connection-missing-dataset
Apr 22, 2026
Merged

Fix SaaS connections missing dataset when created with system#7978
Linker44 merged 4 commits intomainfrom
fix/saas-connection-missing-dataset

Conversation

@Linker44
Copy link
Copy Markdown
Contributor

@Linker44 Linker44 commented Apr 21, 2026

Ticket ENG-3525

Summary

  • Backend: _create_saas_connection_from_template was missing the upsert_dataset_config_from_template call, so SaaS connections created through the PATCH /system/{key}/integrations/connection endpoint had no DatasetConfig and were invisible to DSR execution. Added the dataset creation and a duplicate key guard (matching instantiate_connection).
  • Frontend: ConfigureIntegrationForm was sending the vendor name (e.g. "stripe") as connection_type instead of "saas" + saas_connector_type, causing a 422 validation error when creating SaaS integrations with a system selected.
  • Backend: delete_connection_config crashed with AttributeError: 'NoneType' object has no attribute 'delete' when deleting orphaned SaaS connections that had no CtlDataset. Added a None guard.

Test plan

  • Create a SaaS integration (e.g. Stripe) from the integrations page with a system selected — verify the DatasetConfig is created
  • Run a DSR against a system with two integrations — verify both execute
  • Delete a SaaS connection that was created without a dataset (orphaned) — verify no 500 error
  • Create a SaaS integration without a system selected — verify it still works as before
  • Create a DB int without a system selected -verify it still works as before.

@Linker44 Linker44 requested review from a team as code owners April 21, 2026 14:44
@Linker44 Linker44 requested review from adamsachs and jpople and removed request for a team April 21, 2026 14:44
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 21, 2026

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

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Apr 21, 2026 7:42pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Apr 21, 2026 7:42pm

Request Review

When creating a SaaS connection through the integrations page with a
system selected, the request goes through _create_saas_connection_from_template
which was missing the upsert_dataset_config_from_template call. This meant
the connection had no dataset and was invisible to DSR execution.

Three fixes:
- Backend: add upsert_dataset_config_from_template and duplicate key guard
  to _create_saas_connection_from_template (matching instantiate_connection)
- Frontend: send connection_type "saas" + saas_connector_type instead of
  the vendor name as connection_type for SaaS connectors with a system
- Backend: guard against None CtlDataset in delete_connection_config to
  handle orphaned connections created before this fix
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.34% (2799/44080) 5.59% (1402/25050) 4.44% (579/13039)
fides-js Coverage: 78%
78.98% (1962/2484) 65.55% (1214/1852) 72.57% (336/463)
privacy-center Coverage: 88%
85.97% (331/385) 81.36% (179/220) 78.87% (56/71)

@Linker44 Linker44 force-pushed the fix/saas-connection-missing-dataset branch from df623bd to ba55708 Compare April 21, 2026 14:50
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 30.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.93%. Comparing base (e7a6527) to head (0e79a2f).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/fides/service/connection/connection_service.py 28.57% 4 Missing and 1 partial ⚠️
src/fides/api/util/connection_util.py 33.33% 1 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (30.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project status has failed because the head coverage (84.93%) 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    #7978      +/-   ##
==========================================
- Coverage   84.94%   84.93%   -0.01%     
==========================================
  Files         630      630              
  Lines       41086    41095       +9     
  Branches     4769     4771       +2     
==========================================
+ Hits        34901    34906       +5     
- Misses       5102     5106       +4     
  Partials     1083     1083              

☔ 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.

Copy link
Copy Markdown
Contributor

@Vagoasdf Vagoasdf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looking well and working well

Linker44 and others added 2 commits April 21, 2026 13:43
- Use `raise ... from e` to chain the original exception when SaaS
  dataset upsert fails, preserving the root cause in tracebacks.
- Log a warning when no CtlDataset is found during SaaS connection
  deletion, making orphaned state visible instead of silently skipped.
@Linker44 Linker44 added this pull request to the merge queue Apr 21, 2026
Merged via the queue into main with commit 93e749e Apr 22, 2026
72 of 75 checks passed
@Linker44 Linker44 deleted the fix/saas-connection-missing-dataset branch April 22, 2026 00:01
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.

2 participants