diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b52d91c814..d8a13774ede 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,12 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o ## [Unreleased](https://github.com/ethyca/fides/compare/2.84.0..main) +### Added +- Privacy center form builder: free reordering of identity and custom fields via a new `field_order` array on each privacy request action, persisted end-to-end through the form builder, the privacy center renderer, and admin/property APIs. + +### Deprecated +- `custom_privacy_request_field_order` on privacy center actions — superseded by `field_order`, which orders identity and custom fields jointly. Existing configs continue to read correctly. + ## [2.84.0](https://github.com/ethyca/fides/compare/2.83.3..2.84.0) ### Added diff --git a/clients/admin-ui/cypress/e2e/properties/form-builder.cy.ts b/clients/admin-ui/cypress/e2e/properties/form-builder.cy.ts index 72b4895824b..80b50c44643 100644 --- a/clients/admin-ui/cypress/e2e/properties/form-builder.cy.ts +++ b/clients/admin-ui/cypress/e2e/properties/form-builder.cy.ts @@ -111,7 +111,64 @@ describe("Privacy center form builder", () => { .should("be.visible"); }); - // ── Test 2: dropped-features acknowledgement ──────────────────────────────── + // ── Test 2: field_order persists across save ──────────────────────────────── + it("saves field_order so a custom field can sit between identity fields", () => { + // Spec places `reason` between Email and Phone — i.e. the legacy + // hardcoded name → email → phone → customs ordering can't represent this. + const spec = { + root: "form", + elements: { + form: { + type: "Form", + props: {}, + children: ["f_email", "f_reason", "f_phone"], + }, + f_email: { type: "Email", props: { required: true }, children: [] }, + f_reason: { + type: "Text", + props: { name: "reason", label: "Reason", required: false }, + children: [], + }, + f_phone: { type: "Phone", props: { required: false }, children: [] }, + }, + }; + + cy.intercept( + "POST", + `/api/v1/plus/property/${PROPERTY_ID}/form-builder/chat`, + { + statusCode: 200, + headers: { "content-type": "text/event-stream" }, + body: buildSseBody(spec), + }, + ).as("chatTurn"); + + cy.intercept("PUT", `/api/v1/plus/property/${PROPERTY_ID}`, { + statusCode: 200, + body: buildPropertyFixture(), + }).as("savePut"); + + cy.visit(`/properties/${PROPERTY_ID}/forms/${POLICY_KEY}`); + cy.wait("@getProperty"); + + cy.findByPlaceholderText(/tell the builder/i).type( + "Add a reason field between email and phone", + ); + cy.findByRole("button", { name: /^send$/i }).click(); + cy.wait("@chatTurn"); + + cy.findByRole("button", { name: /^save$/i }).click(); + cy.wait("@savePut").then((interception) => { + const action = ( + interception.request.body.privacy_center_config.actions ?? [] + ).find((a: { policy_key?: string }) => a.policy_key === POLICY_KEY); + expect(action.field_order).to.deep.equal(["email", "reason", "phone"]); + // Deprecated key should not be re-emitted on save. + expect(action).not.to.have.property("custom_privacy_request_field_order"); + }); + }); + + // ── Test 3: dropped-features acknowledgement ──────────────────────────────── it("warns and gates save when conditional logic is present", () => { const spec = buildSpec({ f_state: { diff --git a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/FormBuilderPage.tsx b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/FormBuilderPage.tsx index def98b8cc12..874284a1a36 100644 --- a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/FormBuilderPage.tsx +++ b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/FormBuilderPage.tsx @@ -35,6 +35,7 @@ interface ActionShape { cancelButtonText?: string | null; custom_privacy_request_fields?: PcCustomFields; identity_inputs?: Record | null; + field_order?: string[] | null; // eslint-disable-next-line no-underscore-dangle _form_builder_spec?: { spec: JsonRenderSpec; version: number }; } @@ -55,6 +56,7 @@ interface FormBuilderPageProps { actionPolicyKey: string; pcShape: PcCustomFields; identityInputs: Record; + fieldOrder: string[]; richSpec: JsonRenderSpec; }) => Promise; } @@ -127,6 +129,7 @@ export const FormBuilderPage = ({ return synthesizeSpecFromPcShape( action.custom_privacy_request_fields ?? {}, action.identity_inputs, + action.field_order, ); } // No saved fields yet — seed with the standard DSR defaults so the @@ -328,6 +331,7 @@ export const FormBuilderPage = ({ actionPolicyKey, pcShape: result.pcShape, identityInputs: result.identityInputs, + fieldOrder: result.fieldOrder, richSpec: builder.spec, }); // Save just wrote rich + legacy in lockstep, so any prior drift is diff --git a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/mapper.test.ts b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/mapper.test.ts index 7c27d71a15f..e2d438a7b83 100644 --- a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/mapper.test.ts +++ b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/mapper.test.ts @@ -181,6 +181,72 @@ describe("mapSpecToPcShape", () => { expect(result.pcShape.notes.placeholder).toBeUndefined(); }); + it("emits fieldOrder reflecting children order across identity and custom fields", () => { + const spec = buildSpec( + { + f_email: { type: "Email", props: { required: true }, children: [] }, + f_reason: { + type: "Text", + props: { name: "reason", label: "Reason", required: false }, + children: [], + }, + f_name: { type: "Name", props: { required: false }, children: [] }, + f_topics: { + type: "MultiSelect", + props: { + name: "topics", + label: "Topics", + required: false, + options: ["A", "B"], + }, + children: [], + }, + }, + ["f_email", "f_reason", "f_name", "f_topics"], + ); + + const result = mapSpecToPcShape(spec); + + expect(result.errors).toEqual([]); + expect(result.fieldOrder).toEqual(["email", "reason", "name", "topics"]); + expect(result.identityInputs).toEqual({ + email: "required", + name: "optional", + }); + expect(Object.keys(result.pcShape)).toEqual(["reason", "topics"]); + }); + + it("returns an empty fieldOrder when the spec has no children", () => { + const spec = buildSpec({}, []); + const result = mapSpecToPcShape(spec); + expect(result.fieldOrder).toEqual([]); + }); + + it("excludes unknown components from fieldOrder while reporting them as dropped", () => { + const spec = buildSpec( + { + ok: { + type: "Text", + props: { name: "notes", label: "Notes", required: false }, + children: [], + }, + weird: { + type: "Mystery", + props: {}, + children: [], + }, + }, + ["ok", "weird"], + ); + + const result = mapSpecToPcShape(spec); + + expect(result.fieldOrder).toEqual(["notes"]); + expect(result.droppedFeatures.map((d) => d.kind)).toContain( + "unknown_component", + ); + }); + it("translates json-render `visible` into legacy `visible_when` and does not drop it", () => { const spec = buildSpec( { diff --git a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/synthesize.test.ts b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/synthesize.test.ts index 0cbdbca2705..a2c851d92a6 100644 --- a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/synthesize.test.ts +++ b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/__tests__/synthesize.test.ts @@ -52,4 +52,110 @@ describe("synthesizeSpecFromPcShape", () => { expect(back.pcShape.country.field_type).toBe("location"); expect(back.pcShape.contact_method.field_type).toBe("radio"); }); + + it("uses fieldOrder when provided, interleaving identity and custom fields", () => { + const pcShape = { + reason: { label: "Reason", field_type: "text" as const, required: false }, + topics: { + label: "Topics", + field_type: "multiselect" as const, + options: ["A", "B"], + required: false, + }, + }; + const identityInputs = { + email: "required" as const, + name: "optional" as const, + }; + const fieldOrder = ["email", "reason", "name", "topics"]; + + const spec = synthesizeSpecFromPcShape(pcShape, identityInputs, fieldOrder); + + expect(spec.elements.form.children).toEqual([ + "f_email", + "f_reason", + "f_name", + "f_topics", + ]); + const back = mapSpecToPcShape(spec); + expect(back.fieldOrder).toEqual(["email", "reason", "name", "topics"]); + }); + + it("falls back to legacy ordering when fieldOrder is absent", () => { + const pcShape = { + reason: { label: "Reason", field_type: "text" as const, required: false }, + }; + const identityInputs = { + phone: "optional" as const, + email: "required" as const, + name: "optional" as const, + }; + + const spec = synthesizeSpecFromPcShape(pcShape, identityInputs); + + // Canonical legacy order: name → email → phone → customs. + expect(spec.elements.form.children).toEqual([ + "f_name", + "f_email", + "f_phone", + "f_reason", + ]); + }); + + it("appends configured fields missing from fieldOrder using legacy fallback", () => { + const pcShape = { + reason: { label: "Reason", field_type: "text" as const, required: false }, + topics: { + label: "Topics", + field_type: "multiselect" as const, + options: ["A"], + required: false, + }, + }; + const identityInputs = { email: "required" as const }; + const fieldOrder = ["reason", "email"]; // `topics` configured but absent from order + + const spec = synthesizeSpecFromPcShape(pcShape, identityInputs, fieldOrder); + + expect(spec.elements.form.children).toEqual([ + "f_reason", + "f_email", + "f_topics", + ]); + }); + + it("skips unknown keys in fieldOrder rather than crashing on stale data", () => { + const pcShape = { + reason: { label: "Reason", field_type: "text" as const, required: false }, + }; + const identityInputs = { email: "required" as const }; + const fieldOrder = ["email", "ghost_field", "reason"]; + + const spec = synthesizeSpecFromPcShape(pcShape, identityInputs, fieldOrder); + + expect(spec.elements.form.children).toEqual(["f_email", "f_reason"]); + }); + + it("preserves arbitrary order via a synthesize → map round trip", () => { + const pcShape = { + reason: { label: "Reason", field_type: "text" as const, required: false }, + topics: { + label: "Topics", + field_type: "multiselect" as const, + options: ["A"], + required: false, + }, + }; + const identityInputs = { + email: "required" as const, + name: "optional" as const, + phone: "optional" as const, + }; + const fieldOrder = ["topics", "email", "phone", "reason", "name"]; + + const spec = synthesizeSpecFromPcShape(pcShape, identityInputs, fieldOrder); + const back = mapSpecToPcShape(spec); + + expect(back.fieldOrder).toEqual(fieldOrder); + }); }); diff --git a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/mapper.ts b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/mapper.ts index b581f1b0bd3..9667719a5ad 100644 --- a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/mapper.ts +++ b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/mapper.ts @@ -77,6 +77,11 @@ export type ValidationError = export interface MapResult { pcShape: PcCustomFields; identityInputs: Record; + // Unified render order across identity_inputs and pcShape, in the order the + // form builder's children list resolved. Persisted as `field_order` on the + // privacy center action so the public renderer can interleave identity and + // custom fields freely. Empty if the spec has no children. + fieldOrder: string[]; droppedFeatures: DroppedFeature[]; errors: ValidationError[]; } @@ -204,11 +209,12 @@ export function mapSpecToPcShape(spec: JsonRenderSpec): MapResult { const errors: ValidationError[] = []; const pcShape: PcCustomFields = {}; const identityInputs: Record = {}; + const fieldOrder: string[] = []; const root = spec.elements?.[spec.root]; if (!root || root.type !== "Form") { errors.push({ kind: "missing_form_root", rootId: spec.root }); - return { pcShape, identityInputs, droppedFeatures, errors }; + return { pcShape, identityInputs, fieldOrder, droppedFeatures, errors }; } const seenNames: Record = {}; @@ -260,6 +266,7 @@ export function mapSpecToPcShape(spec: JsonRenderSpec): MapResult { } const { required } = validation.data as { required: boolean }; identityInputs[identityKey] = required ? "required" : "optional"; + fieldOrder.push(identityKey); return; } @@ -380,6 +387,7 @@ export function mapSpecToPcShape(spec: JsonRenderSpec): MapResult { } } pcShape[name] = pcField; + fieldOrder.push(name); }); Object.entries(seenNames).forEach(([name, ids]) => { @@ -388,5 +396,5 @@ export function mapSpecToPcShape(spec: JsonRenderSpec): MapResult { } }); - return { pcShape, identityInputs, droppedFeatures, errors }; + return { pcShape, identityInputs, fieldOrder, droppedFeatures, errors }; } diff --git a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/synthesize.ts b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/synthesize.ts index 33ea3a1e9ac..d99703d75e9 100644 --- a/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/synthesize.ts +++ b/clients/admin-ui/src/features/properties/privacy-center-config/form-builder/synthesize.ts @@ -38,118 +38,167 @@ const visibilityToJsonRender = ( return entry; }); +const buildIdentityElement = ( + identityKey: string, + mode: "required" | "optional", +): JsonRenderSpec["elements"][string] => ({ + type: IDENTITY_COMPONENT_FOR_KEY[identityKey], + props: { required: mode === "required" }, + children: [], +}); + +const buildCustomElement = ( + name: string, + field: PcCustomField, +): JsonRenderSpec["elements"][string] => { + const componentType = COMPONENT_FOR_FIELD[field.field_type]; + const props: Record = { + name, + label: field.label, + required: field.required ?? false, + }; + if (field.placeholder !== undefined) { + props.placeholder = field.placeholder; + } + + switch (field.field_type) { + case "text": { + const text = field as PcTextField; + if (text.default_value !== undefined && text.default_value !== null) { + props.default_value = text.default_value; + } + if (text.hidden !== undefined) { + props.hidden = text.hidden; + } + if (text.query_param_key !== undefined && text.query_param_key !== null) { + props.query_param_key = text.query_param_key; + } + break; + } + case "select": { + const sel = field as PcSelectField; + props.options = sel.options; + if (sel.default_value !== undefined && sel.default_value !== null) { + props.default_value = sel.default_value; + } + break; + } + case "radio": { + const rad = field as PcRadioField; + props.options = rad.options; + if (rad.default_value !== undefined && rad.default_value !== null) { + props.default_value = rad.default_value; + } + break; + } + case "multiselect": { + const ms = field as PcMultiSelectField; + props.options = ms.options; + if (ms.default_value !== undefined && ms.default_value !== null) { + props.default_value = ms.default_value; + } + break; + } + case "location": { + const loc = field as PcLocationField; + if (loc.options !== undefined) { + props.options = loc.options; + } + if (loc.ip_geolocation_hint !== undefined) { + props.ip_geolocation_hint = loc.ip_geolocation_hint; + } + break; + } + default: { + const exhaustive: never = field; + throw new Error( + `Unhandled field type: ${(exhaustive as PcCustomField).field_type}`, + ); + } + } + + const element: JsonRenderSpec["elements"][string] = { + type: componentType, + props, + children: [], + }; + if (field.visible_when && field.visible_when.length > 0) { + (element as { visible?: unknown }).visible = visibilityToJsonRender( + field.visible_when, + ); + } + return element; +}; + export function synthesizeSpecFromPcShape( pcShape: PcCustomFields, identityInputs?: Record | null, + fieldOrder?: string[] | null, ): JsonRenderSpec { const elements: JsonRenderSpec["elements"] = { form: { type: "Form", props: {}, children: [] }, }; - const childIds: string[] = []; + const seenKeys = new Set(); + + // When fieldOrder is provided, render keys in that exact sequence so identity + // and custom fields can interleave freely. Any keys configured but missing + // from fieldOrder fall through to the legacy ordering at the end (so a + // hand-edited YAML adding a field without updating field_order still shows it). + if (fieldOrder && fieldOrder.length > 0) { + fieldOrder.forEach((key) => { + if (seenKeys.has(key)) { + return; + } + const elementId = `f_${key}`; + if (key in IDENTITY_COMPONENT_FOR_KEY) { + const mode = identityInputs?.[key]; + if (mode !== "required" && mode !== "optional") { + return; + } + elements[elementId] = buildIdentityElement(key, mode); + childIds.push(elementId); + seenKeys.add(key); + return; + } + const customField = pcShape[key]; + if (customField) { + elements[elementId] = buildCustomElement(key, customField); + childIds.push(elementId); + seenKeys.add(key); + } + // Unknown key (neither identity nor custom): skipped silently. Backend + // validation rejects this case at save time; we don't crash on stale data. + }); + } - // Seed identity fields first, in canonical order, for any that are present. + // Legacy default order for keys not covered by fieldOrder: identity first + // (name → email → phone), then customs in pcShape iteration order. if (identityInputs) { (["name", "email", "phone"] as const).forEach((key) => { + if (seenKeys.has(key)) { + return; + } const mode = identityInputs[key]; if (mode === "required" || mode === "optional") { const elementId = `f_${key}`; + elements[elementId] = buildIdentityElement(key, mode); childIds.push(elementId); - elements[elementId] = { - type: IDENTITY_COMPONENT_FOR_KEY[key], - props: { required: mode === "required" }, - children: [], - }; + seenKeys.add(key); } }); } Object.entries(pcShape).forEach(([name, field]) => { + if (seenKeys.has(name)) { + return; + } const elementId = `f_${name}`; + elements[elementId] = buildCustomElement(name, field); childIds.push(elementId); - - const componentType = COMPONENT_FOR_FIELD[field.field_type]; - const props: Record = { - name, - label: field.label, - required: field.required ?? false, - }; - if (field.placeholder !== undefined) { - props.placeholder = field.placeholder; - } - - switch (field.field_type) { - case "text": { - const text = field as PcTextField; - if (text.default_value !== undefined && text.default_value !== null) { - props.default_value = text.default_value; - } - if (text.hidden !== undefined) { - props.hidden = text.hidden; - } - if ( - text.query_param_key !== undefined && - text.query_param_key !== null - ) { - props.query_param_key = text.query_param_key; - } - break; - } - case "select": { - const sel = field as PcSelectField; - props.options = sel.options; - if (sel.default_value !== undefined && sel.default_value !== null) { - props.default_value = sel.default_value; - } - break; - } - case "radio": { - const rad = field as PcRadioField; - props.options = rad.options; - if (rad.default_value !== undefined && rad.default_value !== null) { - props.default_value = rad.default_value; - } - break; - } - case "multiselect": { - const ms = field as PcMultiSelectField; - props.options = ms.options; - if (ms.default_value !== undefined && ms.default_value !== null) { - props.default_value = ms.default_value; - } - break; - } - case "location": { - const loc = field as PcLocationField; - if (loc.options !== undefined) { - props.options = loc.options; - } - if (loc.ip_geolocation_hint !== undefined) { - props.ip_geolocation_hint = loc.ip_geolocation_hint; - } - break; - } - default: { - const exhaustive: never = field; - throw new Error( - `Unhandled field type: ${(exhaustive as PcCustomField).field_type}`, - ); - } - } - - const element: JsonRenderSpec["elements"][string] = { - type: componentType, - props, - children: [], - }; - if (field.visible_when && field.visible_when.length > 0) { - (element as { visible?: unknown }).visible = visibilityToJsonRender( - field.visible_when, - ); - } - elements[elementId] = element; + seenKeys.add(name); }); - elements.form.children = childIds; + elements.form.children = childIds; return { root: "form", elements }; } diff --git a/clients/admin-ui/src/pages/properties/[id]/forms/[actionPolicyKey].tsx b/clients/admin-ui/src/pages/properties/[id]/forms/[actionPolicyKey].tsx index bec4b813181..d1ae0b01161 100644 --- a/clients/admin-ui/src/pages/properties/[id]/forms/[actionPolicyKey].tsx +++ b/clients/admin-ui/src/pages/properties/[id]/forms/[actionPolicyKey].tsx @@ -35,11 +35,13 @@ const FormBuilderRoute: NextPage = () => { actionPolicyKey: key, pcShape, identityInputs, + fieldOrder, richSpec, }: { actionPolicyKey: string; pcShape: PcCustomFields; identityInputs: MapResult["identityInputs"]; + fieldOrder: MapResult["fieldOrder"]; richSpec: JsonRenderSpec; }) => { if (!property) { @@ -47,22 +49,29 @@ const FormBuilderRoute: NextPage = () => { } const config = property.privacy_center_config ?? { actions: [] }; const existingActions = (config as { actions?: any[] }).actions ?? []; - const actions = existingActions.map((action: any) => - action.policy_key === key - ? { - ...action, - custom_privacy_request_fields: pcShape, - identity_inputs: - Object.keys(identityInputs).length > 0 ? identityInputs : null, - // eslint-disable-next-line no-underscore-dangle - _form_builder_spec: { - version: 1, - spec: richSpec, - updated_at: new Date().toISOString(), - }, - } - : action, - ); + const actions = existingActions.map((action: any) => { + if (action.policy_key !== key) { + return action; + } + // Drop the deprecated custom_privacy_request_field_order; field_order + // supersedes it. Without this, stale legacy ordering can shadow newly + // saved customs after a rename or reorder. + const rest = { ...action }; + delete rest.custom_privacy_request_field_order; + return { + ...rest, + custom_privacy_request_fields: pcShape, + identity_inputs: + Object.keys(identityInputs).length > 0 ? identityInputs : null, + field_order: fieldOrder, + // eslint-disable-next-line no-underscore-dangle + _form_builder_spec: { + version: 1, + spec: richSpec, + updated_at: new Date().toISOString(), + }, + }; + }); // eslint-disable-next-line @typescript-eslint/naming-convention const { id: propertyId, messaging_templates, ...rest } = property as any; await updateProperty({ diff --git a/clients/privacy-center/__tests__/components/modals/buildOrderedFields.test.ts b/clients/privacy-center/__tests__/components/modals/buildOrderedFields.test.ts new file mode 100644 index 00000000000..95e156b1317 --- /dev/null +++ b/clients/privacy-center/__tests__/components/modals/buildOrderedFields.test.ts @@ -0,0 +1,161 @@ +import { + buildOrderedFields, + OrderedField, +} from "~/components/modals/privacy-request-modal/buildOrderedFields"; +import { CustomConfigField } from "~/types/config"; + +const textField = (label: string): CustomConfigField => ({ + label, + field_type: "text", + required: false, +}); + +// Helper that flattens a descriptor into a stable string for comparison. +const stringifyField = (f: OrderedField): string => { + switch (f.kind) { + case "name": + case "email": + case "phone": + return `${f.kind}:${f.mode}`; + case "custom-identity": + case "custom": + return `${f.kind}:${f.key}`; + default: { + const exhaustive: never = f; + throw new Error(`unhandled: ${JSON.stringify(exhaustive)}`); + } + } +}; + +describe("buildOrderedFields", () => { + describe("with field_order present", () => { + it("renders strictly in field_order, interleaving identity and customs", () => { + const result = buildOrderedFields( + { name: "optional", email: "required", phone: "optional" }, + {}, + { reason: textField("Reason"), topics: textField("Topics") }, + ["email", "reason", "name", "topics", "phone"], + ); + + expect(result.map((f) => stringifyField(f))).toEqual([ + "email:required", + "custom:reason", + "name:optional", + "custom:topics", + "phone:optional", + ]); + }); + + it("places custom identity fields via field_order entries", () => { + const result = buildOrderedFields( + { email: "required" }, + { loyalty_id: textField("Loyalty ID") }, + {}, + ["loyalty_id", "email"], + ); + + expect(result.map((f) => stringifyField(f))).toEqual([ + "custom-identity:loyalty_id", + "email:required", + ]); + }); + + it("appends configured fields missing from field_order to the end", () => { + const result = buildOrderedFields( + { email: "required", phone: "optional" }, + {}, + { reason: textField("Reason") }, + ["reason", "email"], // `phone` configured but not listed + ); + + expect(result.map((f) => stringifyField(f))).toEqual([ + "custom:reason", + "email:required", + "phone:optional", + ]); + }); + + it("ignores unknown keys in field_order without crashing", () => { + const result = buildOrderedFields( + { email: "required" }, + {}, + { reason: textField("Reason") }, + ["email", "ghost_field", "reason"], + ); + + expect(result.map((f) => stringifyField(f))).toEqual([ + "email:required", + "custom:reason", + ]); + }); + + it("dedupes repeated keys (defensive against stale data)", () => { + const result = buildOrderedFields( + { email: "required" }, + {}, + { reason: textField("Reason") }, + ["email", "reason", "email"], + ); + + expect(result.map((f) => stringifyField(f))).toEqual([ + "email:required", + "custom:reason", + ]); + }); + }); + + describe("legacy fallback (no field_order)", () => { + it("renders name → email → phone → custom identities → customs", () => { + const result = buildOrderedFields( + { phone: "optional", email: "required", name: "optional" }, + { loyalty_id: textField("Loyalty ID") }, + { reason: textField("Reason"), topics: textField("Topics") }, + ); + + expect(result.map((f) => stringifyField(f))).toEqual([ + "name:optional", + "email:required", + "phone:optional", + "custom-identity:loyalty_id", + "custom:reason", + "custom:topics", + ]); + }); + + it("skips identity fields not configured", () => { + const result = buildOrderedFields( + { email: "required" }, + {}, + { reason: textField("Reason") }, + ); + + expect(result.map((f) => stringifyField(f))).toEqual([ + "email:required", + "custom:reason", + ]); + }); + + it("returns an empty array when no fields are configured", () => { + expect(buildOrderedFields({}, {}, {})).toEqual([]); + }); + + it("treats null / undefined / empty field_order as absent", () => { + const args = [ + { email: "required" }, + {}, + { reason: textField("Reason") }, + ] as const; + const expected = ["email:required", "custom:reason"]; + + expect( + buildOrderedFields(...args, null).map((f) => stringifyField(f)), + ).toEqual(expected); + expect( + buildOrderedFields(...args, undefined).map((f) => stringifyField(f)), + ).toEqual(expected); + expect( + buildOrderedFields(...args, []).map((f) => stringifyField(f)), + ).toEqual(expected); + }); + }); +}); diff --git a/clients/privacy-center/components/modals/privacy-request-modal/PrivacyRequestForm.tsx b/clients/privacy-center/components/modals/privacy-request-modal/PrivacyRequestForm.tsx index 41af5afd061..ac5534b8a29 100644 --- a/clients/privacy-center/components/modals/privacy-request-modal/PrivacyRequestForm.tsx +++ b/clients/privacy-center/components/modals/privacy-request-modal/PrivacyRequestForm.tsx @@ -9,7 +9,7 @@ import { ModalViews } from "~/components/modals/types"; import { PhoneInput } from "~/components/phone-input"; import { CustomConfigField, PrivacyRequestOption } from "~/types/config"; -import usePrivacyRequestForm from "./usePrivacyRequestForm"; +import usePrivacyRequestForm, { OrderedField } from "./usePrivacyRequestForm"; type PrivacyRequestFormProps = { onExit: () => void; @@ -39,13 +39,7 @@ const PrivacyRequestForm = ({ touched, values, isSubmitting, - legacyIdentityFields: { - name: nameInput, - email: emailInput, - phone: phoneInput, - }, - customIdentityFields, - customPrivacyRequestFields, + orderedFields, } = usePrivacyRequestForm({ onExit, action, @@ -59,6 +53,134 @@ const PrivacyRequestForm = ({ return null; } + const buildCustomFieldProps = ( + key: string, + value: string | string[], + fieldConfig: CustomConfigField, + ): CustomFieldRendererProps => { + const sharedProps = { + fieldKey: key, + onBlur: () => handleBlur({ target: { name: key } }), + error: touched[key] && errors[key] ? errors[key] : undefined, + }; + switch (fieldConfig.field_type) { + case "multiselect": + return { + ...fieldConfig, + ...sharedProps, + value: typeof value === "string" ? [value] : value, + onChange: (v: Array) => setFieldValue(key, v), + }; + default: + return { + ...fieldConfig, + ...sharedProps, + value: typeof value === "string" ? value : value?.[0], + onChange: (v: string) => setFieldValue(key, v), + }; + } + }; + + const renderField = (field: OrderedField): React.ReactElement | null => { + if (field.kind === "name") { + return ( + + + + ); + } + if (field.kind === "email") { + return ( + + + + ); + } + if (field.kind === "phone") { + return ( + + setFieldValue("phone", value, true)} + onBlur={handleBlur} + value={values.phone} + /> + + ); + } + if (field.kind !== "custom" && field.kind !== "custom-identity") { + return null; + } + // custom + custom-identity render via the same CustomFieldRenderer pipeline + // they always have. The hidden / visible_when filters apply only to those — + // legacy identity fields above don't honor those props in the existing UX. + const { key, field: item } = field; + if (!item) { + return null; + } + if (item.hidden || !isFieldVisible(item, values)) { + return null; + } + return ( + + + + ); + }; + return ( {action.description} @@ -72,123 +194,7 @@ const PrivacyRequestForm = ({ {paragraph} ))} - {!!nameInput && ( - - - - )} - {!!emailInput && ( - - - - )} - {!!phoneInput && ( - - { - setFieldValue("phone", value, true); - }} - onBlur={handleBlur} - value={values.phone} - /> - - )} - {Object.entries({ - ...customIdentityFields, - ...customPrivacyRequestFields, - }) - .filter(([, field]) => !field?.hidden) - .filter(([, field]) => (field ? isFieldVisible(field, values) : true)) - .map(([key, item]) => { - const customFieldProps = ( - value: string | string[], - fieldConfig: CustomConfigField, - ): CustomFieldRendererProps => { - const sharedProps = { - fieldKey: key, - onBlur: () => handleBlur({ target: { name: key } }), - error: touched[key] && errors[key] ? errors[key] : undefined, - }; - - switch (fieldConfig.field_type) { - case "multiselect": - return { - ...fieldConfig, - ...sharedProps, - value: typeof value === "string" ? [value] : value, - onChange: (v: Array) => { - setFieldValue(key, v); - }, - }; - default: - return { - ...fieldConfig, - ...sharedProps, - value: typeof value === "string" ? value : value?.[0], - onChange: (v: string) => { - setFieldValue(key, v); - }, - }; - } - }; - - return item ? ( - - - - ) : null; - })} + {orderedFields.map(renderField)}