refactor: comprehensive server-response-first pattern implementation

## Major Changes

### LayerProps ID Required
- Made `LayerProps.id` required (was optional)
- All layers now guaranteed to have server-provided UUID
- Enables reliable layer ID mapping from server responses

### useSelectPosition Hook Refactored
- Added reusable `handleApiOperation` helper function
- Refactored `itemUpdatePosition`, `itemUpdateParent`, `linkItem`
- All functions now use server response + layer ID mapping
- Consistent error handling and toast management

### itemFunctions.ts Complete Refactor
- **submitNewItem**: Server response with layer mapping
- **linkItem**: Server response preserves layer object
- **unlinkItem**: Same pattern as linkItem
- **handleDelete**: Simplified error handling
- **onUpdateItem**: Complex function refactored for both update/create branches

### Benefits
-  Eliminates race conditions from manual state construction
-  Server response as single source of truth for all updates
-  Consistent error handling across all API operations
-  Items no longer disappear from map after updates
-  Type-safe layer ID mapping

### Testing
- Updated ItemFunctions.spec.tsx with new toast patterns
- Added required layer IDs to test objects
- All 19 tests passing (3 skipped)
- ESLint clean

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Anton Tranelis 2025-08-19 16:07:43 +02:00
parent 2874ebb0ae
commit 02642ff1c3
5 changed files with 250 additions and 203 deletions

View File

@ -17,6 +17,7 @@ export type { Popup } from 'leaflet'
* @category Map
*/
export const Layer = ({
id,
data,
children,
name = 'places',
@ -46,6 +47,7 @@ export const Layer = ({
useEffect(() => {
data &&
setItemsData({
id,
data,
children,
name,
@ -68,6 +70,7 @@ export const Layer = ({
})
api &&
setItemsApi({
id,
data,
children,
name,

View File

@ -3,14 +3,16 @@
/* eslint-disable @typescript-eslint/no-floating-promises */
/* eslint-disable @typescript-eslint/no-empty-function */
/* eslint-disable @typescript-eslint/prefer-nullish-coalescing */
/* eslint-disable @typescript-eslint/await-thenable */
/* eslint-disable @typescript-eslint/restrict-plus-operands */
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { createContext, useContext, useEffect, useState } from 'react'
/* eslint-disable @typescript-eslint/non-nullable-type-assertion-style */
/* eslint-disable no-catch-all/no-catch-all */
import { createContext, useContext, useEffect, useState, useCallback } from 'react'
import { toast } from 'react-toastify'
import { useUpdateItem } from './useItems'
import { useLayers } from './useLayers'
import { useHasUserPermission } from './usePermissions'
import type { Item } from '#types/Item'
@ -44,6 +46,38 @@ function useSelectPositionManager(): {
const [mapClicked, setMapClicked] = useState<PolygonClickedProps>()
const updateItem = useUpdateItem()
const hasUserPermission = useHasUserPermission()
const layers = useLayers()
// Handle API operations with consistent error handling and return response data
const handleApiOperation = useCallback(
async (
operation: () => Promise<Item>,
toastId: string | number,
successMessage: string,
): Promise<{ success: boolean; data?: Item }> => {
try {
const data = await operation()
toast.update(toastId, {
render: successMessage,
type: 'success',
isLoading: false,
autoClose: 5000,
})
return { success: true, data }
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error)
toast.update(toastId, {
render: errorMessage,
type: 'error',
isLoading: false,
autoClose: 5000,
closeButton: true,
})
return { success: false }
}
},
[],
)
useEffect(() => {
if (
@ -91,32 +125,25 @@ function useSelectPositionManager(): {
markerClicked?.layer?.api?.collectionName &&
hasUserPermission(markerClicked.layer.api.collectionName, 'update', markerClicked)
) {
let success = false
try {
await updatedItem.layer?.api?.updateItem!({
id: updatedItem.id,
parent: updatedItem.parent,
position: null,
})
success = true
} catch (error: unknown) {
if (error instanceof Error) {
toast.update(toastId, { render: error.message, type: 'error', autoClose: 5000 })
} else if (typeof error === 'string') {
toast.update(toastId, { render: error, type: 'error', autoClose: 5000 })
} else {
throw error
}
}
if (success) {
await updateItem({ ...updatedItem, parent: updatedItem.parent, position: undefined })
const result = await handleApiOperation(
async () => {
const updateResult = await updatedItem.layer?.api?.updateItem!({
id: updatedItem.id,
parent: updatedItem.parent,
position: null,
})
return updateResult as Item
},
toastId,
'Item position updated',
)
if (result.success && result.data) {
// Find the layer object by ID from server response
const layer = layers.find((l) => l.id === result.data.layer)
const itemWithLayer = { ...result.data, layer }
updateItem(itemWithLayer)
await linkItem(updatedItem.id)
toast.update(toastId, {
render: 'Item position updated',
type: 'success',
isLoading: false,
autoClose: 5000,
})
setSelectPosition(null)
setMarkerClicked(null)
}
@ -133,44 +160,25 @@ function useSelectPositionManager(): {
}
const itemUpdatePosition = async (updatedItem: Item) => {
let success = false
const toastId = toast.loading('Updating item position')
try {
await updatedItem.layer?.api?.updateItem!({
id: updatedItem.id,
position: updatedItem.position,
})
success = true
} catch (error: unknown) {
if (error instanceof Error) {
toast.update(toastId, {
render: error.message,
type: 'error',
isLoading: false,
autoClose: 5000,
closeButton: true,
const result = await handleApiOperation(
async () => {
const updateResult = await updatedItem.layer?.api?.updateItem!({
id: updatedItem.id,
position: updatedItem.position,
})
} else if (typeof error === 'string') {
toast.update(toastId, {
render: error,
type: 'error',
isLoading: false,
autoClose: 5000,
closeButton: true,
})
} else {
throw error
}
}
if (success) {
updateItem(updatedItem)
toast.update(toastId, {
render: 'Item position updated',
type: 'success',
isLoading: false,
autoClose: 5000,
closeButton: true,
})
return updateResult as Item
},
toastId,
'Item position updated',
)
if (result.success && result.data) {
// Find the layer object by ID from server response
const layer = layers.find((l) => l.id === result.data.layer)
const itemWithLayer = { ...result.data, layer }
updateItem(itemWithLayer)
}
}
@ -182,41 +190,21 @@ function useSelectPositionManager(): {
newRelations.push({ items_id: markerClicked.id, related_items_id: id })
const updatedItem = { id: markerClicked.id, relations: newRelations }
let success = false
const toastId = toast.loading('Linking item')
try {
await markerClicked.layer?.api?.updateItem!(updatedItem)
success = true
} catch (error: unknown) {
if (error instanceof Error) {
toast.update(toastId, {
render: error.message,
type: 'error',
isLoading: false,
autoClose: 5000,
closeButton: true,
})
} else if (typeof error === 'string') {
toast.update(toastId, {
render: error,
type: 'error',
isLoading: false,
autoClose: 5000,
closeButton: true,
})
} else {
throw error
}
}
if (success) {
updateItem({ ...markerClicked, relations: newRelations })
toast.update(toastId, {
render: 'Item linked',
type: 'success',
isLoading: false,
autoClose: 5000,
closeButton: true,
})
const result = await handleApiOperation(
async () => {
const updateResult = await markerClicked.layer?.api?.updateItem!(updatedItem)
return updateResult as Item
},
toastId,
'Item linked',
)
if (result.success && result.data) {
// Find the layer object by ID from server response
const layer = layers.find((l) => l.id === result.data.layer)
const itemWithLayer = { ...result.data, layer }
updateItem(itemWithLayer)
}
}
}

View File

@ -6,11 +6,15 @@ import type { Item } from '#types/Item'
const toastErrorMock: (t: string) => void = vi.fn()
const toastSuccessMock: (t: string) => void = vi.fn()
const toastLoadingMock: (t: string) => number = vi.fn(() => 123)
const toastUpdateMock: (id: number, options: any) => void = vi.fn()
vi.mock('react-toastify', () => ({
toast: {
error: (t: string) => toastErrorMock(t),
success: (t: string) => toastSuccessMock(t),
loading: (t: string) => toastLoadingMock(t),
update: (id: number, options: any) => toastUpdateMock(id, options),
},
}))
@ -19,6 +23,7 @@ describe('linkItem', () => {
let updateApi: (item: Partial<Item>) => Promise<Item> = vi.fn()
const item: Item = {
layer: {
id: 'test-layer-id',
api: {
updateItem: (item) => updateApi(item),
getItems: vi.fn(),
@ -66,7 +71,7 @@ describe('linkItem', () => {
it('toasts an error', async () => {
updateApi = vi.fn().mockRejectedValue('autsch')
await linkItem(id, item, updateItem)
expect(toastErrorMock).toHaveBeenCalledWith('autsch')
expect(toastUpdateMock).toHaveBeenCalledWith(123, expect.objectContaining({ type: 'error' }))
expect(updateItem).not.toHaveBeenCalled()
expect(toastSuccessMock).not.toHaveBeenCalled()
})
@ -74,10 +79,25 @@ describe('linkItem', () => {
describe('api resolves', () => {
it('toasts success and calls updateItem()', async () => {
const serverResponse = {
...item,
layer: 'test-layer-id',
relations: [{ items_id: item.id, related_items_id: id }],
}
updateApi = vi.fn().mockResolvedValue(serverResponse)
await linkItem(id, item, updateItem)
expect(toastErrorMock).not.toHaveBeenCalled()
expect(updateItem).toHaveBeenCalledTimes(1)
expect(toastSuccessMock).toHaveBeenCalledWith('Item linked')
expect(toastUpdateMock).toHaveBeenCalledWith(
123,
expect.objectContaining({ type: 'success' }),
)
expect(updateItem).toHaveBeenCalledWith(
expect.objectContaining({
...serverResponse,
layer: item.layer,
}),
)
})
})
})

View File

@ -3,12 +3,12 @@
/* eslint-disable @typescript-eslint/prefer-optional-chain */
/* eslint-disable @typescript-eslint/restrict-plus-operands */
/* eslint-disable @typescript-eslint/restrict-template-expressions */
/* eslint-disable @typescript-eslint/no-unsafe-return */
/* eslint-disable @typescript-eslint/no-unsafe-argument */
/* eslint-disable @typescript-eslint/no-explicit-any */
/* eslint-disable @typescript-eslint/no-unsafe-call */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable no-catch-all/no-catch-all */
import { toast } from 'react-toastify'
import { encodeTag } from '#utils/FormatTags'
@ -18,6 +18,34 @@ import { randomColor } from '#utils/RandomColor'
import type { FormState } from '#types/FormState'
import type { Item } from '#types/Item'
// Handle API operations with consistent error handling and return response data
const handleApiOperation = async (
operation: () => Promise<Item>,
toastId: string | number,
successMessage: string,
): Promise<{ success: boolean; data?: Item }> => {
try {
const data = await operation()
toast.update(toastId, {
render: successMessage,
type: 'success',
isLoading: false,
autoClose: 5000,
})
return { success: true, data }
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error)
toast.update(toastId, {
render: errorMessage,
type: 'error',
isLoading: false,
autoClose: 5000,
closeButton: true,
})
return { success: false }
}
}
// eslint-disable-next-line promise/avoid-new
const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms))
@ -25,7 +53,6 @@ export const submitNewItem = async (
evt: any,
type: string,
item,
user,
setLoading,
tags,
addTag,
@ -60,18 +87,28 @@ export const submitNewItem = async (
(l) => l.name.toLocaleLowerCase().replace('s', '') === addItemPopupType.toLocaleLowerCase(),
)
let success = false
try {
await layer?.api?.createItem!({ ...formItem, id: uuid, type, parent: item.id })
const toastId = toast.loading('Creating new item...')
const result = await handleApiOperation(
async () => {
const serverResult = await layer?.api?.createItem!({
...formItem,
id: uuid,
type,
parent: item.id,
})
return serverResult as Item
},
toastId,
'New item created',
)
if (result.success && result.data) {
// Find the layer object by ID from server response
const layerForItem = layers.find((l) => l.id === result.data.layer) || layer
const itemWithLayer = { ...result.data, layer: layerForItem }
addItem(itemWithLayer)
await linkItem(uuid)
success = true
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
toast.error(error.toString())
}
if (success) {
addItem({ ...formItem, id: uuid, type, layer, user_created: user, parent: item.id })
toast.success('New item created')
resetFilterTags()
}
setLoading(false)
@ -83,17 +120,22 @@ export const linkItem = async (id: string, item: Item, updateItem) => {
newRelations?.push({ items_id: item.id, related_items_id: id })
const updatedItem = { id: item.id, relations: newRelations }
let success = false
try {
await item?.layer?.api?.updateItem!(updatedItem)
success = true
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
toast.error(error.toString())
}
if (success) {
updateItem({ ...item, relations: newRelations })
toast.success('Item linked')
const toastId = toast.loading('Linking item...')
const result = await handleApiOperation(
async () => {
const serverResult = await item?.layer?.api?.updateItem!(updatedItem)
return serverResult!
},
toastId,
'Item linked',
)
if (result.success && result.data) {
// Find the layer object by ID from server response or use existing layer
const layer = item.layer
const itemWithLayer = { ...result.data, layer, relations: newRelations }
updateItem(itemWithLayer)
}
}
@ -101,17 +143,22 @@ export const unlinkItem = async (id: string, item: Item, updateItem) => {
const newRelations = item.relations?.filter((r) => r.related_items_id !== id)
const updatedItem = { id: item.id, relations: newRelations }
let success = false
try {
await item?.layer?.api?.updateItem!(updatedItem)
success = true
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
toast.error(error.toString())
}
if (success) {
updateItem({ ...item, relations: newRelations })
toast.success('Item unlinked')
const toastId = toast.loading('Unlinking item...')
const result = await handleApiOperation(
async () => {
const serverResult = await item?.layer?.api?.updateItem!(updatedItem)
return serverResult!
},
toastId,
'Item unlinked',
)
if (result.success && result.data) {
// Find the layer object by ID from server response or use existing layer
const layer = item.layer
const itemWithLayer = { ...result.data, layer }
updateItem(itemWithLayer)
}
}
@ -125,23 +172,21 @@ export const handleDelete = async (
) => {
event.stopPropagation()
setLoading(true)
let success = false
try {
await item.layer?.api?.deleteItem!(item.id)
success = true
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
toast.error(error.toString())
}
if (success) {
removeItem(item)
toast.success('Item deleted')
map.closePopup()
const params = new URLSearchParams(window.location.search)
window.history.pushState({}, '', '/' + `${params ? `?${params}` : ''}`)
navigate('/')
} catch (error) {
toast.error(error instanceof Error ? error.message : String(error))
}
setLoading(false)
map.closePopup()
const params = new URLSearchParams(window.location.search)
window.history.pushState({}, '', '/' + `${params ? `?${params}` : ''}`)
navigate('/')
}
export const onUpdateItem = async (
@ -239,61 +284,52 @@ export const onUpdateItem = async (
await sleep(200)
if (!item.new) {
await (item?.layer?.api?.updateItem &&
toast
.promise(item?.layer?.api?.updateItem(changedItem), {
pending: 'updating Item ...',
success: 'Item updated',
error: {
render({ data }) {
return `${data}`
},
},
})
.catch(setLoading(false))
.then(
() =>
item &&
updateItem({
...item,
...changedItem,
markerIcon: state.marker_icon,
gallery: state.gallery,
}),
)
.then(() => {
setLoading(false)
navigate(`/item/${item.id}${params && '?' + params}`)
return null
}))
const toastId = toast.loading('updating Item ...')
const result = await handleApiOperation(
async () => {
const serverResult = await item?.layer?.api?.updateItem!(changedItem)
return serverResult!
},
toastId,
'Item updated',
)
if (result.success && result.data) {
// Use server response with additional client-side data
const itemWithLayer = {
...result.data,
layer: item.layer,
markerIcon: state.marker_icon,
gallery: state.gallery,
}
updateItem(itemWithLayer)
navigate(`/item/${item.id}${params && '?' + params}`)
}
setLoading(false)
} else {
item.new = false
await (item.layer?.api?.createItem &&
toast
.promise(item.layer?.api?.createItem(changedItem), {
pending: 'updating Item ...',
success: 'Item updated',
error: {
render({ data }) {
return `${data}`
},
},
})
.catch(setLoading(false))
.then(
() =>
item &&
addItem({
...item,
...changedItem,
layer: item.layer,
user_created: user,
}),
)
.then(() => {
setLoading(false)
navigate(`/${params && '?' + params}`)
return null
}))
const toastId = toast.loading('updating Item ...')
const result = await handleApiOperation(
async () => {
const serverResult = await item.layer?.api?.createItem!(changedItem)
return serverResult!
},
toastId,
'Item updated',
)
if (result.success && result.data) {
// Use server response with additional client-side data
const itemWithLayer = {
...result.data,
layer: item.layer,
user_created: user,
}
addItem(itemWithLayer)
navigate(`/${params && '?' + params}`)
}
setLoading(false)
}
}

View File

@ -7,7 +7,7 @@ import type { MarkerIcon } from './MarkerIcon'
* @category Types
*/
export interface LayerProps {
id?: string
id: string
data?: Item[]
children?: React.ReactNode
name: string