From 3383e865cba9a1ee674b0147a645b052d7451b94 Mon Sep 17 00:00:00 2001 From: Oliver Date: Mon, 9 Nov 2026 23:42:12 +1100 Subject: [PATCH] [UI] Fixes for "tags" field (#13233) - Fetch appropriate 'tags' detail in the form request - Additional playwright tests --- src/frontend/lib/types/Forms.tsx | 2 + src/frontend/src/components/forms/ApiForm.tsx | 59 +++++++++++++++---- .../components/forms/fields/ApiFormField.tsx | 1 + .../src/components/forms/fields/TagsField.tsx | 3 +- src/frontend/src/forms/CommonFields.tsx | 4 +- src/frontend/src/pages/build/BuildDetail.tsx | 1 - .../src/pages/company/CompanyDetail.tsx | 1 - .../pages/company/ManufacturerPartDetail.tsx | 1 - .../src/pages/company/SupplierPartDetail.tsx | 1 - src/frontend/src/pages/part/PartDetail.tsx | 1 - .../pages/purchasing/PurchaseOrderDetail.tsx | 1 - .../src/pages/sales/ReturnOrderDetail.tsx | 1 - .../src/pages/sales/SalesOrderDetail.tsx | 1 - .../pages/sales/SalesOrderShipmentDetail.tsx | 1 - src/frontend/src/pages/stock/StockDetail.tsx | 1 - .../src/pages/stock/TransferOrderDetail.tsx | 1 - src/frontend/tests/pages/pui_build.spec.ts | 6 ++ src/frontend/tests/pages/pui_sales.spec.ts | 14 +++++ 18 files changed, 76 insertions(+), 24 deletions(-) diff --git a/src/frontend/lib/types/Forms.tsx b/src/frontend/lib/types/Forms.tsx index 4fdbba6e7e..4a58f3d4c9 100644 --- a/src/frontend/lib/types/Forms.tsx +++ b/src/frontend/lib/types/Forms.tsx @@ -75,6 +75,7 @@ export type ApiFormFieldHeader = { * @param addRow : Callback function to add a new row to a table field * @param headers : Optional definitions of table headers (for table fields) * @param singleFetchFunction : Optional function to fetch a single value for this field (used for fetching the initial value when editing an existing object) + * @param fetchParams : Optional query parameters required on the initial data request to populate this field (e.g. optional serializer fields such as 'tags') */ export type ApiFormFieldType = { label?: string; @@ -138,6 +139,7 @@ export type ApiFormFieldType = { headers?: ApiFormFieldHeader[]; depends_on?: string[]; singleFetchFunction?: (value: any) => Promise | null; + fetchParams?: Record; }; export type ApiFormFieldSet = Record; diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index 1b07395d61..687b4bdec3 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -261,7 +261,28 @@ export function ApiForm({ props.pathParams, props.queryParams ), - [props.url, props.pk, props.pathParams] + [props.url, props.pk, props.pathParams, props.queryParams] + ); + + // Collect any query parameters which individual fields require, + // in order to populate their values from the initial data request + // (e.g. optional serializer fields such as 'tags'). + // This ensures the form fetches the required data independent of where it is launched. + const fetchParams: Record = useMemo(() => { + const params: Record = {}; + + mapFields(props.fields ?? {}, (_path, field) => { + Object.assign(params, field.fetchParams ?? {}); + return undefined; + }); + + return params; + }, [props.fields]); + + // Stable key for the fetch params, for use in query keys + const fetchParamsKey: string = useMemo( + () => new URLSearchParams(fetchParams).toString(), + [fetchParams] ); // Define function to process API response @@ -298,22 +319,26 @@ export function ApiForm({ props.method, props.url, props.pk, - props.pathParams + props.pathParams, + fetchParamsKey ], queryFn: async () => { - return await api.get(url).then((response: any) => { - // Process API response - const fetchedData: any = processFields(fields, response.data); + return await api + .get(url, { params: fetchParams }) + .then((response: any) => { + // Process API response + const fetchedData: any = processFields(fields, response.data); - // Update form values, but only for the fields specified for this form - form.reset(fetchedData); - return fetchedData; - }); + // Update form values, but only for the fields specified for this form + form.reset(fetchedData); + return fetchedData; + }); } }); useEffect(() => { - const _fields: any = props.fields || {}; + // Shallow copy, so that per-key replacements below do not leak into props.fields + const _fields: any = { ...(props.fields || {}) }; const _initialData: any = props.initialData || {}; const _fetchedData: any = initialDataQuery.data || {}; @@ -332,6 +357,17 @@ export function ApiForm({ if (_fetchedData && k in _fetchedData) { _fields[k].value = _fetchedData[k]; } + + // If a field requires specific fetch params, but the server did not return a value, + // exclude the field from the form. Otherwise, submitting an (incorrectly) empty value + // could overwrite existing data on the server (e.g. clearing existing tags) + if ( + _fields[k].fetchParams && + initialDataQuery.data && + _fetchedData[k] === undefined + ) { + _fields[k] = { ..._fields[k], hidden: true, exclude: true }; + } } // Preserve the previous reference for any field whose definition did not @@ -362,7 +398,8 @@ export function ApiForm({ props.method, props.url, props.pk, - props.pathParams + props.pathParams, + fetchParamsKey ] }); initialDataQuery.refetch(); diff --git a/src/frontend/src/components/forms/fields/ApiFormField.tsx b/src/frontend/src/components/forms/fields/ApiFormField.tsx index de4dcd5255..0905349e7c 100644 --- a/src/frontend/src/components/forms/fields/ApiFormField.tsx +++ b/src/frontend/src/components/forms/fields/ApiFormField.tsx @@ -93,6 +93,7 @@ export function ApiFormField({ placeholderWarning: undefined, placeholderWarningCompare: undefined, singleFetchFunction: undefined, + fetchParams: undefined, autoFillFilters: undefined, onValueChange: undefined, adjustFilters: undefined, diff --git a/src/frontend/src/components/forms/fields/TagsField.tsx b/src/frontend/src/components/forms/fields/TagsField.tsx index 667a218f76..db5cc40ae6 100644 --- a/src/frontend/src/components/forms/fields/TagsField.tsx +++ b/src/frontend/src/components/forms/fields/TagsField.tsx @@ -58,7 +58,8 @@ function TagsField({ return { ...definition, allow_null: undefined, - allow_blank: undefined + allow_blank: undefined, + fetchParams: undefined }; }, [definition]); diff --git a/src/frontend/src/forms/CommonFields.tsx b/src/frontend/src/forms/CommonFields.tsx index 988fcb31a4..f4c47c76c9 100644 --- a/src/frontend/src/forms/CommonFields.tsx +++ b/src/frontend/src/forms/CommonFields.tsx @@ -35,7 +35,9 @@ export function TagsField({ field_type: 'tags', label: label ?? t`Tags`, description: description ?? t`Tags for this item`, - placeholder: placeholder ?? t`Select tags` + placeholder: placeholder ?? t`Select tags`, + // 'tags' is an optional serializer field, which must be explicitly requested + fetchParams: { tags: 'true' } }; } diff --git a/src/frontend/src/pages/build/BuildDetail.tsx b/src/frontend/src/pages/build/BuildDetail.tsx index fcbe3e2043..842bce7023 100644 --- a/src/frontend/src/pages/build/BuildDetail.tsx +++ b/src/frontend/src/pages/build/BuildDetail.tsx @@ -413,7 +413,6 @@ export default function BuildDetail() { title: t`Edit Build Order`, modalId: 'edit-build-order', fields: editBuildOrderFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: refreshInstance }); diff --git a/src/frontend/src/pages/company/CompanyDetail.tsx b/src/frontend/src/pages/company/CompanyDetail.tsx index 07100749ed..e17f89f497 100644 --- a/src/frontend/src/pages/company/CompanyDetail.tsx +++ b/src/frontend/src/pages/company/CompanyDetail.tsx @@ -230,7 +230,6 @@ export default function CompanyDetail(props: Readonly) { pk: company?.pk, title: t`Edit Company`, fields: useMemo(() => companyFields({}), []), - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: refreshInstance }); diff --git a/src/frontend/src/pages/company/ManufacturerPartDetail.tsx b/src/frontend/src/pages/company/ManufacturerPartDetail.tsx index e3d42ae150..e9371b68cc 100644 --- a/src/frontend/src/pages/company/ManufacturerPartDetail.tsx +++ b/src/frontend/src/pages/company/ManufacturerPartDetail.tsx @@ -141,7 +141,6 @@ export default function ManufacturerPartDetail() { pk: manufacturerPart?.pk, title: t`Edit Manufacturer Part`, fields: editManufacturerPartFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: refreshInstance }); diff --git a/src/frontend/src/pages/company/SupplierPartDetail.tsx b/src/frontend/src/pages/company/SupplierPartDetail.tsx index 53999b666e..cdcea5f231 100644 --- a/src/frontend/src/pages/company/SupplierPartDetail.tsx +++ b/src/frontend/src/pages/company/SupplierPartDetail.tsx @@ -187,7 +187,6 @@ export default function SupplierPartDetail() { pk: supplierPart?.pk, title: t`Edit Supplier Part`, fields: supplierPartFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: refreshInstance }); diff --git a/src/frontend/src/pages/part/PartDetail.tsx b/src/frontend/src/pages/part/PartDetail.tsx index d1ff84ed9d..64aa3feeab 100644 --- a/src/frontend/src/pages/part/PartDetail.tsx +++ b/src/frontend/src/pages/part/PartDetail.tsx @@ -673,7 +673,6 @@ export default function PartDetail() { pk: part.pk, title: t`Edit Part`, fields: partFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: refreshInstance }); diff --git a/src/frontend/src/pages/purchasing/PurchaseOrderDetail.tsx b/src/frontend/src/pages/purchasing/PurchaseOrderDetail.tsx index b800c45f00..9af4bec9e5 100644 --- a/src/frontend/src/pages/purchasing/PurchaseOrderDetail.tsx +++ b/src/frontend/src/pages/purchasing/PurchaseOrderDetail.tsx @@ -94,7 +94,6 @@ export default function PurchaseOrderDetail() { pk: id, title: t`Edit Purchase Order`, fields: purchaseOrderFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: () => { refreshInstance(); } diff --git a/src/frontend/src/pages/sales/ReturnOrderDetail.tsx b/src/frontend/src/pages/sales/ReturnOrderDetail.tsx index 3ab67e505c..7fa5e1b6e9 100644 --- a/src/frontend/src/pages/sales/ReturnOrderDetail.tsx +++ b/src/frontend/src/pages/sales/ReturnOrderDetail.tsx @@ -205,7 +205,6 @@ export default function ReturnOrderDetail() { pk: order.pk, title: t`Edit Return Order`, fields: returnOrderFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: () => { refreshInstance(); } diff --git a/src/frontend/src/pages/sales/SalesOrderDetail.tsx b/src/frontend/src/pages/sales/SalesOrderDetail.tsx index 83c1ac9c2d..068ea815c3 100644 --- a/src/frontend/src/pages/sales/SalesOrderDetail.tsx +++ b/src/frontend/src/pages/sales/SalesOrderDetail.tsx @@ -110,7 +110,6 @@ export default function SalesOrderDetail() { pk: order.pk, title: t`Edit Sales Order`, fields: salesOrderFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: () => { refreshInstance(); } diff --git a/src/frontend/src/pages/sales/SalesOrderShipmentDetail.tsx b/src/frontend/src/pages/sales/SalesOrderShipmentDetail.tsx index f93e29ef0f..b59e3ef3b2 100644 --- a/src/frontend/src/pages/sales/SalesOrderShipmentDetail.tsx +++ b/src/frontend/src/pages/sales/SalesOrderShipmentDetail.tsx @@ -140,7 +140,6 @@ export default function SalesOrderShipmentDetail() { pk: shipment.pk, fields: editShipmentFields, title: t`Edit Shipment`, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: refreshShipment }); diff --git a/src/frontend/src/pages/stock/StockDetail.tsx b/src/frontend/src/pages/stock/StockDetail.tsx index 3c7710ca5a..27a33e57cc 100644 --- a/src/frontend/src/pages/stock/StockDetail.tsx +++ b/src/frontend/src/pages/stock/StockDetail.tsx @@ -355,7 +355,6 @@ export default function StockDetail() { title: t`Edit Stock Item`, modalId: 'edit-stock-item', fields: editStockItemFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: refreshInstance }); diff --git a/src/frontend/src/pages/stock/TransferOrderDetail.tsx b/src/frontend/src/pages/stock/TransferOrderDetail.tsx index 4d04d050ab..21fc17728a 100644 --- a/src/frontend/src/pages/stock/TransferOrderDetail.tsx +++ b/src/frontend/src/pages/stock/TransferOrderDetail.tsx @@ -229,7 +229,6 @@ export default function TransferOrderDetail() { pk: order.pk, title: t`Edit Transfer Order`, fields: transferOrderFields, - queryParams: new URLSearchParams({ tags: 'true' }), onFormSuccess: () => { refreshInstance(); } diff --git a/src/frontend/tests/pages/pui_build.spec.ts b/src/frontend/tests/pages/pui_build.spec.ts index e5f9a94228..118c1b81bf 100644 --- a/src/frontend/tests/pages/pui_build.spec.ts +++ b/src/frontend/tests/pages/pui_build.spec.ts @@ -166,6 +166,12 @@ test('Build Order - Tags', async ({ browser }) => { }); await expect(tagsField).toBeVisible(); + + // Existing tags are pre-filled in the edit form + const dialog = page.getByRole('dialog'); + await expect(dialog.getByText('Electronics', { exact: true })).toBeVisible(); + await expect(dialog.getByText('PCB Assembly', { exact: true })).toBeVisible(); + await page.getByRole('button', { name: 'Cancel' }).click(); }); diff --git a/src/frontend/tests/pages/pui_sales.spec.ts b/src/frontend/tests/pages/pui_sales.spec.ts index c754c384b1..e6881eeacc 100644 --- a/src/frontend/tests/pages/pui_sales.spec.ts +++ b/src/frontend/tests/pages/pui_sales.spec.ts @@ -306,6 +306,20 @@ test('Sales Orders - Shipments - Tags', async ({ browser }) => { await page.getByRole('button', { name: 'apply-tags-filter' }).click(); await page.getByRole('button', { name: 'filter-drawer-close' }).click(); + // Edit a shipment from the table (not the detail page) + // Existing tags must be pre-filled in the edit form (ref: #13224) + await clickOnRowMenu(page.getByRole('cell', { name: 'SO0008' })); + await page.getByRole('menuitem', { name: 'Edit' }).click(); + + const dialog = page.getByRole('dialog'); + await expect( + dialog.getByRole('combobox', { name: 'tags-field-tags' }) + ).toBeVisible(); + await expect( + dialog.getByText('Requires Payment', { exact: true }) + ).toBeVisible(); + await dialog.getByRole('button', { name: 'Cancel' }).click(); + // Click through to one of the selected shipments await page.getByRole('cell', { name: 'SO0007' }).click(); await page.getByText('Sales Order: SO0007').first().waitFor();