From 7113e04995ff8da32151e601bb9d84b245891ba5 Mon Sep 17 00:00:00 2001 From: Yury Saukou Date: Mon, 27 Jul 2026 15:58:52 +0400 Subject: [PATCH] UIOR-1530 Fix payment terms accordion error state --- .../utils/omitFieldArraysAsyncErrors.js | 77 +++++++- .../utils/omitFieldArraysAsyncErrors.test.js | 183 ++++++++++++++++-- .../POLine/OngoingOrder/OngoingOrderForm.js | 3 +- .../OngoingOrder/OngoingOrderForm.test.js | 24 ++- src/components/POLine/POLineForm.js | 8 +- .../FiscalYearsDistribution.js | 3 +- .../FiscalYearsDistributionTerm.js | 8 +- .../PaymentTermsForm/PaymentTermsForm.js | 2 +- src/components/POLine/const.js | 1 + 9 files changed, 270 insertions(+), 39 deletions(-) diff --git a/src/common/utils/omitFieldArraysAsyncErrors.js b/src/common/utils/omitFieldArraysAsyncErrors.js index de611fdd8..451a07a1d 100644 --- a/src/common/utils/omitFieldArraysAsyncErrors.js +++ b/src/common/utils/omitFieldArraysAsyncErrors.js @@ -6,22 +6,79 @@ import { } from 'lodash'; /* - Final form async validation of field array itself return a Promise instead of resolved value - and set it as "FINAL_FORM/array-error". So form always contain this promise in form's - errors object even if there no actual validation error. + Final form async validation of a FieldArray returns a Promise (not a resolved value) and + stores it under the special "FINAL_FORM/array-error" key. Until the Promise settles, the + form's `errors` object always contains that key, making the form appear invalid even when + there are no real errors yet. + + This utility strips those pending-Promise entries so that downstream consumers (e.g. + accordion error-status indicators) only react to fully-resolved validation results. Issue: https://github.com/final-form/react-final-form-arrays/issues/176 */ -export const omitFieldArraysAsyncErrors = (formErrors, asyncFieldArrays = []) => { - const cloned = cloneDeep(formErrors); - asyncFieldArrays.forEach((field) => { - const arrayFieldAsyncError = get(formErrors, `${field}[${ARRAY_ERROR}].then`); +// Returns true when `value` is a non-null, non-array plain object with no own keys. +const isEmptyObject = (value) => ( + value !== null && + typeof value === 'object' && + !Array.isArray(value) && + Object.keys(value).length === 0 +); + +// Returns true when the ARRAY_ERROR stored at `field` is still a pending Promise +// (i.e. `.then` is a function) rather than a resolved error string. +const hasPendingArrayError = (errors, field) => ( + typeof get(errors, `${field}[${ARRAY_ERROR}].then`) === 'function' +); - if (arrayFieldAsyncError && !get(formErrors, field, []).filter(Boolean).length) { - unset(cloned, field); +// Returns true when the field array contains at least one resolved (truthy) item-level error. +// A pending Promise does NOT count — this guards against treating an in-flight validation +// as an already-resolved failure. +const hasResolvedItemErrors = (errors, field) => ( + get(errors, field, []).some(Boolean) +); + +// Removes all empty-object ancestors of `field` up to (but not including) the root. +// This is necessary because lodash's `unset` removes the leaf key but leaves parent +// objects intact. An empty `{ paymentTerms: {} }` would otherwise still appear as an +// error key to consumers that iterate `Object.keys(errors)`. +const pruneEmptyAncestors = (obj, field) => { + const segments = field.split('.'); + + // Walk from the deepest parent up to (not including) the root key. + // Use reduceRight so the traversal is declarative rather than a manual index loop. + segments.slice(1).reduceRight((_, __, i) => { + const parentPath = segments.slice(0, i + 1).join('.'); + const parentValue = get(obj, parentPath); + + if (isEmptyObject(parentValue)) { + unset(obj, parentPath); } - }); + // reduceRight requires a return value; the accumulator is not used. + + return null; + }, null); +}; + +// Removes a single field's pending-Promise error from the cloned errors object, +// then prunes any ancestor objects that are now empty. +const omitPendingArrayError = (cloned, originalErrors, field) => { + // Only act when the ARRAY_ERROR is a Promise AND there are no resolved item errors. + // If the Promise has already settled to a real error string we must not strip it. + if (!hasPendingArrayError(originalErrors, field) || hasResolvedItemErrors(originalErrors, field)) { + return; + } + + unset(cloned, field); + pruneEmptyAncestors(cloned, field); +}; + +export const omitFieldArraysAsyncErrors = (formErrors, asyncFieldArrays = []) => { + // Deep-clone first so the original `formErrors` reference (owned by final-form) is + // never mutated — final-form treats its state as immutable. + const cloned = cloneDeep(formErrors); + + asyncFieldArrays.forEach((field) => omitPendingArrayError(cloned, formErrors, field)); return cloned; }; diff --git a/src/common/utils/omitFieldArraysAsyncErrors.test.js b/src/common/utils/omitFieldArraysAsyncErrors.test.js index dd60e0beb..1dc5542a1 100644 --- a/src/common/utils/omitFieldArraysAsyncErrors.test.js +++ b/src/common/utils/omitFieldArraysAsyncErrors.test.js @@ -4,46 +4,199 @@ import { omitFieldArraysAsyncErrors } from './omitFieldArraysAsyncErrors'; const ASYNC_FIELD_ARRAY = 'fieldArrayWithAsyncValidation'; +// Helpers to build the error shapes final-form produces. +const pendingArrayErrors = (itemErrors = []) => { + const errors = [...itemErrors]; + + errors[ARRAY_ERROR] = Promise.resolve(null); + + return errors; +}; + +const resolvedArrayErrors = (message = 'Invalid array', itemErrors = []) => { + const errors = [...itemErrors]; + + errors[ARRAY_ERROR] = message; + + return errors; +}; + describe('omitFieldArraysAsyncErrors', () => { + // ─── Existing: top-level (flat) fields ───────────────────────────────────── + it('should omit field-array from form errors if it contains only async error of array itself', () => { - const fieldArrayErrors = []; + const formErrors = { [ASYNC_FIELD_ARRAY]: pendingArrayErrors() }; + const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + + expect(errors).toEqual({}); + }); - fieldArrayErrors[ARRAY_ERROR] = Promise.resolve(null); + it('should keep field-array in form errors object if it contains sync error of array itself', () => { + const formErrors = { [ASYNC_FIELD_ARRAY]: resolvedArrayErrors() }; + const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + expect(ASYNC_FIELD_ARRAY in errors).toBeTruthy(); + }); + + it('should keep field-array in form errors object if it contains its fields\' errors', () => { + const formErrors = { [ASYNC_FIELD_ARRAY]: pendingArrayErrors(['Test field is invalid']) }; + const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + + expect(ASYNC_FIELD_ARRAY in errors).toBeTruthy(); + }); + + // ─── Nested path: ancestor cleanup ───────────────────────────────────────── + + it('should remove the nested field and prune its now-empty parent when async error is pending', () => { + // Mirrors the real-world case: paymentTerms.fiscalYearDistributions has a pending Promise. + // After omitting the leaf, { paymentTerms: {} } must not remain — that would incorrectly + // mark the paymentTerms accordion as having an error. const formErrors = { - [ASYNC_FIELD_ARRAY]: fieldArrayErrors, + paymentTerms: { + fiscalYearDistributions: pendingArrayErrors(), + }, }; - const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + const errors = omitFieldArraysAsyncErrors(formErrors, ['paymentTerms.fiscalYearDistributions']); expect(errors).toEqual({}); }); - it('should keep field-array in form errors object if it contains sync error of array itself', () => { - const fieldArrayErrors = []; + it('should remove only the leaf field when the parent still has other resolved errors', () => { + // paymentTerms.totalPrice has a real error — the parent must survive after + // fiscalYearDistributions (pending Promise) is pruned. + const formErrors = { + paymentTerms: { + totalPrice: 'Required', + fiscalYearDistributions: pendingArrayErrors(), + }, + }; - fieldArrayErrors[ARRAY_ERROR] = 'Invalid array'; + const errors = omitFieldArraysAsyncErrors(formErrors, ['paymentTerms.fiscalYearDistributions']); + expect(errors).toEqual({ paymentTerms: { totalPrice: 'Required' } }); + }); + + it('should keep nested field when the pending Promise co-exists with resolved item errors', () => { const formErrors = { - [ASYNC_FIELD_ARRAY]: fieldArrayErrors, + paymentTerms: { + fiscalYearDistributions: pendingArrayErrors([undefined, 'Fund is required']), + }, }; - const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + const errors = omitFieldArraysAsyncErrors(formErrors, ['paymentTerms.fiscalYearDistributions']); - expect(ASYNC_FIELD_ARRAY in errors).toBeTruthy(); + expect('fiscalYearDistributions' in errors.paymentTerms).toBeTruthy(); }); - it('should keep field-array in form errors object if it contains its fields\' errors', () => { - const fieldArrayErrors = ['Test field is invalid']; + it('should keep nested field when the array error is a resolved string (not a Promise)', () => { + const formErrors = { + paymentTerms: { + fiscalYearDistributions: resolvedArrayErrors('At least 2 fiscal years are required'), + }, + }; + + const errors = omitFieldArraysAsyncErrors(formErrors, ['paymentTerms.fiscalYearDistributions']); - fieldArrayErrors[ARRAY_ERROR] = Promise.resolve(null); + expect('fiscalYearDistributions' in errors.paymentTerms).toBeTruthy(); + }); + + // ─── Deep nesting (3+ levels) ─────────────────────────────────────────────── + + it('should cascade-prune all empty ancestors for a 3-level deep path', () => { + const formErrors = { + a: { + b: { + c: pendingArrayErrors(), + }, + }, + }; + + const errors = omitFieldArraysAsyncErrors(formErrors, ['a.b.c']); + + expect(errors).toEqual({}); + }); + it('should stop pruning when an intermediate ancestor still has sibling keys', () => { + // After removing a.b.c, a.b still has d — so only a.b.c is removed. const formErrors = { - [ASYNC_FIELD_ARRAY]: fieldArrayErrors, + a: { + b: { + c: pendingArrayErrors(), + d: 'Some error', + }, + }, }; + const errors = omitFieldArraysAsyncErrors(formErrors, ['a.b.c']); + + expect(errors).toEqual({ a: { b: { d: 'Some error' } } }); + }); + + // ─── Multiple async field arrays ──────────────────────────────────────────── + + it('should omit all pending-Promise fields when multiple async arrays are registered', () => { + const formErrors = { + fundDistribution: pendingArrayErrors(), + paymentTerms: { + fiscalYearDistributions: pendingArrayErrors(), + }, + }; + + const errors = omitFieldArraysAsyncErrors(formErrors, [ + 'fundDistribution', + 'paymentTerms.fiscalYearDistributions', + ]); + + expect(errors).toEqual({}); + }); + + it('should omit only the pending field when multiple arrays are registered but one has resolved', () => { + const formErrors = { + fundDistribution: resolvedArrayErrors('Required'), + paymentTerms: { + fiscalYearDistributions: pendingArrayErrors(), + }, + }; + + const errors = omitFieldArraysAsyncErrors(formErrors, [ + 'fundDistribution', + 'paymentTerms.fiscalYearDistributions', + ]); + + // The resolved error on fundDistribution must survive; paymentTerms must be fully pruned. + // Note: cloneDeep strips Symbol keys, so ARRAY_ERROR content is not checked here — only + // field presence is within this function's contract. + expect('fundDistribution' in errors).toBeTruthy(); + expect('paymentTerms' in errors).toBeFalsy(); + }); + + // ─── Edge cases ───────────────────────────────────────────────────────────── + + it('should not mutate the original formErrors object', () => { + const fieldErrors = pendingArrayErrors(); + const formErrors = { [ASYNC_FIELD_ARRAY]: fieldErrors }; + const frozen = JSON.parse(JSON.stringify({ [ASYNC_FIELD_ARRAY]: 'snapshot' })); + + omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); + + // The original reference must be untouched (final-form owns it). + expect(ASYNC_FIELD_ARRAY in formErrors).toBeTruthy(); + expect(frozen[ASYNC_FIELD_ARRAY]).toBe('snapshot'); + }); + + it('should return a clone of formErrors unchanged when asyncFieldArrays is empty', () => { + const formErrors = { someField: 'error' }; + const errors = omitFieldArraysAsyncErrors(formErrors, []); + + expect(errors).toEqual(formErrors); + expect(errors).not.toBe(formErrors); + }); + + it('should return a clone unchanged when the field is not present in formErrors', () => { + const formErrors = { otherField: 'error' }; const errors = omitFieldArraysAsyncErrors(formErrors, [ASYNC_FIELD_ARRAY]); - expect(ASYNC_FIELD_ARRAY in errors).toBeTruthy(); + expect(errors).toEqual(formErrors); }); }); diff --git a/src/components/POLine/OngoingOrder/OngoingOrderForm.js b/src/components/POLine/OngoingOrder/OngoingOrderForm.js index 746349835..caae8c93e 100644 --- a/src/components/POLine/OngoingOrder/OngoingOrderForm.js +++ b/src/components/POLine/OngoingOrder/OngoingOrderForm.js @@ -20,6 +20,7 @@ import { import { POL_FORM_FIELDS } from '../../../common/constants'; import { isWorkflowStatusNotPending } from '../../PurchaseOrder/util'; +import calculateEstimatedPrice from '../calculateEstimatedPrice'; const OngoingOrderForm = ({ hiddenFields = {}, @@ -38,7 +39,7 @@ const OngoingOrderForm = ({ change(POL_FORM_FIELDS.multiYearPayment, value); if (value) { - const poLineEstimatedPrice = getState().values?.cost?.poLineEstimatedPrice || 0; + const poLineEstimatedPrice = calculateEstimatedPrice(getState().values); change(`${POL_FORM_FIELDS.paymentTerms}.totalPrice`, poLineEstimatedPrice); } else { diff --git a/src/components/POLine/OngoingOrder/OngoingOrderForm.test.js b/src/components/POLine/OngoingOrder/OngoingOrderForm.test.js index 3ec2017df..9f5385687 100644 --- a/src/components/POLine/OngoingOrder/OngoingOrderForm.test.js +++ b/src/components/POLine/OngoingOrder/OngoingOrderForm.test.js @@ -1,7 +1,11 @@ import { useForm } from 'react-final-form'; import { MemoryRouter } from 'react-router-dom'; -import { render, screen } from '@folio/jest-config-stripes/testing-library/react'; +import { + act, + render, + screen, +} from '@folio/jest-config-stripes/testing-library/react'; import userEvent from '@folio/jest-config-stripes/testing-library/user-event'; import stripesFinalForm from '@folio/stripes/final-form'; @@ -58,14 +62,22 @@ describe('OngoingOrderForm', () => { useForm.mockReturnValue({ change, - getState: jest.fn(() => ({ values: { cost: { poLineEstimatedPrice: 245 } } })), + getState: jest.fn(() => ({ + values: { + cost: { + listUnitPrice: 245, + quantityPhysical: 1, + currency: 'USD', + }, + }, + })), }); - renderOngoingOrderForm({ - initialValues: { cost: { poLineEstimatedPrice: 245 } }, - }); + renderOngoingOrderForm(); - await userEvent.click(screen.getByRole('checkbox')); + await act(async () => { + await userEvent.click(screen.getByRole('checkbox')); + }); expect(change).toHaveBeenCalledWith('multiYearPayment', true); expect(change).toHaveBeenCalledWith('paymentTerms.totalPrice', 245); diff --git a/src/components/POLine/POLineForm.js b/src/components/POLine/POLineForm.js index 2276982c3..c9bdf152d 100644 --- a/src/components/POLine/POLineForm.js +++ b/src/components/POLine/POLineForm.js @@ -11,7 +11,10 @@ import { useRef, useState, } from 'react'; -import { Field } from 'react-final-form'; +import { + Field, + useFormState, +} from 'react-final-form'; import { FormattedMessage } from 'react-intl'; import { useHistory } from 'react-router-dom'; @@ -139,6 +142,8 @@ function POLineForm({ }) { const history = useHistory(); + const { errors: formErrors } = useFormState(); + const [hiddenFields, setHiddenFields] = useState({}); const { validateFundDistributionTotal } = useFundDistributionValidation(formValues); @@ -352,7 +357,6 @@ function POLineForm({ ); }; - const formErrors = form.getState()?.errors; const errors = useMemo(() => ( omitFieldArraysAsyncErrors(formErrors, [ POL_FORM_FIELDS.fundDistribution, diff --git a/src/components/POLine/PaymentTerms/PaymentTermsForm/FiscalYearsDistribution/FiscalYearsDistribution.js b/src/components/POLine/PaymentTerms/PaymentTermsForm/FiscalYearsDistribution/FiscalYearsDistribution.js index cae392b60..92f13d7bb 100644 --- a/src/components/POLine/PaymentTerms/PaymentTermsForm/FiscalYearsDistribution/FiscalYearsDistribution.js +++ b/src/components/POLine/PaymentTerms/PaymentTermsForm/FiscalYearsDistribution/FiscalYearsDistribution.js @@ -59,7 +59,7 @@ export const FiscalYearsDistribution = ({ const handleRemoveFiscalYear = () => { onRemoveFiscalYear(index, fields); }; - const showRemoveButton = fields.length - 1 === index; // Only show remove button for the last fiscal year distribution + const showRemoveButton = (fields.length - 1 === index) && !isNonInteractive; // Only show remove button for the last fiscal year distribution and when the form is not in non-interactive mode const fiscalYearId = fields.value[index].fiscalYearId; const fundDistributions = fields.value[index].fundDistributions || []; const label = intl.formatMessage( @@ -86,7 +86,6 @@ export const FiscalYearsDistribution = ({ { + fields.push({ distributionType: FUND_DISTR_TYPE.amount, value: 0 }); +}; + export const FiscalYearsDistributionTerm = ({ amounts, currency, @@ -42,7 +47,6 @@ export const FiscalYearsDistributionTerm = ({ const intl = useIntl(); const { - onAdd: onAddFund, onChangeToAmount, onChangeToPercent, onRemove: onRemoveFund, @@ -99,7 +103,7 @@ FiscalYearsDistributionTerm.propTypes = { fiscalYearId: PropTypes.string, fundDistributions: PropTypes.arrayOf(PropTypes.object).isRequired, funds: PropTypes.arrayOf(PropTypes.object).isRequired, - label: PropTypes.string.isRequired, + label: PropTypes.node.isRequired, name: PropTypes.string.isRequired, onExpenseClassChange: PropTypes.func.isRequired, onRemoveFiscalYear: PropTypes.func.isRequired, diff --git a/src/components/POLine/PaymentTerms/PaymentTermsForm/PaymentTermsForm.js b/src/components/POLine/PaymentTerms/PaymentTermsForm/PaymentTermsForm.js index eaa986f7b..07a28e077 100644 --- a/src/components/POLine/PaymentTerms/PaymentTermsForm/PaymentTermsForm.js +++ b/src/components/POLine/PaymentTerms/PaymentTermsForm/PaymentTermsForm.js @@ -182,7 +182,7 @@ export const PaymentTermsForm = ({ } name={`${rootFieldName}.totalPrice`} required={isRequired} diff --git a/src/components/POLine/const.js b/src/components/POLine/const.js index b3b94bf5d..0c2d653a1 100644 --- a/src/components/POLine/const.js +++ b/src/components/POLine/const.js @@ -41,6 +41,7 @@ export const MAP_FIELD_ACCORDION = { orderFormat: ACCORDION_ID.lineDetails, other: ACCORDION_ID.other, paymentTerms: ACCORDION_ID.paymentTerms, + 'paymentTerms.fiscalYearDistributions-error': ACCORDION_ID.paymentTerms, physical: ACCORDION_ID.physical, poLineNumber: ACCORDION_ID.lineDetails, publicationDate: ACCORDION_ID.itemDetails,