ENG-3328: Add property picker to integration forms

[jpople]

Ticket ENG-3328

Description Of Changes

Adds a "Properties" multi-select field to integration edit forms, allowing users to assign properties to an integration's dataset configs. Properties scope privacy request processing to specific domains/brands.

The picker appears on two surfaces: the integrations modal (ConfigureIntegrationForm) and the system portal config connector form (ConnectorParametersForm). It is gated behind Plus and only shown when the integration has linked dataset configs.

Property assignments are computed as a diff on submit (add new, remove deselected) and applied via bulk-assign/bulk-remove API calls.

Code Changes

• Added dataset-properties.slice.ts — RTK Query endpoints for GET /plus/dataset-properties/{fides_key}, POST /plus/dataset-properties/bulk-assign, and DELETE /plus/dataset-properties/bulk-remove
• Added useIntegrationPropertySelect.ts — hook that fetches dataset configs, property options, and current assignments, and exposes a savePropertyAssignments diff-based submit handler
• Updated ConfigureIntegrationForm and ConnectorParametersForm to render the ControlledSelect property picker when editing a Plus-enabled integration with datasets
• Error handling: property save failures surface a toast without blocking the parent integration save

Steps to Confirm

1. Navigate to an existing integration that has a linked dataset config (Plus required)
2. Open the edit form — confirm a "Properties" multi-select appears
3. Select one or more properties and save — confirm success toast and that properties persist on re-open
4. Deselect a property and save — confirm it is removed
5. Open the form for an integration with no dataset config — confirm the picker is not shown
6. Open the add integration flow — confirm the picker is not shown (edit-only)

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

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ready	Preview, Comment	Apr 15, 2026 6:54pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
fides-privacy-center	Ignored		Apr 15, 2026 6:54pm

[github-actions]

Title	Lines	Statements	Branches	Functions
admin-ui		6.09% (2663/43695)	5.21% (1293/24780)	4.19% (542/12929)
fides-js		78.98% (1962/2484)	65.55% (1214/1852)	72.57% (336/463)
privacy-center		85.97% (331/385)	81.36% (179/220)	78.87% (56/71)

[claude]

Code Review: Add property picker to integration forms

Good feature overall — the hook abstraction is clean, the diff logic is correct, and the UI integration follows the existing patterns in both forms. A few issues need attention before merge.

Critical (must fix)

1. Duplicate hand-rolled interfaces in dataset-properties.slice.ts
Five interfaces declared locally (DatasetPropertyIdsResponse, BulkPropertyAssignment, BulkPropertyRemoval, DatasetPropertyResponse, BulkDatasetPropertyResponse) already exist as auto-generated types in ~/types/api. These will silently drift if the backend schema changes. Import from ~/types/api instead and delete the local declarations. BulkPropertyRemoval doesn't appear in the generated models at all (the removal endpoint reuses BulkPropertyAssignment) and should be removed entirely. See inline comment.

2. Empty-array guard is wrong — can silently delete all property assignments
if (isEditing && values.property_ids) passes when property_ids is [] (arrays are always truthy in JS). When the user opens the form and saves without touching the picker, savePropertyAssignments([]) fires and computes all existing assignments as toRemove, issuing a bulkRemove that deletes everything silently. Fix: values.property_ids !== undefined && hasDatasets. Affects both ConnectorParametersForm.tsx and ConfigureIntegrationForm.tsx. See inline comments.

3. onSaveClick is not awaited before savePropertyAssignments in ConnectorParametersForm.tsx
The integration patch and property save run in parallel rather than sequentially. If the patch fails, the property save has already committed. ConfigureIntegrationForm.tsx correctly awaits the patch result first. The fix requires either making onSaveClick return a Promise, or moving the property save logic into the parent ConnectorParameters.tsx alongside the existing mutation handler. See inline comment.

Suggestions

• Duplicated save block in ConfigureIntegrationForm.tsx: the if (isEditing && values.property_ids) block is copy-pasted verbatim at two save paths (~lines 296 and 354). Extract a shared handlePropertySave local function so both get the same fix and future changes only need to happen once.

• BulkDatasetPropertyResponse.failed is never checked: unwrap() throws on HTTP errors but not on partial failures. If one dataset config fails while others succeed, the operation silently half-completes. Check result.failed?.length after each bulk call and surface it to the user.

• "First dataset config" heuristic: the hook fetches property IDs only for datasetFidesKeys[0], documented as an assumption that all configs share the same assignments. If they ever diverge (e.g., via API), the picker silently shows incomplete state and overwrites on save. Worth a // TODO at minimum.

• Hardcoded size: 100: mirrors the same un-paginated pattern already flagged with a // TODO in the dataset fetch. Add a matching TODO here.

• property_ids not declared in ConnectionConfigFormValues: the type has a [key: string]: any index signature so TypeScript won't catch a key rename. Add property_ids?: string[] explicitly, matching how dataset is typed.

• useIntegrationPropertySelect not re-exported from properties/index.ts: the hook is imported via its full path, bypassing the feature barrel. Either export from the barrel or document it as intentionally internal.

🔬 Codegraph: unavailable

---

💡 Write /code-review in a comment to re-run this review.

[claude]

Review: ENG-3328 — Property picker for integration forms

Overall this is a clean, well-scoped addition. The hook abstraction is the right call — keeping the fetch/diff/save logic out of both form components avoids duplication nicely. The tag-based cache invalidation in the slice is correct, and the error-toast-without-blocking pattern for property failures is the right UX tradeoff. A few things to address before merge:

Bug (should fix)

"Remove all properties" is silently skipped on the secrets-save path. In ConfigureIntegrationForm, the secrets submit guard is if (isEditing && values.property_ids) — an empty array is falsy, so clearing all properties and saving secrets leaves the old assignments in place. The main submit path uses !== undefined correctly; the secrets path should too. See inline comment.

Design concern (worth discussing)

Only the first dataset's assignments seed the diff. If dataset configs under a connection ever have diverging property assignments (direct API, partial prior failure, etc.), the UI silently treats the first dataset as canonical and overwrites the rest on save. The "bulk-assign keeps them in sync" invariant is reasonable but fragile without a guard. At minimum a code comment explaining the invariant would help future readers; a cheap runtime check (warn if any dataset has different IDs) would make bugs surface faster.

Minor

• Hard-coded size: 100 for the property dropdown — fine for now but worth a follow-up if property counts grow.
• Redundant isEditingConnection ? connectionConfig?.key : undefined expression in ConnectorParametersForm — can just be connectionConfig?.key.

---

🔬 Codegraph: connected (46711 nodes)

---

💡 Write /code-review in a comment to re-run this review.

[gilluminate]

Approved with a tiny nit-pick

[gilluminate]

nit: this message is repeated below. Can we set it as a constant?