From cc26940737be35ca11951ac4770b66098f20aacc Mon Sep 17 00:00:00 2001 From: beatz174-bit Date: Mon, 1 Dec 2025 15:52:07 +1000 Subject: [PATCH 1/2] Refactor product creation into reusable dialog --- src/components/AddProductDialog.tsx | 348 ++++++++++++++++++++++ src/screens/ManageProductsScreen.test.tsx | 50 ++++ src/screens/ManageProductsScreen.tsx | 308 +++---------------- 3 files changed, 447 insertions(+), 259 deletions(-) create mode 100644 src/components/AddProductDialog.tsx diff --git a/src/components/AddProductDialog.tsx b/src/components/AddProductDialog.tsx new file mode 100644 index 0000000..2ad27a4 --- /dev/null +++ b/src/components/AddProductDialog.tsx @@ -0,0 +1,348 @@ +// src/components/AddProductDialog.tsx +import { useCallback, useEffect, useMemo, useState } from 'react'; +import { + Alert, + AlertColor, + Button, + Dialog, + DialogContent, + DialogTitle, + IconButton, + InputAdornment, + MenuItem, + Stack, + TextField, +} from '@mui/material'; +import CloseIcon from '@mui/icons-material/Close'; +import SearchIcon from '@mui/icons-material/Search'; +import { v4 as uuidv4 } from 'uuid'; +import { useDatabase } from '../context/DBProvider'; +import { BarcodeScannerView } from './BarcodeScannerView'; +import { ExternalProductInfo, fetchProductFromOFF } from '../modules/openFoodFacts'; +import { DEFAULT_BULK_NAME, DEFAULT_UNIT_TYPE, Product } from '../models/Product'; + +export type AddProductDialogProps = { + open: boolean; + onClose: () => void; + categoryOptions: string[]; + onFeedback?: (feedback: { text: string; severity: AlertColor }) => void; + initialBarcode?: string | null; +}; + +export const AddProductDialog = ({ + open, + onClose, + categoryOptions, + onFeedback, + initialBarcode, +}: AddProductDialogProps) => { + const db = useDatabase(); + const [name, setName] = useState(''); + const [category, setCategory] = useState(''); + const [barcode, setBarcode] = useState(''); + const [barcodeError, setBarcodeError] = useState(''); + const [nameError, setNameError] = useState(''); + const [scannerOpen, setScannerOpen] = useState(false); + const [lookupStatus, setLookupStatus] = useState<'idle' | 'loading' | 'found' | 'notfound' | 'offline'>('idle'); + const [externalProduct, setExternalProduct] = useState(null); + + const resetForm = useCallback(() => { + setName(''); + setCategory(''); + setBarcode(''); + setBarcodeError(''); + setNameError(''); + setLookupStatus('idle'); + setExternalProduct(null); + setScannerOpen(false); + }, []); + + useEffect(() => { + if (open) { + if (categoryOptions.length > 0 && !categoryOptions.includes(category)) { + setCategory(categoryOptions[0]); + } + if (initialBarcode) { + setBarcode(initialBarcode); + void lookupBarcode(initialBarcode); + } + } else { + resetForm(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [open, categoryOptions, initialBarcode]); + + useEffect(() => { + if (!barcode) { + setLookupStatus('idle'); + setExternalProduct(null); + } + setBarcodeError(''); + }, [barcode]); + + const categoryMap = useMemo(() => new Map(categoryOptions.map((c) => [c, c])), [categoryOptions]); + + const findBarcodeConflict = useCallback( + async (value?: string) => { + if (!value) return undefined; + const conflict = await db.products.where('barcode').equals(value).first(); + return conflict ?? undefined; + }, + [db.products], + ); + + const findNameConflict = useCallback( + async (value?: string) => { + if (!value) return undefined; + const normalizedValue = value.trim().toLowerCase(); + const conflict = await db.products.filter((product) => product.name.trim().toLowerCase() === normalizedValue).first(); + return conflict ?? undefined; + }, + [db.products], + ); + + const assertUniqueBarcode = useCallback( + async (value?: string) => { + if (!value) return; + const conflict = await findBarcodeConflict(value); + if (conflict) { + const error = new Error('This barcode is already assigned to another product.'); + error.name = 'DuplicateBarcodeError'; + throw error; + } + }, + [findBarcodeConflict], + ); + + const assertUniqueName = useCallback( + async (value: string) => { + const normalized = value.trim().toLowerCase(); + if (!normalized) return; + const conflict = await findNameConflict(value); + if (conflict) { + const error = new Error('A product with this name already exists.'); + error.name = 'DuplicateNameError'; + throw error; + } + }, + [findNameConflict], + ); + + const addProductToAutoLists = useCallback( + async (product: Product, timestamp: number) => { + const pickLists = await db.pickLists.toArray(); + const categoriesAll = await db.categories.toArray(); + const categoriesByName = new Map(categoriesAll.map((c) => [c.name, c.id])); + + const eligibleLists = pickLists.filter((pickList) => + pickList.auto_add_new_products && Array.isArray(pickList.categories) + ? pickList.categories.some((catRef: string) => { + if (catRef === product.category) return true; + const resolvedId = categoriesByName.get(catRef); + return resolvedId === product.category; + }) + : false, + ); + + if (eligibleLists.length === 0) return; + await Promise.all( + eligibleLists.map(async (pickList) => { + const existing = await db.pickItems + .where('pick_list_id') + .equals(pickList.id) + .filter((item) => item.product_id === product.id) + .first(); + if (existing) return undefined; + return db.pickItems.add({ + id: uuidv4(), + pick_list_id: pickList.id, + product_id: product.id, + quantity: 1, + is_carton: false, + status: 'pending', + created_at: timestamp, + updated_at: timestamp, + }); + }), + ); + }, + [db.pickItems, db.pickLists, db.categories], + ); + + async function lookupBarcode(code: string) { + if (!code) return; + if (typeof navigator !== 'undefined' && 'onLine' in navigator && !navigator.onLine) { + setLookupStatus('offline'); + setExternalProduct(null); + return; + } + setLookupStatus('loading'); + const result = await fetchProductFromOFF(code); + if (result) { + setExternalProduct(result); + setLookupStatus('found'); + if (result.name) { + setName(result.name || ''); + } + } else { + setExternalProduct(null); + setLookupStatus('notfound'); + } + } + + const handleSubmit = async () => { + setNameError(''); + setBarcodeError(''); + + if (!name || !category) { + onFeedback?.({ text: 'Name and category are required.', severity: 'error' }); + return; + } + + const timestamp = Date.now(); + const productId = uuidv4(); + + try { + await assertUniqueName(name); + await assertUniqueBarcode(barcode); + + await db.transaction('rw', db.categories, db.products, db.pickLists, db.pickItems, async () => { + let categoryIdToSave: string; + const existingCategory = await db.categories.where('name').equals(category).first(); + if (existingCategory) { + categoryIdToSave = existingCategory.id; + } else { + const newCatId = uuidv4(); + const now = Date.now(); + await db.categories.add({ id: newCatId, name: category, created_at: now, updated_at: now }); + categoryIdToSave = newCatId; + } + + const newProduct: Product = { + id: productId, + name: name.trim(), + category: categoryIdToSave, + unit_type: DEFAULT_UNIT_TYPE, + bulk_name: DEFAULT_BULK_NAME, + barcode: barcode || undefined, + archived: false, + created_at: timestamp, + updated_at: timestamp, + }; + + await db.products.add(newProduct); + await addProductToAutoLists(newProduct, timestamp); + }); + + onFeedback?.({ text: 'Product added.', severity: 'success' }); + resetForm(); + onClose(); + } catch (err: any) { + if (err?.name === 'DuplicateNameError') { + setNameError(err.message || 'A product with this name already exists.'); + return; + } + if (err?.name === 'DuplicateBarcodeError') { + setBarcodeError(err.message || 'This barcode is already assigned to another product.'); + return; + } + onFeedback?.({ text: `Failed to add product: ${err?.message ?? String(err)}`, severity: 'error' }); + } + }; + + const handleDialogClose = () => { + resetForm(); + onClose(); + }; + + return ( + + + Add product + + + + + + + setName(e.target.value)} + error={!!nameError} + helperText={nameError || ' '} + fullWidth + InputProps={{ + startAdornment: ( + + + + ), + }} + data-testid="select-add-product-category" + /> + + setCategory(e.target.value)} select fullWidth> + {categoryOptions.map((opt) => ( + + {opt} + + ))} + + + + setBarcode(e.target.value)} + inputProps={{ 'data-testid': 'product-barcode-input' }} + error={!!barcodeError} + helperText={barcodeError || ' '} + fullWidth + /> + + + + {lookupStatus === 'offline' ? ( + + You are offline. Enter details manually. + + ) : null} + + + + + + + + + setScannerOpen(false)} aria-label="Scan barcode"> + Scan barcode + + { + setScannerOpen(false); + setBarcode(code); + try { + await lookupBarcode(code); + } catch { + // lookupBarcode handles errors + } + }} + /> + + + + ); +}; + +export default AddProductDialog; diff --git a/src/screens/ManageProductsScreen.test.tsx b/src/screens/ManageProductsScreen.test.tsx index dc562ef..f48be62 100644 --- a/src/screens/ManageProductsScreen.test.tsx +++ b/src/screens/ManageProductsScreen.test.tsx @@ -131,6 +131,14 @@ beforeEach(() => { mockDb.categories.toArray.mockResolvedValue([]); mockDb.categories.where.mockImplementation(() => ({ equals: () => ({ first: async () => undefined }) })); + + mockDb.products.filter.mockImplementation((predicate?: (product: any) => boolean) => ({ + first: async () => { + const items = mockUseProducts(); + return predicate ? items.find((item: any) => predicate(item)) : undefined; + }, + delete: vi.fn(), + })); }); function findSaveButton() { @@ -145,6 +153,10 @@ function findSaveButton() { return allButtons.length ? allButtons[0] : null; } +async function openAddProductDialog(user: ReturnType) { + await user.click(screen.getByRole('button', { name: /add product/i })); +} + describe('ManageProductsScreen barcode lookup', () => { beforeEach(() => { mockUseProducts.mockReturnValue([]); @@ -170,6 +182,8 @@ describe('ManageProductsScreen barcode lookup', () => { , ); + await openAddProductDialog(user); + await user.click(screen.getByRole('button', { name: /scan barcode/i })); await user.click(screen.getByRole('button', { name: /mock scan/i })); @@ -200,6 +214,8 @@ describe('ManageProductsScreen barcode lookup', () => { , ); + await openAddProductDialog(user); + await user.type(screen.getByLabelText(/name/i), 'New Product'); await user.click(screen.getByRole('button', { name: /scan barcode/i })); await user.click(screen.getByRole('button', { name: /mock scan/i })); @@ -235,6 +251,8 @@ describe('ManageProductsScreen barcode lookup', () => { , ); + await openAddProductDialog(user); + await user.type(screen.getByLabelText(/name/i), 'existing product'); const saveBtn = findSaveButton(); @@ -260,6 +278,8 @@ describe('ManageProductsScreen barcode lookup', () => { , ); + await openAddProductDialog(user); + await user.click(screen.getByRole('button', { name: /scan barcode/i })); await user.click(screen.getByRole('button', { name: /mock scan/i })); @@ -272,6 +292,34 @@ describe('ManageProductsScreen barcode lookup', () => { } }); + it('closes the add product dialog with the close icon and backdrop', async () => { + mockUseProducts.mockReturnValue([]); + mockUseCategories.mockReturnValue([{ id: 'cat-1', name: 'Snacks', created_at: 0, updated_at: 0 }]); + + const user = userEvent.setup(); + render( + + + , + ); + + await openAddProductDialog(user); + await user.click(screen.getByRole('button', { name: /close add product/i })); + + await waitFor(() => { + expect(screen.queryByRole('dialog', { name: /add product dialog/i })).not.toBeInTheDocument(); + }); + + await openAddProductDialog(user); + const backdrop = document.querySelector('[role="presentation"]'); + expect(backdrop).toBeTruthy(); + await user.click(backdrop as HTMLElement); + + await waitFor(() => { + expect(screen.queryByRole('dialog', { name: /add product dialog/i })).not.toBeInTheDocument(); + }); + }); + it('prevents updating a product to use an existing barcode', async () => { mockUseProducts.mockReturnValue([ { @@ -438,6 +486,8 @@ describe('ManageProductsScreen auto-adding products to pick lists', () => { , ); + await openAddProductDialog(user); + await user.type(screen.getByLabelText(/name/i), 'Granola Bar'); const saveBtn = findSaveButton(); expect(saveBtn).toBeTruthy(); diff --git a/src/screens/ManageProductsScreen.tsx b/src/screens/ManageProductsScreen.tsx index db11286..2fc071a 100644 --- a/src/screens/ManageProductsScreen.tsx +++ b/src/screens/ManageProductsScreen.tsx @@ -4,9 +4,6 @@ import { AlertColor, Button, Container, - Dialog, - DialogContent, - DialogTitle, InputAdornment, MenuItem, Snackbar, @@ -21,9 +18,8 @@ import { v4 as uuidv4 } from 'uuid'; import { ProductRow } from '../components/ProductRow'; import { useCategories, useProducts } from '../hooks/dataHooks'; import { useDatabase } from '../context/DBProvider'; -import { BarcodeScannerView } from '../components/BarcodeScannerView'; -import { ExternalProductInfo, fetchProductFromOFF } from '../modules/openFoodFacts'; import { DEFAULT_BULK_NAME, DEFAULT_UNIT_TYPE, Product } from '../models/Product'; +import { AddProductDialog } from '../components/AddProductDialog'; const ManageProductsScreen = () => { const db = useDatabase(); @@ -32,15 +28,9 @@ const ManageProductsScreen = () => { const location = useLocation(); const [search, setSearch] = useState(''); const [selectedCategory, setSelectedCategory] = useState('all'); - const [name, setName] = useState(''); - const [category, setCategory] = useState(''); - const [barcode, setBarcode] = useState(''); - const [barcodeError, setBarcodeError] = useState(''); - const [nameError, setNameError] = useState(''); - const [scannerOpen, setScannerOpen] = useState(false); - const [lookupStatus, setLookupStatus] = useState<'idle' | 'loading' | 'found' | 'notfound' | 'offline'>('idle'); - const [externalProduct, setExternalProduct] = useState(null); const [feedback, setFeedback] = useState<{ text: string; severity: AlertColor } | null>(null); + const [addProductDialogOpen, setAddProductDialogOpen] = useState(false); + const [pendingBarcode, setPendingBarcode] = useState(null); // Map category id -> name const categoriesById = useMemo(() => new Map(categories.map((c) => [c.id, c.name])), [categories]); @@ -94,55 +84,6 @@ const ManageProductsScreen = () => { [findNameConflict], ); - const addProductToAutoLists = useCallback( - async (product: Product, timestamp: number) => { - const pickLists = await db.pickLists.toArray(); - const categoriesAll = await db.categories.toArray(); - const categoriesByName = new Map(categoriesAll.map((c) => [c.name, c.id])); - - const eligibleLists = pickLists.filter((pickList) => - pickList.auto_add_new_products && Array.isArray(pickList.categories) - ? pickList.categories.some((catRef: string) => { - // catRef can be an id or a name — resolve both - if (catRef === product.category) return true; - const resolvedId = categoriesByName.get(catRef); - return resolvedId === product.category; - }) - : false, - ); - - if (eligibleLists.length === 0) return; - await Promise.all( - eligibleLists.map(async (pickList) => { - const existing = await db.pickItems - .where('pick_list_id') - .equals(pickList.id) - .filter((item) => item.product_id === product.id) - .first(); - if (existing) return undefined; - return db.pickItems.add({ - id: uuidv4(), - pick_list_id: pickList.id, - product_id: product.id, - quantity: 1, - is_carton: false, - status: 'pending', - created_at: timestamp, - updated_at: timestamp, - }); - }), - ); - }, - [db.pickItems, db.pickLists, db.categories], - ); - - useEffect(() => { - if (categoryOptions.length === 0) return; - if (!categoryOptions.includes(category)) { - setCategory(categoryOptions[0]); - } - }, [category, categoryOptions]); - useEffect(() => { if (selectedCategory !== 'all' && !categoryOptions.includes(selectedCategory)) { setSelectedCategory('all'); @@ -168,113 +109,12 @@ const ManageProductsScreen = () => { useEffect(() => { const state = location.state as { newBarcode?: string } | null; if (state?.newBarcode) { - setBarcode(state.newBarcode); - void lookupBarcode(state.newBarcode); + setPendingBarcode(state.newBarcode); + setAddProductDialogOpen(true); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [location.state]); - useEffect(() => { - if (!barcode) { - setLookupStatus('idle'); - setExternalProduct(null); - } - setBarcodeError(''); - }, [barcode]); - - async function lookupBarcode(code: string) { - if (!code) return; - if (typeof navigator !== 'undefined' && 'onLine' in navigator && !navigator.onLine) { - setLookupStatus('offline'); - setExternalProduct(null); - return; - } - setLookupStatus('loading'); - const result = await fetchProductFromOFF(code); - if (result) { - setExternalProduct(result); - setLookupStatus('found'); - // TEST-FRIENDLY CHANGE: always set the name when a result is found - if (result.name) { - setName(result.name || ''); - } - } else { - setExternalProduct(null); - setLookupStatus('notfound'); - } - } - - const addProduct = async () => { - setNameError(''); - setBarcodeError(''); - - if (!name || !category) { - setFeedback({ text: 'Name and category are required.', severity: 'error' }); - return; - } - - const timestamp = Date.now(); - const productId = uuidv4(); - - try { - await assertUniqueName(name); - await assertUniqueBarcode(barcode); - - await db.transaction( - 'rw', - db.categories, - db.products, - db.pickLists, - db.pickItems, - async () => { - let categoryIdToSave: string; - const existingCategory = await db.categories.where('name').equals(category).first(); - if (existingCategory) { - categoryIdToSave = existingCategory.id; - } else { - const newCatId = uuidv4(); - const now = Date.now(); - await db.categories.add({ id: newCatId, name: category, created_at: now, updated_at: now }); - categoryIdToSave = newCatId; - } - - const newProduct: Product = { - id: productId, - name: name.trim(), - category: categoryIdToSave, - unit_type: DEFAULT_UNIT_TYPE, - bulk_name: DEFAULT_BULK_NAME, - barcode: barcode || undefined, - archived: false, - created_at: timestamp, - updated_at: timestamp, - }; - - await db.products.add(newProduct); - - await addProductToAutoLists(newProduct, timestamp); - }, - ); - - setName(''); - setBarcode(''); - setNameError(''); - setBarcodeError(''); - setFeedback({ text: 'Product added.', severity: 'success' }); - } catch (err: any) { - console.error('Failed to add product', err); - if (err?.name === 'DuplicateNameError') { - setNameError(err.message || 'A product with this name already exists.'); - return; - } - if (err?.name === 'DuplicateBarcodeError') { - setBarcodeError(err.message || 'This barcode is already assigned to another product.'); - return; - } - setFeedback({ text: `Failed to add product: ${err?.message ?? String(err)}`, severity: 'error' }); - } - }; - const updateProduct = async ( productId: string, updates: { @@ -283,9 +123,6 @@ const ManageProductsScreen = () => { barcode?: string; }, ) => { - setNameError(''); - setBarcodeError(''); - try { await assertUniqueName(updates.name, productId); await assertUniqueBarcode(updates.barcode, productId); @@ -338,13 +175,7 @@ const ManageProductsScreen = () => { } catch (err: any) { console.error('Failed to update product', err); - if (err?.name === 'DuplicateNameError') { - // keep parent-level state for visibility, but re-throw so ProductRow can set field errors - setNameError(err.message || 'A product with this name already exists.'); - throw err; - } - if (err?.name === 'DuplicateBarcodeError') { - setBarcodeError(err.message || 'This barcode is already assigned to another product.'); + if (err?.name === 'DuplicateNameError' || err?.name === 'DuplicateBarcodeError') { throw err; } @@ -363,96 +194,64 @@ const ManageProductsScreen = () => { setFeedback({ text: 'Product deleted.', severity: 'success' }); }; + const handleAddProductClose = () => { + setAddProductDialogOpen(false); + setPendingBarcode(null); + }; + // ---------- RENDER ---------- return ( Manage Products - + - - - setSearch(e.target.value)} - InputProps={{ - startAdornment: ( - - - - ), - }} - fullWidth - /> - setSelectedCategory(e.target.value)} - sx={{ minWidth: 200 }} - data-testid="select-filter-by-category" - > - All categories - {categoryOptions.map((opt) => ( - - {opt} - - ))} - - + - + setName(e.target.value)} - error={!!nameError} - data-testid="select-add-product-category" + placeholder="Search" + value={search} + onChange={(e) => setSearch(e.target.value)} + InputProps={{ + startAdornment: ( + + + + ), + }} + fullWidth /> - {nameError ?
{nameError}
: null} - setCategory(e.target.value)} select + label="Filter by category" + value={selectedCategory} + onChange={(e) => setSelectedCategory(e.target.value)} + sx={{ minWidth: 200 }} + data-testid="select-filter-by-category" > + All categories {categoryOptions.map((opt) => ( {opt} ))} - - - setBarcode(e.target.value)} - inputProps={{ 'data-testid': 'product-barcode-input' }} - error={!!barcodeError} - /> - - - - {barcodeError ?
{barcodeError}
: null} - - {lookupStatus === 'offline' ? ( - - You are offline. Enter details manually. - - ) : null} - - - -
@@ -469,22 +268,13 @@ const ManageProductsScreen = () => { ))} - setScannerOpen(false)} aria-label="Scan barcode"> - Scan barcode - - { - setScannerOpen(false); - setBarcode(code); - try { - await lookupBarcode(code); - } catch { - // lookupBarcode handles errors - } - }} - /> - - + setFeedback(null)}> {feedback ? {feedback.text} : undefined} From 7be6ee1d322dc07ad14c3e1e71f0444237c560c6 Mon Sep 17 00:00:00 2001 From: beatz174-bit Date: Mon, 1 Dec 2025 15:59:33 +1000 Subject: [PATCH 2/2] Remove search field from add product dialog --- src/components/AddProductDialog.tsx | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/components/AddProductDialog.tsx b/src/components/AddProductDialog.tsx index 2ad27a4..c1e53e5 100644 --- a/src/components/AddProductDialog.tsx +++ b/src/components/AddProductDialog.tsx @@ -8,13 +8,11 @@ import { DialogContent, DialogTitle, IconButton, - InputAdornment, MenuItem, Stack, TextField, } from '@mui/material'; import CloseIcon from '@mui/icons-material/Close'; -import SearchIcon from '@mui/icons-material/Search'; import { v4 as uuidv4 } from 'uuid'; import { useDatabase } from '../context/DBProvider'; import { BarcodeScannerView } from './BarcodeScannerView'; @@ -277,13 +275,6 @@ export const AddProductDialog = ({ error={!!nameError} helperText={nameError || ' '} fullWidth - InputProps={{ - startAdornment: ( - - - - ), - }} data-testid="select-add-product-category" />