Skip to content

ENG-3832: Render multi-select dropdown for taxonomy[] custom fields#8197

Open
wadesdev wants to merge 27 commits into
mainfrom
ENG-3832/multi-select-taxonomy-custom-fields
Open

ENG-3832: Render multi-select dropdown for taxonomy[] custom fields#8197
wadesdev wants to merge 27 commits into
mainfrom
ENG-3832/multi-select-taxonomy-custom-fields

Conversation

@wadesdev
Copy link
Copy Markdown
Contributor

Ticket ENG-3832

Description Of Changes

Frontend companion to the fidesplus backend PR for ENG-3832. Custom taxonomy-backed custom fields now support multi-select when their field_type ends in [] (e.g. "risk[]"). The taxonomy detail page renders a multi-select dropdown for these fields and correctly preserves array values through the form.

Code Changes

  • clients/admin-ui/src/features/common/custom-fields/hooks.ts — update array detection in customFieldValues memo: previously only treated "string[]" with an allow list as arrays; now any field_type ending in [] is preserved as an array (covers taxonomy multi-select)
  • clients/admin-ui/src/features/taxonomy/components/TaxonomyCustomFieldsForm.tsx — detect [] suffix on fieldType, strip it for the taxonomyKey prop passed to CustomTaxonomySelect, and pass mode="multiple" when the field is multi-select

Steps to Confirm

  1. Start the local dev server (npm run dev in clients/admin-ui/, backend running)
  2. Create a custom field definition with field_type: "risk[]" via the API
  3. Navigate to a taxonomy detail page for the resource type — the custom field should render as a multi-select dropdown
  4. Select multiple values and save — verify values persist on reload
  5. Confirm a single-select "risk" field (no []) still renders as a single-select dropdown

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • 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 (Cypress E2E tests in taxonomies-plus.cy.ts)
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

wadesdev and others added 20 commits March 18, 2026 10:10
…] suffix

Update hooks.ts to treat any field_type ending in "[]" as a multi-value array
(not just "string[]"), so taxonomy-backed multi-select values are preserved.
Update TaxonomyCustomFieldsForm to strip the [] suffix and pass mode="multiple"
to CustomTaxonomySelect when the definition uses a "taxonomy[]" field type.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…#8197)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 15, 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 20, 2026 9:42pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored May 20, 2026 9:42pm

Request Review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 15, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 11%
8.53% (4053/47479) 7.58% (2094/27593) 5.82% (812/13947)
fides-js Coverage: 78%
79.17% (1977/2497) 66.25% (1249/1885) 73.31% (349/476)
privacy-center Coverage: 86%
83.95% (403/480) 81.25% (208/256) 76.13% (67/88)

…itions

Show a "Selection mode" dropdown (single/multiple) in CustomFieldForm when a
taxonomy template is selected. Submitting with "multiple" appends "[]" to the
field_type sent to the backend. Editing an existing "[]" definition pre-populates
the selection mode correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…omField

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…fields table

Fixes the Type column showing lowercase "geo[]" for multi-select taxonomy
fields — strip the [] suffix before looking up the taxonomy name so it
resolves to the taxonomy's display name (e.g. "Geo") instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wadesdev wadesdev marked this pull request as ready for review May 15, 2026 16:26
@wadesdev wadesdev requested a review from a team as a code owner May 15, 2026 16:26
@wadesdev wadesdev removed the request for review from a team May 15, 2026 16:26
@wadesdev
Copy link
Copy Markdown
Contributor Author

/code-review

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Code Review – ENG-3832: Multi-select taxonomy custom fields

The overall approach is well-structured: the []-suffix convention is applied consistently across hooks, table rendering, form initialisation, payload construction, and the taxonomy detail form. The CustomTaxonomySelect interface already constrains mode to "multiple" | undefined, so the pass-through is clean.

Bug (needs fix before merge)

CustomFieldForm.tsxForm.Item initialValue overrides Form initialValues on edit.
The selection_mode Form.Item has initialValue="single". In Ant Design, Form.Item-level initialValue takes precedence over the Form's initialValues. This means when editing an existing "risk[]" field, parseFieldToFormValues correctly sets selection_mode: "multiple" in Form initialValues, but the Form.Item resets it to "single" on mount. The edit flow would always show "Single select" as the pre-selected value. The fix is to drop the initialValue from the Form.Item — parseFieldToFormValues and the onChange handler already cover all initialisation paths.

Minor issues

  • useCustomFieldsTable.tsx[] suffix leaks through getCustomFieldTypeLabel fallback. If the taxonomy lookup returns nothing (taxonomy deleted, not yet loaded), getCustomFieldTypeLabel is called with the original record still carrying field_type: "risk[]". getCustomFieldType doesn't strip [], so the rendered cell label is the raw "risk[]" string. Passing { ...record, field_type: baseFieldType } to the fallback avoids this.

  • hooks.ts — implicit behavioural change for string[] without allow_list_id. The old guard required allow_list_id to be set before treating string[] as an array. The new endsWith("[]") check removes that gate. In practice this is correct, but a short comment explaining the intentional broadening would help future readers.

  • TaxonomyCustomFieldsForm.tsx — redundant defaultValue on CustomTaxonomySelect. This is pre-existing, but with the new multi-select path it becomes more relevant: in a controlled Ant Design Form, defaultValue on a child input is shadowed by the Form field value injected via Form.Item name. The value is already in Form initialValues; the defaultValue prop is a no-op at best and confusing at worst.

What's good

  • []-suffix stripping is applied in every affected callsite — no missed spots spotted.
  • useCreateOrUpdateCustomField correctly appends/strips [] at the persistence boundary, keeping the convention contained.
  • The selection_mode field is correctly excluded from the API payload via destructuring.
  • CustomTaxonomySelect interface already restricts mode to "multiple" — no risk of passing unsupported Ant Design modes.

🔬 Codegraph: connected (50522 nodes)

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

Comment thread clients/admin-ui/src/features/custom-fields/CustomFieldForm.tsx
Comment thread clients/admin-ui/src/features/common/custom-fields/hooks.ts
taxonomyKey={fieldType}
taxonomyKey={taxonomyKey}
mode={isMultiSelectTaxonomy ? "multiple" : undefined}
defaultValue={customFields.customFieldValues[id]}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clients/admin-ui/src/features/taxonomy/components/TaxonomyCustomFieldsForm.tsx:80

defaultValue is pre-existing here, but it's worth flagging with the new multi-select path: in a controlled Ant Design Form, defaultValue on an inner component is generally ignored in favour of the Form's field value (set via name={id} on the parent Form.Item). The value from customFields.customFieldValues should already be injected through Form initialValues on the wrapping <Form> at line 35. Passing it again as defaultValue is redundant and won't cause a bug here, but it can be confusing and mask issues if the form re-initialises. Consider removing defaultValue and relying solely on the Form's controlled state.

…changes

- Remove initialValue="single" from Form.Item to fix edit form always
  showing "Single select" for existing multi-select definitions
- Pass stripped baseFieldType to getCustomFieldTypeLabel fallback so
  deleted/unloaded taxonomies don't show raw "geo[]" in the type column
- Add comment on endsWith("[]") condition in hooks.ts clarifying it
  intentionally covers both legacy string[] and new taxonomy[] cases

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add required validation rule to selection_mode Form.Item
- Default selection_mode to "single" in Form initialValues for new fields
- Strip selection_mode from OPEN_TEXT and SELECT payloads so it is not
  sent to the backend on non-taxonomy field types

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate left a comment

Choose a reason for hiding this comment

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

Approving with the caveat that the lint errors still need to be addressed.

<Form.Item
label="Selection mode"
name="selection_mode"
rules={[{ required: true, message: "Please select a selection mode" }]}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@wadesdev run npm run check to clean up these lint warnings

@galvana
Copy link
Copy Markdown
Contributor

galvana commented May 20, 2026

DatamapReportTableColumns.tsx:129isArrayField still hardcodes field_type === "string[]". Multi-select taxonomy fields (e.g. "risk[]") won't get ListExpandableCell rendering. Should use .endsWith("[]") to match what was done in hooks.ts:114.

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