From cf936f63f06811d680b232f4768199cc643f3f87 Mon Sep 17 00:00:00 2001 From: Steve Cassidy Date: Wed, 27 Nov 2024 14:49:23 +1100 Subject: [PATCH 1/5] Make autoincrementer a popup and trigger when we don't have a range Signed-off-by: Steve Cassidy --- .../components/autoincrement/edit-form.tsx | 264 ++++++++++-------- .../notebook/settings/auto_incrementers.tsx | 123 ++++---- app/src/gui/fields/BasicAutoIncrementer.tsx | 235 +++++----------- 3 files changed, 274 insertions(+), 348 deletions(-) diff --git a/app/src/gui/components/autoincrement/edit-form.tsx b/app/src/gui/components/autoincrement/edit-form.tsx index be00976e1..190380dca 100644 --- a/app/src/gui/components/autoincrement/edit-form.tsx +++ b/app/src/gui/components/autoincrement/edit-form.tsx @@ -18,7 +18,7 @@ * TODO */ -import React from 'react'; +import React, {useContext, useEffect, useState} from 'react'; import {Formik, Form, Field, yupToFormErrors} from 'formik'; import { ButtonGroup, @@ -28,6 +28,12 @@ import { LinearProgress, Grid, Divider, + Dialog, + DialogTitle, + Typography, + IconButton, + DialogContent, + DialogActions, } from '@mui/material'; import AddIcon from '@mui/icons-material/Add'; import {TextField} from 'formik-mui'; @@ -41,12 +47,16 @@ import { setLocalAutoincrementRangesForField, createNewAutoincrementRange, } from '../../../local-data/autoincrement'; +import CloseIcon from '@mui/icons-material/Close'; +import {all} from 'ol/events/condition'; interface Props { project_id: ProjectID; form_id: string; field_id: string; label: string; + open: boolean; + handleClose: () => void; } interface State { @@ -59,97 +69,64 @@ const FORM_SCHEMA = yup.object().shape({ stop: yup.number().required().positive().integer(), }); -export default class BasicAutoIncrementer extends React.Component< - Props, - State -> { - constructor(props: Props) { - super(props); - this.state = { - ranges: null, - ranges_initialised: false, - }; - } +export const AutoIncrementEditForm = ({ + project_id, + form_id, + field_id, + label, + open, + handleClose, +}: Props) => { + const [ranges, setRanges] = useState(null); + const [rangesInitialised, setRangesInitialised] = useState(false); + const {dispatch} = useContext(store); - async ensure_ranges() { - if (this.state.ranges === null) { - const ranges = await getLocalAutoincrementRangesForField( - this.props.project_id, - this.props.form_id, - this.props.field_id + const ensureRanges = async () => { + if (ranges === null) { + const fetchedRanges = await getLocalAutoincrementRangesForField( + project_id, + form_id, + field_id ); - this.setState({ranges: ranges}); + setRanges(fetchedRanges); } - } + }; - async add_new_range() { - await this.ensure_ranges(); - const ranges = [ - ...((this.state.ranges || []) as LocalAutoIncrementRange[]), - ]; - ranges.push(createNewAutoincrementRange(0, 0)); - this.setState({ranges: ranges}); - } + const addNewRange = async () => { + await ensureRanges(); + const updatedRanges = [...(ranges || [])]; + updatedRanges.push(createNewAutoincrementRange(0, 0)); + setRanges(updatedRanges); + }; - cloned_ranges(): LocalAutoIncrementRange[] | null { - if (this.state.ranges === null) { - return null; - } - // react requires us to make deep copied if we want to modify state... - const ranges = [] as LocalAutoIncrementRange[]; - for (const old_range of this.state.ranges) { - // This assumes we don't need to go another level down to deep clone - const new_range = {...old_range}; - ranges.push(new_range); - } - return ranges; - } + const clonedRanges = (): LocalAutoIncrementRange[] | null => { + if (ranges === null) return null; + return ranges.map(range => ({...range})); + }; - render_ranges() { - const ranges = this.cloned_ranges(); - if (ranges === null || ranges.length === 0) { - return ( - - No ranges allocated yet. - - ); - } - return ( -
- {ranges.map((range, range_index) => { - return this.render_range(range, range_index, ranges); - })} -
- ); - } - - async update_ranges(ranges: LocalAutoIncrementRange[]) { - const {project_id, form_id, field_id} = this.props; + const updateRanges = async (newRanges: LocalAutoIncrementRange[]) => { try { await setLocalAutoincrementRangesForField( project_id, form_id, field_id, - ranges - ).then(() => { - (this.context as any).dispatch({ - type: ActionType.ADD_ALERT, - payload: { - message: 'Range successfully updated', - severity: 'success', - }, - }); + newRanges + ); + dispatch({ + type: ActionType.ADD_ALERT, + payload: { + message: 'Range successfully updated', + severity: 'success', + }, }); - let ranges_initialised = false; - for (const range of ranges) { - if (range.using || range.fully_used) { - ranges_initialised = true; - break; - } - } - this.setState({ranges: ranges, ranges_initialised: ranges_initialised}); + + const isInitialised = newRanges.some( + range => range.using || range.fully_used + ); + setRanges(newRanges); + setRangesInitialised(isInitialised); } catch (err: any) { - (this.context as any).dispatch({ + dispatch({ type: ActionType.ADD_ALERT, payload: { message: err.toString(), @@ -157,13 +134,13 @@ export default class BasicAutoIncrementer extends React.Component< }, }); } - } + }; - render_range( + const renderRange = ( range: LocalAutoIncrementRange, - range_index: number, + rangeIndex: number, ranges: LocalAutoIncrementRange[] - ) { + ) => { const range_count = ranges.length; const start_props = { id: 'start', @@ -186,7 +163,7 @@ export default class BasicAutoIncrementer extends React.Component< return ( { range.start = values.start; range.stop = values.stop; - await this.update_ranges(ranges).then(() => { + await updateRanges(ranges).then(() => { setSubmitting(false); }); }} @@ -246,7 +223,7 @@ export default class BasicAutoIncrementer extends React.Component< onClick={async () => { range.fully_used = true; - await this.update_ranges(ranges); + await updateRanges(ranges); }} > Disable range @@ -255,13 +232,11 @@ export default class BasicAutoIncrementer extends React.Component< - +
+ {all_used && ( + + All ranges are fully used. Please add a new range. + + )} + {currentRanges.map((range, index) => + renderRange(range, index, currentRanges) + )} +
); - } -} -BasicAutoIncrementer.contextType = store; + }; + + useEffect(() => { + ensureRanges(); + }, []); + + return ( + + Edit Settings for {label} + + + + + + This form uses an auto-increment field to generate new identifiers for + each record. Here you can set a range of numbers to use on this + device. + + + Set a start and end for the range, numbers will be allocated in order + until used up. You must add at least one range. If there is more than + one range, the ranges will be used in order. + + + + {renderRanges()} + + + + + + + + ); +}; diff --git a/app/src/gui/components/notebook/settings/auto_incrementers.tsx b/app/src/gui/components/notebook/settings/auto_incrementers.tsx index 797a3aaf5..2da0fd50d 100644 --- a/app/src/gui/components/notebook/settings/auto_incrementers.tsx +++ b/app/src/gui/components/notebook/settings/auto_incrementers.tsx @@ -1,9 +1,9 @@ -import React, {useEffect} from 'react'; -import {Box, Grid, Typography, Paper, Alert} from '@mui/material'; +import React, {useEffect, useState} from 'react'; +import {Box, Grid, Typography, Paper, Alert, Button} from '@mui/material'; import {ProjectInformation, ProjectUIModel} from '@faims3/data-model'; import {getAutoincrementReferencesForProject} from '../../../../local-data/autoincrement'; import {AutoIncrementReference} from '@faims3/data-model'; -import AutoIncrementEditForm from '../../autoincrement/edit-form'; +import {AutoIncrementEditForm} from '../../autoincrement/edit-form'; import {logError} from '../../../../logging'; interface AutoIncrementerSettingsListProps { @@ -11,21 +11,11 @@ interface AutoIncrementerSettingsListProps { uiSpec: ProjectUIModel; } -function get_form(section_id: string, uiSpec: ProjectUIModel) { - let form = ''; - uiSpec.visible_types.map(viewset => - uiSpec.viewsets[viewset].views.includes(section_id) - ? (form = uiSpec.viewsets[viewset].label ?? viewset) - : viewset - ); - return form; -} export default function AutoIncrementerSettingsList( props: AutoIncrementerSettingsListProps ) { - const [references, setReferences] = React.useState( - [] as AutoIncrementReference[] - ); + const [open, setOpen] = useState(false); + const [references, setReferences] = useState([] as AutoIncrementReference[]); useEffect(() => { getAutoincrementReferencesForProject(props.project_info.project_id) .then(refs => { @@ -35,62 +25,57 @@ export default function AutoIncrementerSettingsList( }, [props.project_info.project_id]); return ( - + <> {references.length === 0 ? ( - - - - Edit Allocations - - - This project has no Auto-Incrementers - - - + <> ) : ( - '' + + + Edit auto-incrementers + + + View and modify the range settings for auto-incrementers. These are + used to generate unique identifiers for records within a defined + range. + + + {references.map(ai => { + const label = ai.label ?? ''; + return ( + + + setOpen(false)} + /> + + + + ); + })} + )} - {references.map(ai => { - // display form section label for user to fill Auto correctly - const section = - props.uiSpec['views'][ai.form_id] !== undefined - ? (props.uiSpec['views'][ai.form_id]['label'] ?? ai.form_id) - : ai.form_id; - const label = - get_form(ai.form_id, props.uiSpec) + - ' <' + - section + - '> ' + - (ai.label ?? ''); - return ( - - - - Edit Allocations for {label} - - - The allocated range will be ≥ the start value and < the - stop value. e.g., a range allocation of start:1, stop:5 will - generate hrids in the range (1,2,3,4). There must always be a - least one range to ensure that new IDs can be generated. - - - - - ); - })} - + ); } diff --git a/app/src/gui/fields/BasicAutoIncrementer.tsx b/app/src/gui/fields/BasicAutoIncrementer.tsx index 8cc0da06d..999fc0ffb 100644 --- a/app/src/gui/fields/BasicAutoIncrementer.tsx +++ b/app/src/gui/fields/BasicAutoIncrementer.tsx @@ -15,29 +15,20 @@ * * Filename: BasicAutoIncrementer.tsx * Description: - * TODO + * Implements an auto-incrementer field that is hidden but provides + * a value that can be used in templated string fields. */ -import React from 'react'; import Input from '@mui/material/Input'; import {FieldProps} from 'formik'; -import {ActionType} from '../../context/actions'; -import {store} from '../../context/store'; +import {useEffect, useState} from 'react'; import { getLocalAutoincrementStateForField, setLocalAutoincrementStateForField, } from '../../local-data/autoincrement'; -import { - Grid, - Button, - Dialog, - DialogTitle, - DialogContent, - DialogContentText, - DialogActions, -} from '@mui/material'; -import {NOTEBOOK_NAME, NOTEBOOK_NAME_CAPITALIZED} from '../../buildconfig'; +import {AutoIncrementEditForm} from '../components/autoincrement/edit-form'; + interface Props { num_digits: number; // This could be dropped depending on how multi-stage forms are configured @@ -45,92 +36,24 @@ interface Props { label?: string; } -interface State { - has_run: boolean; - is_ranger: boolean; - label: string; -} - -function AddRangeDialog() { - const [open, setOpen] = React.useState(false); - return ( - - - setOpen(false)} - aria-labelledby="alert-dialog-title" - aria-describedby="alert-dialog-description" - > - Information - - - {`Go to ${NOTEBOOK_NAME_CAPITALIZED} > Settings Tab > Edit Allocations to Add New Range`}{' '} -
- - - - - - - - ); -} - -export class BasicAutoIncrementer extends React.Component< - FieldProps & Props, - State -> { - constructor(props: FieldProps & Props) { - super(props); - const label = - this.props.label !== '' && this.props.label !== undefined - ? this.props.label - : this.props.form_id; - this.state = { - has_run: false, - is_ranger: true, - label: label ?? this.props.form_id, - }; - } +export const BasicAutoIncrementer = (props: FieldProps & Props) => { + const [showAutoIncrementEditForm, setShowAutoIncrementEditForm] = + useState(false); - async get_id(): Promise { - const project_id = this.props.form.values['_project_id']; - const form_id = this.props.form_id; - const field_id = this.props.field.name; + const get_id = async (): Promise => { + const project_id = props.form.values['_project_id']; + const form_id = props.form_id; + const field_id = props.field.name; const local_state = await getLocalAutoincrementStateForField( project_id, form_id, field_id ); - console.debug( - `local_auto_inc for ${project_id} ${form_id} ${field_id} is`, - local_state - ); if (local_state.last_used_id === null && local_state.ranges.length === 0) { - // We have no range allocations, block - // TODO: add link to range allocation - (this.context as any).dispatch({ - type: ActionType.ADD_ALERT, - payload: { - message: `No ranges exist for this ${NOTEBOOK_NAME} yet. Go to the ${NOTEBOOK_NAME} Settings tab to add/edit ranges.`, - severity: 'error', - }, - }); - this.setState({...this.state, is_ranger: false}); - return null; + setShowAutoIncrementEditForm(true); } + if (local_state.last_used_id === null) { - console.debug('local_auto_inc starting with clean slate'); // We've got a clean slate with ranges allocated, start allocating ids const new_id = local_state.ranges[0].start; local_state.ranges[0].using = true; @@ -138,12 +61,12 @@ export class BasicAutoIncrementer extends React.Component< await setLocalAutoincrementStateForField(local_state); return new_id; } + // We're now using the allocated ranges, find where we've up to: // If we're using a range, find it for (const range of local_state.ranges) { if (range.using) { if (local_state.last_used_id + 1 < range.stop) { - console.debug('local_auto_inc using existing range', range); const next_id = local_state.last_used_id + 1; local_state.last_used_id = next_id; await setLocalAutoincrementStateForField(local_state); @@ -151,100 +74,86 @@ export class BasicAutoIncrementer extends React.Component< } range.fully_used = true; range.using = false; - console.debug('local_auto_inc finished with range', range); } } + // find a new range to use for (const range of local_state.ranges) { + console.debug('checking range', range); if (!range.fully_used) { const next_id = range.start; range.using = true; local_state.last_used_id = next_id; - console.debug('local_auto_inc staring with range', range); await setLocalAutoincrementStateForField(local_state); return next_id; } } - // we've got no new ranges to use, either we block, or use the highest range - // as a starting point - // TODO: Add blocking logic - let max_stop = local_state.last_used_id; - for (const range of local_state.ranges) { - if (range.stop > max_stop) { - max_stop = range.stop; - } - } - if (max_stop === local_state.last_used_id) { - max_stop = max_stop + 1; - } - local_state.last_used_id = max_stop; - await setLocalAutoincrementStateForField(local_state); - console.debug('local_auto_inc using overrun', local_state); - return max_stop; - } + // we've got no new ranges to use, ask the user to allocate more + + setShowAutoIncrementEditForm(true); + return null; + }; - async compute_id(num_digits: number): Promise { - const new_id = await this.get_id(); + const compute_id = async ( + num_digits: number + ): Promise => { + const new_id = await get_id(); if (new_id === null || new_id === undefined) { return undefined; } return new_id.toString().padStart(num_digits, '0'); - } - - async update_form() { - const current_value = this.props.form.values[this.props.field.name]; + }; - if (!this.state.has_run) { - this.setState({has_run: true}); - console.debug('running autoinc'); - if (current_value === null) { - const new_id = await this.compute_id(this.props.num_digits || 4); - if (new_id === undefined) { - (this.context as any).dispatch({ - type: ActionType.ADD_ALERT, - payload: { - message: 'Failed to get autoincremented ID', - severity: 'error', - }, - }); - } else { - this.props.form.setFieldValue(this.props.field.name, new_id, true); - if (this.props.form.errors[this.props.field.name] !== undefined) - this.props.form.setFieldError(this.props.field.name, undefined); - } + const update_form = async () => { + const current_value = props.form.values[props.field.name]; + // we'll set the value in a form if the value has not already been set + // assume it has been set if the value is not null, empty or undefined + if ( + current_value === null || + current_value === '' || + current_value === undefined + ) { + const new_id = await compute_id(props.num_digits || 4); + if (new_id === undefined) { + setShowAutoIncrementEditForm(true); } else { - if (this.props.form.errors[this.props.field.name] !== undefined) - this.props.form.setFieldError(this.props.field.name, undefined); + props.form.setFieldValue(props.field.name, new_id, true); + if (props.form.errors[props.field.name] !== undefined) + props.form.setFieldError(props.field.name, undefined); } + } else { + if (props.form.errors[props.field.name] !== undefined) + props.form.setFieldError(props.field.name, undefined); } - } - - async componentDidMount() { - console.debug('did mount', this.props.form.values[this.props.field.name]); - await this.update_form(); - } - // remove the update for form, should only be update once - // async componentDidUpdate() { - // console.debug('did update',this.props.form.values[this.props.field.name]) - // await this.update_form(); - // } + }; - render() { - return ( - <> - - {this.state.is_ranger === false && } - - ); - } -} + useEffect(() => { + update_form(); + }, [props.form.values[props.field.name]]); -BasicAutoIncrementer.contextType = store; + return ( + <> + + { + console.debug('closing edit form'); + await update_form(); + setShowAutoIncrementEditForm(false); + }} + /> + + ); +}; // const uiSpec = { // 'component-namespace': 'faims-custom', // this says what web component to use to render/acquire value from From c748d8aa5585b73b7e3aa0219c6b7c4c27e375d4 Mon Sep 17 00:00:00 2001 From: Steve Cassidy Date: Wed, 27 Nov 2024 15:04:53 +1100 Subject: [PATCH 2/5] fix lint and build errors Signed-off-by: Steve Cassidy --- .../gui/components/autoincrement/edit-form.test.tsx | 10 ++++++---- app/src/gui/components/autoincrement/edit-form.tsx | 6 ------ app/src/gui/components/notebook/index.tsx | 2 +- .../components/notebook/settings/auto_incrementers.tsx | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/app/src/gui/components/autoincrement/edit-form.test.tsx b/app/src/gui/components/autoincrement/edit-form.test.tsx index 89ad9feab..6d1b18ecd 100644 --- a/app/src/gui/components/autoincrement/edit-form.test.tsx +++ b/app/src/gui/components/autoincrement/edit-form.test.tsx @@ -19,7 +19,7 @@ */ import {fireEvent, render, screen, waitFor} from '@testing-library/react'; -import BasicAutoIncrementer from './edit-form'; +import {AutoIncrementEditForm} from './edit-form'; import {expect, describe, it} from 'vitest'; const props = { @@ -27,11 +27,13 @@ const props = { form_id: '', field_id: '', label: '', + open: true, + handleClose: () => {}, }; describe('Check edit-form component', () => { it('Check add btn', async () => { - render(); + render(); const addRangeBtn = screen.getByTestId('addNewRangeBtn'); await waitFor(() => { @@ -45,7 +47,7 @@ describe('Check edit-form component', () => { }); }); it('Check remove btn', async () => { - render(); + render(); const addRangeBtn = screen.getByTestId('addNewRangeBtn'); await waitFor(() => { @@ -67,7 +69,7 @@ describe('Check edit-form component', () => { }); }); it('Check adding range start and stop fields', async () => { - render(); + render(); const addRangeBtn = screen.getByTestId('addNewRangeBtn'); await waitFor(() => { diff --git a/app/src/gui/components/autoincrement/edit-form.tsx b/app/src/gui/components/autoincrement/edit-form.tsx index 190380dca..face61dfa 100644 --- a/app/src/gui/components/autoincrement/edit-form.tsx +++ b/app/src/gui/components/autoincrement/edit-form.tsx @@ -48,7 +48,6 @@ import { createNewAutoincrementRange, } from '../../../local-data/autoincrement'; import CloseIcon from '@mui/icons-material/Close'; -import {all} from 'ol/events/condition'; interface Props { project_id: ProjectID; @@ -59,11 +58,6 @@ interface Props { handleClose: () => void; } -interface State { - ranges: LocalAutoIncrementRange[] | null; - ranges_initialised: boolean; -} - const FORM_SCHEMA = yup.object().shape({ start: yup.number().required().positive().integer(), stop: yup.number().required().positive().integer(), diff --git a/app/src/gui/components/notebook/index.tsx b/app/src/gui/components/notebook/index.tsx index b0674439c..1ba4e7e86 100644 --- a/app/src/gui/components/notebook/index.tsx +++ b/app/src/gui/components/notebook/index.tsx @@ -154,7 +154,7 @@ export default function NotebookComponent({project}: NotebookComponentProps) { const {data: template_id} = useQuery({ queryKey: ['project-template-id', project.project_id], - queryFn: async () : Promise => { + queryFn: async (): Promise => { // don't return undefined from queryFn const id = await getMetadataValue(project.project_id, 'template_id'); if (id !== undefined) return id as string; diff --git a/app/src/gui/components/notebook/settings/auto_incrementers.tsx b/app/src/gui/components/notebook/settings/auto_incrementers.tsx index 2da0fd50d..d35b87b88 100644 --- a/app/src/gui/components/notebook/settings/auto_incrementers.tsx +++ b/app/src/gui/components/notebook/settings/auto_incrementers.tsx @@ -1,5 +1,5 @@ import React, {useEffect, useState} from 'react'; -import {Box, Grid, Typography, Paper, Alert, Button} from '@mui/material'; +import {Box, Grid, Typography, Paper, Button} from '@mui/material'; import {ProjectInformation, ProjectUIModel} from '@faims3/data-model'; import {getAutoincrementReferencesForProject} from '../../../../local-data/autoincrement'; import {AutoIncrementReference} from '@faims3/data-model'; From 52074f09e438d471b3b0c39ab2ca1f547faded62 Mon Sep 17 00:00:00 2001 From: Steve Cassidy Date: Wed, 4 Dec 2024 13:32:31 +1100 Subject: [PATCH 3/5] tidy up Signed-off-by: Steve Cassidy --- app/src/gui/fields/BasicAutoIncrementer.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/app/src/gui/fields/BasicAutoIncrementer.tsx b/app/src/gui/fields/BasicAutoIncrementer.tsx index 999fc0ffb..5c5724d14 100644 --- a/app/src/gui/fields/BasicAutoIncrementer.tsx +++ b/app/src/gui/fields/BasicAutoIncrementer.tsx @@ -51,6 +51,7 @@ export const BasicAutoIncrementer = (props: FieldProps & Props) => { ); if (local_state.last_used_id === null && local_state.ranges.length === 0) { setShowAutoIncrementEditForm(true); + return null; } if (local_state.last_used_id === null) { @@ -79,7 +80,6 @@ export const BasicAutoIncrementer = (props: FieldProps & Props) => { // find a new range to use for (const range of local_state.ranges) { - console.debug('checking range', range); if (!range.fully_used) { const next_id = range.start; range.using = true; @@ -118,13 +118,11 @@ export const BasicAutoIncrementer = (props: FieldProps & Props) => { setShowAutoIncrementEditForm(true); } else { props.form.setFieldValue(props.field.name, new_id, true); - if (props.form.errors[props.field.name] !== undefined) - props.form.setFieldError(props.field.name, undefined); } - } else { - if (props.form.errors[props.field.name] !== undefined) - props.form.setFieldError(props.field.name, undefined); } + // reset any errors, we don't want to report these to the user + if (props.form.errors[props.field.name] !== undefined) + props.form.setFieldError(props.field.name, undefined); }; useEffect(() => { @@ -146,7 +144,6 @@ export const BasicAutoIncrementer = (props: FieldProps & Props) => { label={props.field.name} open={showAutoIncrementEditForm} handleClose={async () => { - console.debug('closing edit form'); await update_form(); setShowAutoIncrementEditForm(false); }} From 55a74eb2b489a6cb717b227992681d057498689e Mon Sep 17 00:00:00 2001 From: Steve Cassidy Date: Wed, 4 Dec 2024 13:32:56 +1100 Subject: [PATCH 4/5] Add notebook migration to set initial value of autoincrement fields properly Signed-off-by: Steve Cassidy --- designer/src/state/migrateNotebook.test.ts | 7 ++++++ designer/src/state/migrateNotebook.ts | 26 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/designer/src/state/migrateNotebook.test.ts b/designer/src/state/migrateNotebook.test.ts index e92d41dc0..5b064abad 100644 --- a/designer/src/state/migrateNotebook.test.ts +++ b/designer/src/state/migrateNotebook.test.ts @@ -97,6 +97,13 @@ describe('Migrate Notebook Tests', () => { } }); + test('fix auto incrementer initial value', () => { + const migrated = migrateNotebook(sampleNotebook); + const fields = migrated['ui-specification'].fields; + const targetField = fields['Field-ID']; + expect(targetField.initialValue).toBe(''); + }); + test('update form descriptions', () => { const migrated = migrateNotebook(sampleNotebook); const fviews = migrated['ui-specification'].fviews; diff --git a/designer/src/state/migrateNotebook.ts b/designer/src/state/migrateNotebook.ts index 24a1c53fd..f588cbb62 100644 --- a/designer/src/state/migrateNotebook.ts +++ b/designer/src/state/migrateNotebook.ts @@ -44,6 +44,9 @@ export const migrateNotebook = (notebook: unknown) => { // fix validation for photo fields which had a bad default fixPhotoValidation(notebookCopy); + // fix bad autoincrementer initial value + fixAutoIncrementerInitialValue(notebookCopy); + return notebookCopy; }; @@ -240,3 +243,26 @@ const fixPhotoValidation = (notebook: Notebook) => { notebook['ui-specification'].fields = fields; }; + +/** + * In some old notebooks, the initialValue of an auto incrementer was null + * which conflicts with the validate schema and triggers an error + * message on load in some cases. Here we replace that with the empty string. + * + * @param notebook A notebook that might be out of date, modified + */ +const fixAutoIncrementerInitialValue = (notebook: Notebook) => { + const fields: {[key: string]: FieldType} = {}; + + for (const fieldName in notebook['ui-specification'].fields) { + const field = notebook['ui-specification'].fields[fieldName]; + + if (field['component-name'] === 'BasicAutoIncrementer') { + if (field.initialValue === null) field.initialValue = ''; + } + + fields[fieldName] = field; + } + + notebook['ui-specification'].fields = fields; +}; From 30e1d90b36787abf1fe60d0cc19f518e21d40dd3 Mon Sep 17 00:00:00 2001 From: Steve Cassidy Date: Wed, 4 Dec 2024 18:02:11 +1100 Subject: [PATCH 5/5] rewrite the edit form to avoid need for a 'save' operation Signed-off-by: Steve Cassidy --- .../components/autoincrement/edit-form.tsx | 379 ++++++++---------- 1 file changed, 172 insertions(+), 207 deletions(-) diff --git a/app/src/gui/components/autoincrement/edit-form.tsx b/app/src/gui/components/autoincrement/edit-form.tsx index face61dfa..db83ef1c6 100644 --- a/app/src/gui/components/autoincrement/edit-form.tsx +++ b/app/src/gui/components/autoincrement/edit-form.tsx @@ -15,39 +15,36 @@ * * Filename: form.tsx * Description: - * TODO + * Defines a form for editing the auto-incrementer ranges for a field */ -import React, {useContext, useEffect, useState} from 'react'; -import {Formik, Form, Field, yupToFormErrors} from 'formik'; +import AddIcon from '@mui/icons-material/Add'; import { - ButtonGroup, - Box, - Alert, + Badge, Button, - LinearProgress, - Grid, - Divider, + ButtonGroup, Dialog, + DialogActions, + DialogContent, DialogTitle, - Typography, + Divider, IconButton, - DialogContent, - DialogActions, + Stack, + TextField, + Typography, } from '@mui/material'; -import AddIcon from '@mui/icons-material/Add'; -import {TextField} from 'formik-mui'; -import * as yup from 'yup'; +import {useContext, useState} from 'react'; +import {LocalAutoIncrementRange, ProjectID} from '@faims3/data-model'; +import CloseIcon from '@mui/icons-material/Close'; +import {useQuery, useQueryClient} from '@tanstack/react-query'; import {ActionType} from '../../../context/actions'; import {store} from '../../../context/store'; -import {ProjectID, LocalAutoIncrementRange} from '@faims3/data-model'; import { + createNewAutoincrementRange, getLocalAutoincrementRangesForField, setLocalAutoincrementRangesForField, - createNewAutoincrementRange, } from '../../../local-data/autoincrement'; -import CloseIcon from '@mui/icons-material/Close'; interface Props { project_id: ProjectID; @@ -58,11 +55,6 @@ interface Props { handleClose: () => void; } -const FORM_SCHEMA = yup.object().shape({ - start: yup.number().required().positive().integer(), - stop: yup.number().required().positive().integer(), -}); - export const AutoIncrementEditForm = ({ project_id, form_id, @@ -71,31 +63,31 @@ export const AutoIncrementEditForm = ({ open, handleClose, }: Props) => { - const [ranges, setRanges] = useState(null); - const [rangesInitialised, setRangesInitialised] = useState(false); const {dispatch} = useContext(store); - const ensureRanges = async () => { - if (ranges === null) { - const fetchedRanges = await getLocalAutoincrementRangesForField( + // useQuery to get the current ranges for the field, + // we will invalidate the query when we update the ranges + // so that they get re-fetched + const queryClient = useQueryClient(); + const queryKey = ['autoincrement', project_id, form_id, field_id]; + const {data: ranges} = useQuery({ + queryKey: queryKey, + queryFn: async () => { + const ranges = await getLocalAutoincrementRangesForField( project_id, form_id, field_id ); - setRanges(fetchedRanges); - } - }; + return ranges; + }, + initialData: [], + enabled: true, + }); const addNewRange = async () => { - await ensureRanges(); const updatedRanges = [...(ranges || [])]; updatedRanges.push(createNewAutoincrementRange(0, 0)); - setRanges(updatedRanges); - }; - - const clonedRanges = (): LocalAutoIncrementRange[] | null => { - if (ranges === null) return null; - return ranges.map(range => ({...range})); + updateRanges(updatedRanges); }; const updateRanges = async (newRanges: LocalAutoIncrementRange[]) => { @@ -106,19 +98,7 @@ export const AutoIncrementEditForm = ({ field_id, newRanges ); - dispatch({ - type: ActionType.ADD_ALERT, - payload: { - message: 'Range successfully updated', - severity: 'success', - }, - }); - - const isInitialised = newRanges.some( - range => range.using || range.fully_used - ); - setRanges(newRanges); - setRangesInitialised(isInitialised); + queryClient.invalidateQueries({queryKey: queryKey}); } catch (err: any) { dispatch({ type: ActionType.ADD_ALERT, @@ -130,163 +110,23 @@ export const AutoIncrementEditForm = ({ } }; - const renderRange = ( - range: LocalAutoIncrementRange, - rangeIndex: number, - ranges: LocalAutoIncrementRange[] - ) => { - const range_count = ranges.length; - const start_props = { - id: 'start', - label: 'start', - name: 'start', - required: true, - type: 'number', - readOnly: range.using || range.fully_used, - disabled: range.using || range.fully_used, - }; - const stop_props = { - id: 'stop', - label: 'stop', - name: 'stop', - required: true, - type: 'number', - readOnly: range.fully_used, - disabled: range.fully_used, + const updateRange = (index: number) => { + return (range: LocalAutoIncrementRange) => { + const rangesCopy = [...ranges]; + rangesCopy[index] = range; + updateRanges(rangesCopy); }; - - return ( - { - return FORM_SCHEMA.validate(values) - .then(v => { - if (!(v.stop > v.start)) { - return {stop: 'Must be greater than start'}; - } - return {}; - }) - .catch(err => { - return yupToFormErrors(err); - }); - }} - onSubmit={async (values, {setSubmitting}) => { - range.start = values.start; - range.stop = values.stop; - await updateRanges(ranges).then(() => { - setSubmitting(false); - }); - }} - > - {({submitForm, isSubmitting}) => ( - -
- - - - - - - - - - - - {range.using ? ( - - ) : ( - - )} - - - -
- {isSubmitting && } - - - )} - - ); }; - const renderRanges = () => { - const currentRanges = clonedRanges(); - - const all_used = currentRanges?.every( - (range: LocalAutoIncrementRange) => range.fully_used - ); - - if (currentRanges === null || currentRanges.length === 0) { - return ( - - No ranges allocated yet. - - ); + const handleRemoveRange = (index: number) => { + const newRanges = ranges?.filter((_, i) => i !== index); + if (newRanges !== undefined) { + updateRanges(newRanges); + } else { + updateRanges([]); } - - return ( -
- {all_used && ( - - All ranges are fully used. Please add a new range. - - )} - {currentRanges.map((range, index) => - renderRange(range, index, currentRanges) - )} -
- ); }; - useEffect(() => { - ensureRanges(); - }, []); - return ( Edit Settings for {label} @@ -314,11 +154,21 @@ export const AutoIncrementEditForm = ({ - {renderRanges()} - - + + {ranges?.map((range, index) => { + return ( + 1} + /> + ); + })} + + + + )} + + {!(props.range.using || props.range.fully_used) && ( + + )} + {props.range.fully_used && ( + + )} + + + + ); +};