From 4a90675ef50701e350ca4473fa4d5f37e1255bb4 Mon Sep 17 00:00:00 2001 From: Tom Butcher Date: Mon, 9 Nov 2026 00:59:37 +0000 Subject: [PATCH] Refactor ControlPrinter and ObjectForm Components for Improved State Management and Fetch Handling - Updated ControlPrinter to utilize getReferenceId for cleaner ID retrieval of current jobs and filament stocks. - Refactored actions in ControlPrinter to use useMemo for better performance and dependency management. - Enhanced ObjectForm to manage fetch states and updates more effectively, ensuring real-time data consistency during asynchronous operations. - Improved handling of object data updates during fetch to prevent race conditions and ensure accurate state representation. --- .../Production/Printers/ControlPrinter.jsx | 129 +++++++++--------- .../Dashboard/common/ObjectForm.jsx | 91 ++++++++++-- 2 files changed, 150 insertions(+), 70 deletions(-) diff --git a/src/components/Dashboard/Production/Printers/ControlPrinter.jsx b/src/components/Dashboard/Production/Printers/ControlPrinter.jsx index ffcceeb0..229f971d 100644 --- a/src/components/Dashboard/Production/Printers/ControlPrinter.jsx +++ b/src/components/Dashboard/Production/Printers/ControlPrinter.jsx @@ -40,6 +40,7 @@ import PrinterControlButtons from '../../common/PrinterControlButtons.jsx' import PrinterQueue from '../../common/PrinterQueue.jsx' import ListIcon from '../../../Icons/ListIcon.jsx' import { useObjectNavigationTabPage } from '../../hooks/useObjectNavigationTabPage' +import { getReferenceId } from '../../utils/objectPropertyRefs' const log = loglevel.getLogger('ControlPrinter') log.setLevel(config.logLevel) @@ -60,7 +61,7 @@ const ControlPrinter = ({ slicerIntegration = false }) => { { printer: true, job: true, - subjob: true, + subJob: true, gcodeFile: true, queue: true, filamentStock: true, @@ -137,53 +138,63 @@ const ControlPrinter = ({ slicerIntegration = false }) => { ? { showHyperlink: false, objectPropertyProps: { showHyperlink: false } } : {} - const actions = { - edit: () => { - objectFormRef?.current.startEditing() - return false - }, - cancelEdit: () => { - objectFormRef?.current.cancelEditing() - return true - }, - finishEdit: () => { - objectFormRef?.current.handleUpdate() - return true - }, - startQueue: () => { - if (connected == true) { - sendObjectAction(printerId, 'printer', { - type: 'startQueue' - }) - } - return true - }, - pauseJob: () => { - if (connected == true) { - sendObjectAction(printerId, 'printer', { - type: 'pauseJob' - }) - } - return true - }, + const currentJobId = getReferenceId(objectFormState.objectData?.currentJob) + const currentSubJobId = getReferenceId( + objectFormState.objectData?.currentSubJob + ) + const currentFilamentStockId = getReferenceId( + objectFormState.objectData?.currentFilamentStock + ) - resumeJob: () => { - if (connected == true) { - sendObjectAction(printerId, 'printer', { - type: 'resumeJob' - }) + const actions = useMemo( + () => ({ + edit: () => { + objectFormRef?.current.startEditing() + return false + }, + cancelEdit: () => { + objectFormRef?.current.cancelEditing() + return true + }, + finishEdit: () => { + objectFormRef?.current.handleUpdate() + return true + }, + startQueue: () => { + if (connected == true) { + sendObjectAction(printerId, 'printer', { + type: 'startQueue' + }) + } + return true + }, + pauseJob: () => { + if (connected == true) { + sendObjectAction(printerId, 'printer', { + type: 'pauseJob' + }) + } + return true + }, + resumeJob: () => { + if (connected == true) { + sendObjectAction(printerId, 'printer', { + type: 'resumeJob' + }) + } + return true + }, + cancelJob: () => { + if (connected == true) { + sendObjectAction(printerId, 'printer', { + type: 'cancelJob' + }) + } + return true } - return true - }, - cancelJob: () => { - if (connected == true) { - sendObjectAction(printerId, 'printer', { - type: 'cancelJob' - }) - } - return true - } - } + }), + [connected, printerId, sendObjectAction] + ) const sideBarItems = ( @@ -375,7 +386,9 @@ const ControlPrinter = ({ slicerIntegration = false }) => { @@ -479,9 +492,9 @@ const ControlPrinter = ({ slicerIntegration = false }) => { updateCollapseState('job', expanded) } > - {objectFormState.objectData?.currentJob?._id ? ( + {currentJobId ? ( {}} > @@ -518,14 +531,14 @@ const ControlPrinter = ({ slicerIntegration = false }) => { title={'Sub Job'} icon={} collapseKey='subJob' - active={collapseState.subJob} + active={collapseState.subJob ?? collapseState.subjob} onToggle={(expanded) => updateCollapseState('subJob', expanded) } > - {objectFormState.objectData?.currentSubJob?._id ? ( + {currentSubJobId ? ( {}} > @@ -572,9 +585,7 @@ const ControlPrinter = ({ slicerIntegration = false }) => { > @@ -587,13 +598,9 @@ const ControlPrinter = ({ slicerIntegration = false }) => { updateCollapseState('filamentStock', expanded) } > - {objectFormState.objectData?.currentFilamentStock - ?._id ? ( + {currentFilamentStockId ? ( {}} > diff --git a/src/components/Dashboard/common/ObjectForm.jsx b/src/components/Dashboard/common/ObjectForm.jsx index 0b106dda..5a121778 100644 --- a/src/components/Dashboard/common/ObjectForm.jsx +++ b/src/components/Dashboard/common/ObjectForm.jsx @@ -282,6 +282,10 @@ const ObjectForm = forwardRef( const currentHasFilePropertiesRef = useRef(hasFileProperties) const currentFlushOrphanFilesRef = useRef(flushOrphanFiles) const fetchedObjectRef = useRef(null) + const fetchInFlightRef = useRef(false) + const updatesDuringFetchRef = useRef([]) + const hasCompletedFetchRef = useRef(false) + const loadedObjectKeyRef = useRef(null) // Update refs when values change useEffect(() => { @@ -480,12 +484,17 @@ const ObjectForm = forwardRef( .validateFields({ validateOnly: true }) .then(() => { setFormValid(true) + // While viewing, objectData is published from the fetched/live + // record. Publishing the form snapshot here races a print start: + // the snapshot still has no current job and unmounts the job forms. onStateChangeRef.current({ formValid: true, - objectData: mergedObjectData, + ...(isEditingRef.current ? { objectData: mergedObjectData } : {}), editDisabled: getEditDisabled( model, - mergedObjectData, + isEditingRef.current + ? mergedObjectData + : serverObjectData.current || mergedObjectData, userProfile ) }) @@ -494,10 +503,12 @@ const ObjectForm = forwardRef( setFormValid(false) onStateChangeRef.current({ formValid: false, - objectData: mergedObjectData, + ...(isEditingRef.current ? { objectData: mergedObjectData } : {}), editDisabled: getEditDisabled( model, - mergedObjectData, + isEditingRef.current + ? mergedObjectData + : serverObjectData.current || mergedObjectData, userProfile ) }) @@ -531,6 +542,18 @@ const ObjectForm = forwardRef( async (options = {}) => { const objectKey = `${type}:${id}` const skipObjectChildrenValue = options.skipObjectChildrenValue === true + if (loadedObjectKeyRef.current !== objectKey) { + loadedObjectKeyRef.current = objectKey + hasCompletedFetchRef.current = false + } + // A print can assign currentJob before this request returns. Keep that + // live record for the first load; later refetches should take the server. + const liveBeforeFetch = hasCompletedFetchRef.current + ? null + : serverObjectData.current + const updatesDuringFetch = [] + updatesDuringFetchRef.current = updatesDuringFetch + fetchInFlightRef.current = true try { setFetchLoading(true) @@ -538,7 +561,10 @@ const ObjectForm = forwardRef( const data = await fetchObject(id, type) const initialActivities = await fetchObjectActivities(id, type) - if (fetchedObjectRef.current !== objectKey) { + if ( + fetchedObjectRef.current !== objectKey || + updatesDuringFetchRef.current !== updatesDuringFetch + ) { return } @@ -551,16 +577,39 @@ const ObjectForm = forwardRef( releaseEditingState() } - serverObjectData.current = data - - // Calculate and set computed values on initial load + // Calculate and set computed values on initial load. + // Updates that arrived while this request was in flight (a print + // starting and assigning currentJob/currentSubJob) must win over + // the older fetched snapshot. const computedEntries = calculateComputedValues(data, model, { skipObjectChildrenValue }) - const initialFormData = applyComputedEntries(data, computedEntries) + let initialFormData = applyComputedEntries(data, computedEntries) if (skipObjectChildrenValue) { copyObjectChildren(initialFormData, data, model) } + if (liveBeforeFetch) { + const liveId = liveBeforeFetch._id + const fetchedId = initialFormData?._id + if ( + !liveId || + !fetchedId || + String(liveId).toLowerCase() === String(fetchedId).toLowerCase() + ) { + initialFormData = mergeFormData(initialFormData, liveBeforeFetch) + if (!isEditingRef.current) { + copyObjectChildren(initialFormData, liveBeforeFetch, model) + } + } + } + updatesDuringFetch.forEach((update) => { + initialFormData = mergeFormData(initialFormData, update) + if (!isEditingRef.current) { + copyObjectChildren(initialFormData, update, model) + } + }) + hasCompletedFetchRef.current = true + serverObjectData.current = initialFormData setObjectData({ ...initialFormData, _isEditing: isEditingRef.current @@ -573,6 +622,15 @@ const ObjectForm = forwardRef( showError( `Failed to fetch ${getModelMessageLabel(type).toLowerCase()} info.` ) + if (updatesDuringFetchRef.current === updatesDuringFetch) { + setFetchLoading(false) + onStateChangeRef.current({ loading: false }) + } + } finally { + if (updatesDuringFetchRef.current === updatesDuringFetch) { + fetchInFlightRef.current = false + updatesDuringFetchRef.current = [] + } } }, [ @@ -592,6 +650,21 @@ const ObjectForm = forwardRef( // Update event handler const updateObjectEventHandler = useCallback( (value) => { + if ( + fetchInFlightRef.current && + Array.isArray(updatesDuringFetchRef.current) + ) { + updatesDuringFetchRef.current.push(value) + } else if (!isEditingRef.current) { + serverObjectData.current = mergeFormData( + serverObjectData.current, + value + ) + if (serverObjectData.current) { + copyObjectChildren(serverObjectData.current, value, model) + } + } + setObjectData((prev) => { const next = mergeFormData(prev, value) if (!isEditingRef.current) {