From 89279ef0912edfae802c3d840c7f46634cda8ca5 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 23 Jul 2025 18:31:39 +1000 Subject: [PATCH] [bug] Part param edit (#10059) * Fix for BooleanField - Ensure that an "undefined" value reads "false" by default * Tweak part parameter form * Enhanced playwright tests * Better boolean field management * Update src/frontend/src/forms/PartForms.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/frontend/src/components/forms/ApiForm.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/frontend/src/components/forms/ApiForm.tsx | 6 +++ .../components/forms/fields/ApiFormField.tsx | 27 ++++------- .../components/forms/fields/BooleanField.tsx | 45 +++++++++++++++++++ src/frontend/src/components/render/Stock.tsx | 2 - src/frontend/src/forms/PartForms.tsx | 13 +++++- src/frontend/tests/pages/pui_part.spec.ts | 29 +++++++++++- 6 files changed, 99 insertions(+), 23 deletions(-) create mode 100644 src/frontend/src/components/forms/fields/BooleanField.tsx diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index 3efe39057e..f2cf89098d 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -22,6 +22,7 @@ import { } from 'react-hook-form'; import { type NavigateFunction, useNavigate } from 'react-router-dom'; +import { isTrue } from '@lib/functions/Conversion'; import { getDetailUrl } from '@lib/functions/Navigation'; import type { ApiFormFieldSet, @@ -372,6 +373,11 @@ export function ApiForm({ hasFiles = true; } + // Ensure any boolean values are actually boolean + if (field_type === 'boolean') { + value = isTrue(value) || false; + } + // Stringify any JSON objects if (typeof value === 'object') { switch (field_type) { diff --git a/src/frontend/src/components/forms/fields/ApiFormField.tsx b/src/frontend/src/components/forms/fields/ApiFormField.tsx index 50519f31f9..9e0a3bc2e5 100644 --- a/src/frontend/src/components/forms/fields/ApiFormField.tsx +++ b/src/frontend/src/components/forms/fields/ApiFormField.tsx @@ -1,11 +1,11 @@ import { t } from '@lingui/core/macro'; -import { Alert, FileInput, NumberInput, Stack, Switch } from '@mantine/core'; +import { Alert, FileInput, NumberInput, Stack } from '@mantine/core'; import { useId } from '@mantine/hooks'; import { useCallback, useEffect, useMemo } from 'react'; import { type Control, type FieldValues, useController } from 'react-hook-form'; -import { isTrue } from '@lib/functions/Conversion'; import type { ApiFormFieldSet, ApiFormFieldType } from '@lib/types/Forms'; +import { BooleanField } from './BooleanField'; import { ChoiceField } from './ChoiceField'; import DateField from './DateField'; import { DependentField } from './DependentField'; @@ -126,11 +126,6 @@ export function ApiFormField({ return val; }, [definition.field_type, value]); - // Coerce the value to a (stringified) boolean value - const booleanValue: boolean = useMemo(() => { - return isTrue(value); - }, [value]); - // Construct the individual field const fieldInstance = useMemo(() => { switch (fieldDefinition.field_type) { @@ -174,16 +169,13 @@ export function ApiFormField({ ); case 'boolean': return ( - onChange(event.currentTarget.checked)} + { + onChange(value); + }} /> ); case 'date': @@ -273,7 +265,6 @@ export function ApiFormField({ ); } }, [ - booleanValue, control, controller, definition, diff --git a/src/frontend/src/components/forms/fields/BooleanField.tsx b/src/frontend/src/components/forms/fields/BooleanField.tsx new file mode 100644 index 0000000000..a9a5ab4b23 --- /dev/null +++ b/src/frontend/src/components/forms/fields/BooleanField.tsx @@ -0,0 +1,45 @@ +import { isTrue } from '@lib/functions/Conversion'; +import type { ApiFormFieldType } from '@lib/types/Forms'; +import { Switch } from '@mantine/core'; +import { useId } from '@mantine/hooks'; +import { useMemo } from 'react'; +import type { FieldValues, UseControllerReturn } from 'react-hook-form'; + +export function BooleanField({ + controller, + definition, + fieldName, + onChange +}: Readonly<{ + controller: UseControllerReturn; + definition: ApiFormFieldType; + fieldName: string; + onChange: (value: boolean) => void; +}>) { + const fieldId = useId(); + + const { + field, + fieldState: { error } + } = controller; + + const { value } = field; + + // Coerce the value to a (stringified) boolean value + const booleanValue: boolean = useMemo(() => { + return isTrue(value); + }, [value]); + + return ( + onChange(event.currentTarget.checked || false)} + /> + ); +} diff --git a/src/frontend/src/components/render/Stock.tsx b/src/frontend/src/components/render/Stock.tsx index 5dd2af05b6..3384bec3a3 100644 --- a/src/frontend/src/components/render/Stock.tsx +++ b/src/frontend/src/components/render/Stock.tsx @@ -68,8 +68,6 @@ export function RenderStockItem( quantity_string = `${t`Quantity`}: ${instance.quantity}`; } - console.log('item:', instance); - let batch_string = ''; if (!!instance.batch) { diff --git a/src/frontend/src/forms/PartForms.tsx b/src/frontend/src/forms/PartForms.tsx index db7dcba159..818aac1dc6 100644 --- a/src/frontend/src/forms/PartForms.tsx +++ b/src/frontend/src/forms/PartForms.tsx @@ -245,10 +245,19 @@ export function usePartParameterFields({ type: fieldType, field_type: fieldType, choices: fieldType === 'choice' ? choices : undefined, - default: fieldType === 'boolean' ? 'false' : undefined, + default: fieldType === 'boolean' ? false : undefined, adjustValue: (value: any) => { // Coerce boolean value into a string (required by backend) - return value.toString(); + + let v: string = value.toString().trim(); + + if (fieldType === 'boolean') { + if (v.toLowerCase() !== 'true') { + v = 'false'; + } + } + + return v; } }, note: {} diff --git a/src/frontend/tests/pages/pui_part.spec.ts b/src/frontend/tests/pages/pui_part.spec.ts index 8004a1ea8e..4aa243166e 100644 --- a/src/frontend/tests/pages/pui_part.spec.ts +++ b/src/frontend/tests/pages/pui_part.spec.ts @@ -463,14 +463,41 @@ test('Parts - Parameters', async ({ browser }) => { // Select the "polarized" parameter template (should create a "checkbox" field) await page.getByLabel('related-field-template').fill('Polarized'); await page.getByText('Is this part polarized?').click(); + + // Submit with "false" value + await page.getByRole('button', { name: 'Submit' }).click(); + + // Check for the expected values in the table + let row = await getRowFromCell( + await page.getByRole('cell', { name: 'Polarized', exact: true }) + ); + await row.getByRole('cell', { name: 'No', exact: true }).waitFor(); + await row.getByRole('cell', { name: 'allaccess' }).waitFor(); + await row.getByLabel(/row-action-menu-/i).click(); + await page.getByRole('menuitem', { name: 'Edit' }).click(); + + // Toggle false to true await page .locator('label') .filter({ hasText: 'DataParameter Value' }) .locator('div') .first() .click(); + await page.getByRole('button', { name: 'Submit' }).click(); - await page.getByRole('button', { name: 'Cancel' }).click(); + row = await getRowFromCell( + await page.getByRole('cell', { name: 'Polarized', exact: true }) + ); + await row.getByRole('cell', { name: 'Yes', exact: true }).waitFor(); + + await page.getByText('1 - 1 / 1').waitFor(); + + // Finally, delete the parameter + await row.getByLabel(/row-action-menu-/i).click(); + await page.getByRole('menuitem', { name: 'Delete' }).click(); + await page.getByRole('button', { name: 'Delete' }).click(); + + await page.getByText('No records found').first().waitFor(); }); test('Parts - Parameter Filtering', async ({ browser }) => {