Skip to content

ENG-3000: stop using deprecated /system/{key}/connection endpoints, surface DSR controls#8121

Draft
adamsachs wants to merge 9 commits intomainfrom
ENG-3000-fix-saas-integration-system-link
Draft

ENG-3000: stop using deprecated /system/{key}/connection endpoints, surface DSR controls#8121
adamsachs wants to merge 9 commits intomainfrom
ENG-3000-fix-saas-integration-system-link

Conversation

@adamsachs
Copy link
Copy Markdown
Contributor

@adamsachs adamsachs commented May 6, 2026

Ticket ENG-3000

Description Of Changes

Four related changes to ConfigureIntegrationForm (the new top-level Integrations page):

1. Stop using deprecated /system/{key}/connection endpoints. The form was wired to two system-scoped, deprecated endpoints that conflate connection lifecycle with system linking:

  • PATCH /system/{fides_key}/connection
  • POST /(plus/)system/{fides_key}/connection/instantiate/{type}

Both bake a single system into the connection-create/update call. That implicitly assumes one-integration-per-system semantics, and now that the linking refactor (#7432) supports multiple integrations per system, the system-scoped endpoints don't compose cleanly. The most visible symptom is the bug reported in ENG-3000: creating a SaaS integration with a system selected on the new Integrations page errors out / produces an incomplete connection. The patch path was made SaaS-aware in #7978, but it routes to template instantiation only when both connection_type == 'saas' AND secrets is truthy (connection_service.py:476), so a SaaS connector with no required secrets falls through to the standard path and never builds a saas_config or dataset.

The deprecation notes on both system-scoped endpoints already point at the right replacement: do connection work via the top-level /connection endpoints, then manage links via PUT /connection/{key}/system-links.

handleSubmit is refactored to that two-step flow:

  1. Create / update the connection without any system context. New SaaS connections go through POST /connection/instantiate/{type} (createUnlinkedSassConnectionConfig); everything else (non-SaaS create, all edits) goes through PATCH /connection (patchDatastoreConnection).
  2. Reconcile the system link via PUT /connection/{key}/system-links — but only when the desired state differs from initialSystemFidesKey, so common edits (e.g. updating only secrets) skip a redundant idempotent call. Passing links: [] correctly unlinks, fixing the secondary bug where clearing the System field on edit silently left the prior link in place.

The form no longer references usePatchSystemConnectionConfigsMutation, useCreatePlusSaasConnectionConfigMutation, or useCreateSassConnectionConfigMutation.

2. Add an "Enable for privacy requests" toggle. The System → Integrations form already exposes the equivalent control via ConnectorParametersForm, backed by ConnectionConfig.disabled. The new top-level Integrations form was missing it, so once an integration was created its DSR-usage state could only be flipped by going back through the System page. Added a Switch to ConfigureIntegrationForm (always visible, defaults to enabled, saved with the rest of the form on Save). Label and tooltip make the privacy-request scope visible in the form itself.

3. Add a "Privacy request datasets" picker. Mirrors the dataset picker in System → Integrations: edit-only, gated to SystemType.DATABASE integrations, backed by the existing useDatasetConfigField hook and the same PUT /connection/{key}/datasetconfig endpoint. DataHub keeps its own BigQuery-filtered picker untouched.

4. Document ConnectionConfig.disabled as a DSR-scoped flag. It excludes the connection from privacy request execution only — discovery monitors, connection tests, and other non-DSR consumers all have their own enable/disable controls. Added comments to the model column and the two related Pydantic schemas. No behaviour change.

Code Changes

  • clients/admin-ui/src/features/integrations/add-integration/ConfigureIntegrationForm.tsx:
    • Drop imports/hook calls for the three system-scoped mutations and the now-unused systemPatchIsLoading.
    • Replace the three-way connection-creation branch with a binary one: SaaS-create → createUnlinkedSassConnectionConfig, everything else → patchDatastoreConnection.
    • Extract a single reconcileSystemLink() helper that fires only when values.system_fides_key !== initialSystemFidesKey, sends [] to unlink, and consolidates the two prior setSystemLinks invocations.
    • Add an enabled form field (Switch) backed by !ConnectionConfig.disabled; default true for new integrations, mirror existing connection state on edit; route the value into the patch payload as disabled: !values.enabled.
    • Add a "Privacy request datasets" Form.Item for SystemType.DATABASE integrations on edit, using useDatasetConfigField's dropdownOptions and the shared DatasetSelectOption renderer; extend the patchConnectionDatasetConfig calls in handleSubmit to fire for both DataHub and DATABASE-type edits.
  • src/fides/api/models/connectionconfig.py: comment on the disabled column documenting that it gates DSR execution only.
  • src/fides/api/schemas/connection_configuration/connection_config.py: matching comments on CreateConnectionConfiguration.disabled and ConnectionConfigurationResponse.disabled.

Steps to Confirm

In the new top-level Integrations management UI (/integrations):

  1. Create a SaaS integration linked to a system (e.g. Stripe). Pick a system from the System dropdown. Submit and confirm the integration is created, the system link shows on the detail page, and a corresponding DatasetConfig exists. Network tab: POST /api/v1/connection/instantiate/{type} then PUT /api/v1/connection/{key}/system-links. No call to /system/{key}/connection.
  2. Repeat with a system that already has another integration linked to it. Confirm both integrations end up linked to the system (no replacement, no errors).
  3. Edit an existing linked integration → change name/description only (don't touch the System field). Confirm setSystemLinks is not called (no-op skip).
  4. Edit an existing linked integration → clear the System field and save. Confirm the system link is removed (GET /api/v1/connection/{key}/system-links returns []).
  5. Edit an existing linked integration → change to a different system. Confirm the link is replaced.
  6. Create a non-SaaS integration with a system (e.g. Postgres). Should now go through PATCH /connection followed by setSystemLinks. Connection works and is linked.
  7. Create a SaaS integration with no required secrets (the original bug — a connector type whose secrets schema has no required fields, with a system selected). Confirm a saas_config and dataset are created.
  8. Enable toggle, create flow. New integrations default to enabled; toggling off before save persists disabled: true on the connection.
  9. Enable toggle, edit flow. Toggling the switch and saving updates connection.disabled (verify via GET /api/v1/connection/{key}). Confirm a privacy request that would have hit this connection now skips it (e.g. graph_task.skip_if_disabled raises CollectionDisabled).
  10. Datasets picker, database integration. Edit a BigQuery integration. The "Privacy request datasets" multi-select shows currently linked datasets plus any unlinked ones; selecting a new combination and saving updates the connection's DatasetConfig set.

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

🤖 Generated with Claude Code

… new Integrations form

ConfigureIntegrationForm relied on two deprecated, system-scoped endpoints
that conflate connection lifecycle with system linking:

  * PATCH /system/{fides_key}/connection
  * POST  /(plus/)system/{fides_key}/connection/instantiate/{type}

Both bake a single system into the connection-create/update call. That
implicitly assumes one-integration-per-system semantics and produces
inconsistent behavior now that a system can have multiple linked
integrations — for example, creating a SaaS integration with a system
selected would error out (the patch path only routes to template
instantiation when secrets is truthy, so SaaS connectors without required
secrets fell through to the standard path and never got a saas_config or
dataset). The deprecation notes on both endpoints already point at the
right replacement: do connection work via the top-level /connection
endpoints, then manage the link via PUT /connection/{key}/system-links.

Refactor handleSubmit to that two-step flow:

  1. Create / update the connection without any system context. New SaaS
     connections go through POST /connection/instantiate/{type}; everything
     else (non-SaaS create, all edits) goes through PATCH /connection.
  2. Reconcile the system link via PUT /connection/{key}/system-links —
     only when the desired state differs from initialSystemFidesKey, so
     common edits (e.g. updating only secrets) skip a redundant idempotent
     call. Passing links: [] correctly unlinks, fixing the secondary bug
     where clearing the System field on edit silently left the prior link
     in place.

Drops the form's dependency on usePatchSystemConnectionConfigsMutation,
useCreatePlusSaasConnectionConfigMutation, and
useCreateSassConnectionConfigMutation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 6, 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 May 7, 2026 11:44am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored May 7, 2026 11:44am

Request Review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.6% (3042/46036) 5.92% (1572/26527) 4.62% (630/13612)
fides-js Coverage: 78%
79.56% (2028/2549) 66.4% (1259/1896) 73.31% (349/476)
privacy-center Coverage: 85%
82.53% (364/441) 79.74% (189/237) 74.07% (60/81)

adamsachs and others added 2 commits May 6, 2026 14:12
The System -> Integrations form has long had an "Enable integration" switch
backed by ConnectionConfig.disabled. The new top-level Integrations form was
missing this control, so once a SaaS integration was created its DSR-usage
state could only be changed by going back through the System page.

Add an "Enable integration" Switch to ConfigureIntegrationForm:

  * Always visible (create + edit), defaults to enabled.
  * Saved alongside name / description / secrets via the standard Save
    button — not a separate immediate-save toggle. Tooltip clarifies the
    DSR-only scope.

Add backend comments documenting that ConnectionConfig.disabled is a
privacy-request (DSR) execution gate, not a global pause: it does not
exclude the connection from discovery monitors or connection tests.
Comments only — no behavior change on the backend.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adamsachs adamsachs changed the title ENG-3000: stop using deprecated /system/{key}/connection endpoints in new Integrations form ENG-3000: stop using deprecated /system/{key}/connection endpoints, add Enable toggle May 6, 2026
Rename the form label to "Enable for privacy requests" so the DSR scope is
visible at a glance without relying on the tooltip. Tooltip is now a plain
restatement: "When enabled, this integration is used during privacy request
execution." Update the changelog fragment to match the new label.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirror the dataset picker that the System -> Integrations tab already
exposes for database integrations. Renders only on edit (the
PUT /connection/{key}/datasetconfig endpoint requires an existing
connection) and only for SystemType.DATABASE — DataHub keeps its own,
BigQuery-filtered picker untouched.

Field is named "Privacy request datasets" (with a matching tooltip) so
the DSR scoping is visible in the form itself, not just the tooltip.
Backed by the existing useDatasetConfigField hook
(dropdownOptions = currently linked + unlinked available datasets,
rendered with the shared DatasetSelectOption component) and the same
patchConnectionDatasetConfig call already used for DataHub.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adamsachs and others added 2 commits May 6, 2026 14:43
Drop the longer 'discover and act on relevant data' phrasing in favour of
the more direct 'for privacy request fulfillment.'

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adamsachs adamsachs changed the title ENG-3000: stop using deprecated /system/{key}/connection endpoints, add Enable toggle ENG-3000: stop using deprecated /system/{key}/connection endpoints, surface DSR controls May 6, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.23%. Comparing base (aaf933d) to head (8261d22).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8121      +/-   ##
==========================================
+ Coverage   85.19%   85.23%   +0.04%     
==========================================
  Files         638      638              
  Lines       42008    42011       +3     
  Branches     4937     4937              
==========================================
+ Hits        35788    35808      +20     
+ Misses       5111     5096      -15     
+ Partials     1109     1107       -2     

☔ 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 'add a new integration associated with a system' test was waiting on
PATCH /api/v1/system/{key}/connection — the deprecated, system-scoped
endpoint this PR routes off of. With the form now creating the connection
via PATCH /connection and reconciling the link via
PUT /connection/{key}/system-links, the test waits on those two requests
instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant