Merge pull request #19 from beatz174-bit/codex/fix-duplicate-barcode-addition-in-product-table
Enforce unique barcodes in product management
This commit is contained in:
@@ -50,28 +50,41 @@ export const ProductRow = ({ product, categories, onSave, onDelete }: ProductRow
|
|||||||
const [isEditing, setIsEditing] = useState(false);
|
const [isEditing, setIsEditing] = useState(false);
|
||||||
const [formState, setFormState] = useState<ProductFormState>(() => getInitialFormState(product));
|
const [formState, setFormState] = useState<ProductFormState>(() => getInitialFormState(product));
|
||||||
const [isScannerOpen, setIsScannerOpen] = useState(false);
|
const [isScannerOpen, setIsScannerOpen] = useState(false);
|
||||||
|
const [saveError, setSaveError] = useState('');
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
setFormState(getInitialFormState(product));
|
setFormState(getInitialFormState(product));
|
||||||
|
setSaveError('');
|
||||||
}, [product]);
|
}, [product]);
|
||||||
|
|
||||||
const handleChange = (field: keyof ProductFormState) => (event: ChangeEvent<HTMLInputElement>) => {
|
const handleChange = (field: keyof ProductFormState) => (event: ChangeEvent<HTMLInputElement>) => {
|
||||||
setFormState((prev) => ({ ...prev, [field]: event.target.value }));
|
setFormState((prev) => ({ ...prev, [field]: event.target.value }));
|
||||||
|
setSaveError('');
|
||||||
};
|
};
|
||||||
|
|
||||||
const handleSave = async () => {
|
const handleSave = async () => {
|
||||||
if (!formState.name) return;
|
if (!formState.name) return;
|
||||||
await onSave(product.id, {
|
try {
|
||||||
name: formState.name,
|
await onSave(product.id, {
|
||||||
category: formState.category,
|
name: formState.name,
|
||||||
barcode: formState.barcode || undefined,
|
category: formState.category,
|
||||||
});
|
barcode: formState.barcode || undefined,
|
||||||
setIsEditing(false);
|
});
|
||||||
|
setIsEditing(false);
|
||||||
|
setSaveError('');
|
||||||
|
} catch (error) {
|
||||||
|
if (error instanceof Error && error.name === 'DuplicateBarcodeError') {
|
||||||
|
setSaveError('This barcode is already assigned to another product.');
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
const handleCancel = () => {
|
const handleCancel = () => {
|
||||||
setIsEditing(false);
|
setIsEditing(false);
|
||||||
setFormState(getInitialFormState(product));
|
setFormState(getInitialFormState(product));
|
||||||
|
setSaveError('');
|
||||||
};
|
};
|
||||||
|
|
||||||
return (
|
return (
|
||||||
@@ -99,9 +112,17 @@ export const ProductRow = ({ product, categories, onSave, onDelete }: ProductRow
|
|||||||
value={formState.barcode}
|
value={formState.barcode}
|
||||||
onChange={handleChange('barcode')}
|
onChange={handleChange('barcode')}
|
||||||
size="small"
|
size="small"
|
||||||
|
error={Boolean(saveError)}
|
||||||
|
helperText={saveError || undefined}
|
||||||
InputProps={{
|
InputProps={{
|
||||||
endAdornment: (
|
endAdornment: (
|
||||||
<Button size="small" onClick={() => setFormState((prev) => ({ ...prev, barcode: '' }))}>
|
<Button
|
||||||
|
size="small"
|
||||||
|
onClick={() => {
|
||||||
|
setFormState((prev) => ({ ...prev, barcode: '' }));
|
||||||
|
setSaveError('');
|
||||||
|
}}
|
||||||
|
>
|
||||||
Clear
|
Clear
|
||||||
</Button>
|
</Button>
|
||||||
),
|
),
|
||||||
|
|||||||
@@ -51,6 +51,45 @@ export class StockFillDB extends Dexie {
|
|||||||
})),
|
})),
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
this.version(3)
|
||||||
|
.stores({
|
||||||
|
products:
|
||||||
|
'id, name, category, barcode, archived, created_at, updated_at',
|
||||||
|
areas: 'id, name, created_at, updated_at',
|
||||||
|
pickLists: 'id, area_id, created_at, completed_at',
|
||||||
|
pickItems: 'id, pick_list_id, product_id, status, created_at, updated_at',
|
||||||
|
categories: 'id, name, created_at, updated_at',
|
||||||
|
})
|
||||||
|
.upgrade(async (tx) => {
|
||||||
|
const products = await tx.table('products').toArray();
|
||||||
|
const seenBarcodes = new Set<string>();
|
||||||
|
const now = Date.now();
|
||||||
|
|
||||||
|
await Promise.all(
|
||||||
|
products.map((product) => {
|
||||||
|
if (!product.barcode) return undefined;
|
||||||
|
|
||||||
|
if (seenBarcodes.has(product.barcode)) {
|
||||||
|
return tx.table('products').update(product.id, {
|
||||||
|
barcode: undefined,
|
||||||
|
updated_at: now,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
seenBarcodes.add(product.barcode);
|
||||||
|
return undefined;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
this.version(4).stores({
|
||||||
|
products: 'id, name, category, &barcode, archived, created_at, updated_at',
|
||||||
|
areas: 'id, name, created_at, updated_at',
|
||||||
|
pickLists: 'id, area_id, created_at, completed_at',
|
||||||
|
pickItems: 'id, pick_list_id, product_id, status, created_at, updated_at',
|
||||||
|
categories: 'id, name, created_at, updated_at',
|
||||||
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,42 +1,39 @@
|
|||||||
import { http, HttpResponse } from 'msw';
|
import { http, HttpResponse } from 'msw';
|
||||||
import { setupServer } from 'msw/node';
|
import { setupServer } from 'msw/node';
|
||||||
import { MemoryRouter } from 'react-router-dom';
|
import { MemoryRouter } from 'react-router-dom';
|
||||||
import { render, screen, waitFor } from '@testing-library/react';
|
import { fireEvent, render, screen, waitFor, waitForElementToBeRemoved } from '@testing-library/react';
|
||||||
import userEvent from '@testing-library/user-event';
|
import userEvent from '@testing-library/user-event';
|
||||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
|
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
import { ManageProductsScreen } from './ManageProductsScreen';
|
import { ManageProductsScreen } from './ManageProductsScreen';
|
||||||
|
|
||||||
const productsMock = [{ id: 'prod-1', name: 'Chips', category: 'Snacks', archived: false, created_at: 0, updated_at: 0 }];
|
let mockScannedBarcode = '123456';
|
||||||
const categoriesMock = [{ id: 'cat-1', name: 'Snacks', created_at: 0, updated_at: 0 }];
|
const mockUseProducts = vi.fn();
|
||||||
|
const mockUseCategories = vi.fn();
|
||||||
|
|
||||||
|
const mockDb = {
|
||||||
|
products: {
|
||||||
|
add: vi.fn(),
|
||||||
|
update: vi.fn(),
|
||||||
|
delete: vi.fn(),
|
||||||
|
where: vi.fn(),
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
vi.mock('../hooks/dataHooks', () => ({
|
vi.mock('../hooks/dataHooks', () => ({
|
||||||
useProducts: () => productsMock,
|
useProducts: () => mockUseProducts(),
|
||||||
useCategories: () => categoriesMock,
|
useCategories: () => mockUseCategories(),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const productDeleteMock = vi.fn();
|
const productDeleteMock = vi.fn();
|
||||||
const pickItemCountMock = vi.fn();
|
const pickItemCountMock = vi.fn();
|
||||||
|
|
||||||
vi.mock('../context/DBProvider', () => ({
|
vi.mock('../context/DBProvider', () => ({
|
||||||
useDatabase: () => ({
|
useDatabase: () => mockDb,
|
||||||
products: {
|
|
||||||
add: vi.fn(),
|
|
||||||
update: vi.fn(),
|
|
||||||
delete: productDeleteMock,
|
|
||||||
},
|
|
||||||
pickItems: {
|
|
||||||
where: () => ({
|
|
||||||
equals: () => ({
|
|
||||||
count: pickItemCountMock,
|
|
||||||
}),
|
|
||||||
}),
|
|
||||||
},
|
|
||||||
}),
|
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock('../components/BarcodeScannerView', () => ({
|
vi.mock('../components/BarcodeScannerView', () => ({
|
||||||
BarcodeScannerView: ({ onDetected }: { onDetected?: (code: string) => void }) => (
|
BarcodeScannerView: ({ onDetected }: { onDetected?: (code: string) => void }) => (
|
||||||
<button type="button" onClick={() => onDetected?.('123456')}>
|
<button type="button" onClick={() => onDetected?.(mockScannedBarcode)}>
|
||||||
Mock Scan
|
Mock Scan
|
||||||
</button>
|
</button>
|
||||||
),
|
),
|
||||||
@@ -49,6 +46,18 @@ describe('ManageProductsScreen barcode lookup', () => {
|
|||||||
afterEach(() => server.resetHandlers());
|
afterEach(() => server.resetHandlers());
|
||||||
afterAll(() => server.close());
|
afterAll(() => server.close());
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
mockUseProducts.mockReturnValue([]);
|
||||||
|
mockUseCategories.mockReturnValue([{ id: 'cat-1', name: 'Snacks', created_at: 0, updated_at: 0 }]);
|
||||||
|
mockScannedBarcode = '123456';
|
||||||
|
Object.values(mockDb.products).forEach((fn) => fn.mockReset());
|
||||||
|
mockDb.products.where.mockImplementation(() => ({
|
||||||
|
equals: (value: string) => ({
|
||||||
|
first: () => Promise.resolve(mockUseProducts().find((product: any) => product.barcode === value)),
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
});
|
||||||
|
|
||||||
it('prefills the product name after scanning a barcode', async () => {
|
it('prefills the product name after scanning a barcode', async () => {
|
||||||
server.use(
|
server.use(
|
||||||
http.get('https://world.openfoodfacts.org/api/v2/product/123456.json', () =>
|
http.get('https://world.openfoodfacts.org/api/v2/product/123456.json', () =>
|
||||||
@@ -75,6 +84,84 @@ describe('ManageProductsScreen barcode lookup', () => {
|
|||||||
expect(screen.getByLabelText(/name/i)).toHaveValue('OFF Test Product');
|
expect(screen.getByLabelText(/name/i)).toHaveValue('OFF Test Product');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('prevents adding a product with a duplicate barcode', async () => {
|
||||||
|
mockUseProducts.mockReturnValue([
|
||||||
|
{
|
||||||
|
id: 'prod-1',
|
||||||
|
name: 'Existing Product',
|
||||||
|
category: 'Snacks',
|
||||||
|
barcode: '123456',
|
||||||
|
unit_type: 'unit',
|
||||||
|
bulk_name: 'pack',
|
||||||
|
archived: false,
|
||||||
|
created_at: 0,
|
||||||
|
updated_at: 0,
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(
|
||||||
|
<MemoryRouter>
|
||||||
|
<ManageProductsScreen />
|
||||||
|
</MemoryRouter>,
|
||||||
|
);
|
||||||
|
|
||||||
|
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 }));
|
||||||
|
await waitForElementToBeRemoved(() => screen.queryByRole('dialog', { name: /scan barcode/i }));
|
||||||
|
await user.click(screen.getByRole('button', { name: /save product/i }));
|
||||||
|
|
||||||
|
expect(await screen.findByText(/barcode is already assigned/i)).toBeVisible();
|
||||||
|
expect(mockDb.products.add).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('prevents updating a product to use an existing barcode', async () => {
|
||||||
|
mockUseProducts.mockReturnValue([
|
||||||
|
{
|
||||||
|
id: 'prod-1',
|
||||||
|
name: 'Existing Product',
|
||||||
|
category: 'Snacks',
|
||||||
|
barcode: '123456',
|
||||||
|
unit_type: 'unit',
|
||||||
|
bulk_name: 'pack',
|
||||||
|
archived: false,
|
||||||
|
created_at: 0,
|
||||||
|
updated_at: 0,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 'prod-2',
|
||||||
|
name: 'Another Product',
|
||||||
|
category: 'Snacks',
|
||||||
|
barcode: '654321',
|
||||||
|
unit_type: 'unit',
|
||||||
|
bulk_name: 'pack',
|
||||||
|
archived: false,
|
||||||
|
created_at: 0,
|
||||||
|
updated_at: 0,
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const user = userEvent.setup();
|
||||||
|
render(
|
||||||
|
<MemoryRouter>
|
||||||
|
<ManageProductsScreen />
|
||||||
|
</MemoryRouter>,
|
||||||
|
);
|
||||||
|
|
||||||
|
await user.click(screen.getByLabelText(/edit existing product/i));
|
||||||
|
const barcodeField = screen.getByLabelText(/barcode/i);
|
||||||
|
fireEvent.change(barcodeField, { target: { value: '654321' } });
|
||||||
|
expect(barcodeField).toHaveValue('654321');
|
||||||
|
await user.click(screen.getByLabelText(/save product/i));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(barcodeField).toHaveAccessibleDescription('This barcode is already assigned to another product.');
|
||||||
|
expect(barcodeField).toHaveAttribute('aria-invalid', 'true');
|
||||||
|
});
|
||||||
|
expect(mockDb.products.update).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('ManageProductsScreen deletion safeguards', () => {
|
describe('ManageProductsScreen deletion safeguards', () => {
|
||||||
|
|||||||
@@ -32,6 +32,7 @@ export const ManageProductsScreen = () => {
|
|||||||
const [name, setName] = useState('');
|
const [name, setName] = useState('');
|
||||||
const [category, setCategory] = useState('');
|
const [category, setCategory] = useState('');
|
||||||
const [barcode, setBarcode] = useState('');
|
const [barcode, setBarcode] = useState('');
|
||||||
|
const [barcodeError, setBarcodeError] = useState('');
|
||||||
const [scannerOpen, setScannerOpen] = useState(false);
|
const [scannerOpen, setScannerOpen] = useState(false);
|
||||||
const [lookupStatus, setLookupStatus] = useState<'idle' | 'loading' | 'found' | 'notfound' | 'offline'>(
|
const [lookupStatus, setLookupStatus] = useState<'idle' | 'loading' | 'found' | 'notfound' | 'offline'>(
|
||||||
'idle',
|
'idle',
|
||||||
@@ -69,6 +70,30 @@ export const ManageProductsScreen = () => {
|
|||||||
return Array.from(new Set([...categoryNames, ...productCategories]));
|
return Array.from(new Set([...categoryNames, ...productCategories]));
|
||||||
}, [categories, products]);
|
}, [categories, products]);
|
||||||
|
|
||||||
|
const findBarcodeConflict = useCallback(
|
||||||
|
(value?: string, productId?: string) =>
|
||||||
|
value
|
||||||
|
? products.find((product) => product.barcode === value && product.id !== productId)
|
||||||
|
: undefined,
|
||||||
|
[products],
|
||||||
|
);
|
||||||
|
|
||||||
|
const assertUniqueBarcode = useCallback(
|
||||||
|
async (value?: string, productId?: string) => {
|
||||||
|
if (!value) return;
|
||||||
|
|
||||||
|
const conflict =
|
||||||
|
findBarcodeConflict(value, productId) ?? (await db.products.where('barcode').equals(value).first());
|
||||||
|
|
||||||
|
if (conflict && conflict.id !== productId) {
|
||||||
|
const error = new Error('This barcode is already assigned to another product.');
|
||||||
|
error.name = 'DuplicateBarcodeError';
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
},
|
||||||
|
[db.products, findBarcodeConflict],
|
||||||
|
);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (categoryOptions.length === 0) return;
|
if (categoryOptions.length === 0) return;
|
||||||
if (!categoryOptions.includes(category)) {
|
if (!categoryOptions.includes(category)) {
|
||||||
@@ -97,10 +122,22 @@ export const ManageProductsScreen = () => {
|
|||||||
setLookupStatus('idle');
|
setLookupStatus('idle');
|
||||||
setExternalProduct(null);
|
setExternalProduct(null);
|
||||||
}
|
}
|
||||||
|
setBarcodeError('');
|
||||||
}, [barcode]);
|
}, [barcode]);
|
||||||
|
|
||||||
const addProduct = async () => {
|
const addProduct = async () => {
|
||||||
if (!name || !category) return;
|
if (!name || !category) return;
|
||||||
|
|
||||||
|
try {
|
||||||
|
await assertUniqueBarcode(barcode || undefined);
|
||||||
|
} catch (error) {
|
||||||
|
if (error instanceof Error && error.name === 'DuplicateBarcodeError') {
|
||||||
|
setBarcodeError(error.message);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
|
||||||
await db.products.add({
|
await db.products.add({
|
||||||
id: uuidv4(),
|
id: uuidv4(),
|
||||||
name,
|
name,
|
||||||
@@ -125,6 +162,8 @@ export const ManageProductsScreen = () => {
|
|||||||
barcode?: string;
|
barcode?: string;
|
||||||
},
|
},
|
||||||
) => {
|
) => {
|
||||||
|
await assertUniqueBarcode(updates.barcode, productId);
|
||||||
|
|
||||||
await db.products.update(productId, {
|
await db.products.update(productId, {
|
||||||
...updates,
|
...updates,
|
||||||
unit_type: DEFAULT_UNIT_TYPE,
|
unit_type: DEFAULT_UNIT_TYPE,
|
||||||
@@ -200,6 +239,8 @@ export const ManageProductsScreen = () => {
|
|||||||
label="Barcode"
|
label="Barcode"
|
||||||
value={barcode}
|
value={barcode}
|
||||||
onChange={(event) => setBarcode(event.target.value)}
|
onChange={(event) => setBarcode(event.target.value)}
|
||||||
|
error={Boolean(barcodeError)}
|
||||||
|
helperText={barcodeError || undefined}
|
||||||
InputProps={{
|
InputProps={{
|
||||||
endAdornment: (
|
endAdornment: (
|
||||||
<Button onClick={() => setBarcode('')} size="small">
|
<Button onClick={() => setBarcode('')} size="small">
|
||||||
|
|||||||
Reference in New Issue
Block a user