From 104519668a1c803f895bc51116818be012b9bdac Mon Sep 17 00:00:00 2001 From: Nico Date: Sat, 28 Mar 2026 22:50:52 -0700 Subject: [PATCH] fix(ui): restore classification detail modal flow --- frontend/src/api/list.js | 64 ++-- .../modals/AddItemWithDetailsModal.jsx | 10 +- .../src/components/modals/EditItemModal.jsx | 50 +-- .../components/AddItemWithDetailsModal.css | 102 +++--- frontend/tests/classification-details.spec.ts | 318 ++++++++++++++++++ 5 files changed, 446 insertions(+), 98 deletions(-) create mode 100644 frontend/tests/classification-details.spec.ts diff --git a/frontend/src/api/list.js b/frontend/src/api/list.js index e75d3fa..259d402 100644 --- a/frontend/src/api/list.js +++ b/frontend/src/api/list.js @@ -54,25 +54,51 @@ export const getClassification = (householdId, storeId, itemName) => params: { item_name: itemName } }); -/** - * Set item classification - */ -export const setClassification = (householdId, storeId, itemName, classification) => - api.post(`/households/${householdId}/stores/${storeId}/list/classification`, { - item_name: itemName, - classification - }); - -/** - * Update item with classification (legacy method - split into separate calls) - */ -export const updateItemWithClassification = (householdId, storeId, itemName, quantity, classification) => { - // This is now two operations: update item + set classification - return Promise.all([ - updateItem(householdId, storeId, itemName, quantity), - classification ? setClassification(householdId, storeId, itemName, classification) : Promise.resolve() - ]); -}; +/** + * Set item classification + */ +export const setClassification = (householdId, storeId, itemName, classification) => + api.post(`/households/${householdId}/stores/${storeId}/list/classification`, { + item_name: itemName, + classification + }); + +function normalizeClassificationPayload(classification) { + if (!classification) return null; + if (typeof classification === "string") { + return classification.trim() || null; + } + if (typeof classification !== "object" || Array.isArray(classification)) { + return null; + } + + const payload = { + item_type: typeof classification.item_type === "string" && classification.item_type.trim() + ? classification.item_type.trim() + : null, + item_group: typeof classification.item_group === "string" && classification.item_group.trim() + ? classification.item_group.trim() + : null, + zone: typeof classification.zone === "string" && classification.zone.trim() + ? classification.zone.trim() + : null, + }; + + return payload.item_type || payload.item_group || payload.zone ? payload : null; +} + +/** + * Update item with optional classification details. + */ +export const updateItemWithClassification = (householdId, storeId, itemName, quantity, classification) => { + const normalizedClassification = normalizeClassificationPayload(classification); + return Promise.all([ + updateItem(householdId, storeId, itemName, quantity), + normalizedClassification + ? setClassification(householdId, storeId, itemName, normalizedClassification) + : Promise.resolve() + ]); +}; /** * Update item details (quantity, notes) diff --git a/frontend/src/components/modals/AddItemWithDetailsModal.jsx b/frontend/src/components/modals/AddItemWithDetailsModal.jsx index b9e31fd..fab0ab9 100644 --- a/frontend/src/components/modals/AddItemWithDetailsModal.jsx +++ b/frontend/src/components/modals/AddItemWithDetailsModal.jsx @@ -1,9 +1,11 @@ import { useState } from "react"; import "../../styles/components/AddItemWithDetailsModal.css"; import ClassificationSection from "../forms/ClassificationSection"; +import useActionToast from "../../hooks/useActionToast"; import ImageUploadSection from "../forms/ImageUploadSection"; export default function AddItemWithDetailsModal({ itemName, onConfirm, onSkip, onCancel }) { + const toast = useActionToast(); const [selectedImage, setSelectedImage] = useState(null); const [imagePreview, setImagePreview] = useState(null); const [itemType, setItemType] = useState(""); @@ -30,15 +32,15 @@ export default function AddItemWithDetailsModal({ itemName, onConfirm, onSkip, o }; const handleConfirm = () => { - // Validate classification if provided if (itemType && !itemGroup) { - alert("Please select an item group"); + toast.error("Add item failed", `Add item failed: Select an item group for ${itemName}`); return; } - const classification = itemType ? { + const hasClassificationDetails = Boolean(itemType || itemGroup || zone); + const classification = hasClassificationDetails ? { item_type: itemType, - item_group: itemGroup, + item_group: itemGroup || null, zone: zone || null } : null; diff --git a/frontend/src/components/modals/EditItemModal.jsx b/frontend/src/components/modals/EditItemModal.jsx index 22f38a9..b56b236 100644 --- a/frontend/src/components/modals/EditItemModal.jsx +++ b/frontend/src/components/modals/EditItemModal.jsx @@ -1,7 +1,6 @@ import { useEffect, useState } from "react"; import { ITEM_GROUPS, ITEM_TYPES, getItemTypeLabel, getZoneValues } from "../../constants/classifications"; import useActionToast from "../../hooks/useActionToast"; -import getApiErrorMessage from "../../lib/getApiErrorMessage"; import "../../styles/components/EditItemModal.css"; import AddImageModal from "./AddImageModal"; @@ -15,50 +14,54 @@ export default function EditItemModal({ item, onSave, onCancel, onImageUpdate }) const [loading, setLoading] = useState(false); const [showImageModal, setShowImageModal] = useState(false); - // Load existing classification useEffect(() => { if (item.classification) { setItemType(item.classification.item_type || ""); setItemGroup(item.classification.item_group || ""); setZone(item.classification.zone || ""); + return; } + + setItemType(""); + setItemGroup(""); + setZone(""); }, [item]); const handleItemTypeChange = (newType) => { setItemType(newType); - setItemGroup(""); // Reset group when type changes + setItemGroup(""); }; const handleSave = async () => { if (!itemName.trim()) { - alert("Item name is required"); + toast.error("Save item failed", "Save item failed: Item name is required"); return; } if (quantity < 1) { - alert("Quantity must be at least 1"); + toast.error("Save item failed", "Save item failed: Quantity must be at least 1"); return; } - // If classification fields are filled, validate them if (itemType && !itemGroup) { - alert("Please select an item group"); + toast.error("Save item failed", `Save item failed: Select an item group for ${itemName}`); return; } setLoading(true); try { - const classification = itemType ? { - item_type: itemType, - item_group: itemGroup, - zone: zone || null - } : null; + const hasClassificationDetails = Boolean(itemType || itemGroup || zone); + const classification = hasClassificationDetails + ? { + item_type: itemType, + item_group: itemGroup || null, + zone: zone || null + } + : null; await onSave(item.id, itemName, quantity, classification); } catch (error) { console.error("Failed to save:", error); - const message = getApiErrorMessage(error, "Failed to save changes"); - toast.error("Save item failed", `Save item failed: ${message}`); } finally { setLoading(false); } @@ -71,18 +74,18 @@ export default function EditItemModal({ item, onSave, onCancel, onImageUpdate }) setShowImageModal(false); } catch (error) { console.error("Failed to upload image:", error); - const message = getApiErrorMessage(error, "Failed to upload image"); + const message = error?.response?.data?.error?.message || error?.response?.data?.message || "Failed to upload image"; toast.error("Upload image failed", `Upload image failed: ${message}`); } } }; const incrementQuantity = () => { - setQuantity(prev => prev + 1); + setQuantity((prev) => prev + 1); }; const decrementQuantity = () => { - setQuantity(prev => Math.max(1, prev - 1)); + setQuantity((prev) => Math.max(1, prev - 1)); }; const availableGroups = itemType ? (ITEM_GROUPS[itemType] || []) : []; @@ -92,7 +95,6 @@ export default function EditItemModal({ item, onSave, onCancel, onImageUpdate })
e.stopPropagation()}>

Edit Item

- {/* Item Name - no label */} - {/* Quantity Control - like AddItemForm */}
- {/* Inline Classification Fields */}
@@ -188,7 +188,7 @@ export default function EditItemModal({ item, onSave, onCancel, onImageUpdate }) disabled={loading} type="button" > - {item.item_image ? "🖼️ Change Image" : "📷 Set Image"} + {item.item_image ? "Change Image" : "Set Image"}
diff --git a/frontend/src/styles/components/AddItemWithDetailsModal.css b/frontend/src/styles/components/AddItemWithDetailsModal.css index 071f1be..500981c 100644 --- a/frontend/src/styles/components/AddItemWithDetailsModal.css +++ b/frontend/src/styles/components/AddItemWithDetailsModal.css @@ -61,7 +61,7 @@ .add-item-details-image-options { display: flex; - gap: 0.8em; + gap: var(--spacing-sm); flex-wrap: wrap; } @@ -69,7 +69,7 @@ flex: 1; min-width: 140px; padding: var(--button-padding-y) var(--button-padding-x); - font-size: 0.95em; + font-size: var(--font-size-base); border: var(--border-width-medium) solid var(--color-primary); background: var(--color-bg-surface); color: var(--color-primary); @@ -101,97 +101,99 @@ .add-item-details-remove-image { position: absolute; - top: 0.5em; - right: 0.5em; - background: rgba(220, 53, 69, 0.9); - color: white; + top: var(--spacing-sm); + right: var(--spacing-sm); + background: var(--color-danger); + color: var(--color-text-inverse); border: none; - border-radius: 6px; - padding: 0.4em 0.8em; + border-radius: var(--border-radius-md); + padding: var(--spacing-sm) var(--spacing-md); cursor: pointer; - font-weight: 600; - font-size: 0.9em; - transition: background 0.2s; + font-weight: var(--font-weight-semibold); + font-size: var(--font-size-sm); + transition: var(--transition-base); } .add-item-details-remove-image:hover { - background: rgba(220, 53, 69, 1); + background: var(--color-danger-hover); } /* Classification Section */ .add-item-details-field { - margin-bottom: 1em; + margin-bottom: var(--spacing-md); } .add-item-details-field label { display: block; - margin-bottom: 0.4em; - font-weight: 600; - color: #333; - font-size: 0.9em; + margin-bottom: var(--spacing-sm); + font-weight: var(--font-weight-semibold); + color: var(--color-text-primary); + font-size: var(--font-size-sm); } .add-item-details-select { width: 100%; - padding: 0.6em; - font-size: 1em; - border: 1px solid #ccc; - border-radius: 6px; + padding: var(--input-padding-y) var(--input-padding-x); + font-size: var(--font-size-base); + border: var(--border-width-thin) solid var(--input-border-color); + border-radius: var(--input-border-radius); box-sizing: border-box; - transition: border-color 0.2s; - background: white; + transition: var(--transition-base); + background: var(--color-bg-surface); + color: var(--color-text-primary); } .add-item-details-select:focus { outline: none; - border-color: #007bff; + border-color: var(--input-focus-border-color); + box-shadow: var(--input-focus-shadow); } /* Actions */ .add-item-details-actions { display: flex; - gap: 0.6em; - margin-top: 1.5em; - padding-top: 1em; - border-top: 1px solid #e0e0e0; + gap: var(--spacing-sm); + margin-top: var(--spacing-lg); + padding-top: var(--spacing-md); + border-top: var(--border-width-thin) solid var(--color-border-light); } .add-item-details-btn { flex: 1; - padding: 0.7em; - font-size: 1em; + padding: var(--button-padding-y) var(--button-padding-x); + font-size: var(--font-size-base); border: none; - border-radius: 6px; + border-radius: var(--button-border-radius); cursor: pointer; - font-weight: 600; - transition: all 0.2s; + font-weight: var(--button-font-weight); + transition: var(--transition-base); } .add-item-details-btn.cancel { - background: #6c757d; - color: white; + background: var(--color-secondary); + color: var(--color-text-inverse); } .add-item-details-btn.cancel:hover { - background: #5a6268; + background: var(--color-secondary-hover); } .add-item-details-btn.skip { - background: #ffc107; - color: #333; + background: var(--color-warning); + color: var(--color-text-primary); } .add-item-details-btn.skip:hover { - background: #e0a800; + background: var(--color-warning-hover); } .add-item-details-btn.confirm { - background: #007bff; - color: white; + background: var(--color-primary); + color: var(--color-text-inverse); } .add-item-details-btn.confirm:hover { - background: #0056b3; + background: var(--color-primary-hover); } /* Mobile responsiveness */ @@ -207,7 +209,7 @@ } .add-item-details-title { - font-size: 1.3em; + font-size: var(--font-size-xl); } .add-item-details-select { @@ -236,20 +238,20 @@ @media (max-width: 480px) { .add-item-details-modal { - padding: 1rem; - border-radius: 8px; + padding: var(--spacing-md); + border-radius: var(--border-radius-lg); } .add-item-details-title { - font-size: 1.15em; + font-size: var(--font-size-lg); } .add-item-details-subtitle { - font-size: 0.85em; + font-size: var(--font-size-sm); } .add-item-details-section-title { - font-size: 1em; + font-size: var(--font-size-base); } .add-item-details-image-options { @@ -258,10 +260,10 @@ .add-item-details-image-btn { min-width: 100%; - font-size: 0.9em; + font-size: var(--font-size-sm); } .add-item-details-field label { - font-size: 0.85em; + font-size: var(--font-size-sm); } } diff --git a/frontend/tests/classification-details.spec.ts b/frontend/tests/classification-details.spec.ts new file mode 100644 index 0000000..8f59c3c --- /dev/null +++ b/frontend/tests/classification-details.spec.ts @@ -0,0 +1,318 @@ +import { expect, test } from "@playwright/test"; + +function seedAuthStorage(page: import("@playwright/test").Page) { + return page.addInitScript(() => { + localStorage.setItem("token", "test-token"); + localStorage.setItem("userId", "1"); + localStorage.setItem("role", "admin"); + localStorage.setItem("username", "classification-user"); + }); +} + +async function mockConfig(page: import("@playwright/test").Page) { + await page.route("**/config", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + maxFileSizeMB: 20, + maxImageDimension: 800, + imageQuality: 85, + }), + }); + }); +} + +async function setupGroceryListRoutes(page: import("@playwright/test").Page) { + let currentItem: { + id: number; + item_id: number; + item_name: string; + quantity: number; + bought: boolean; + item_image: string | null; + image_mime_type: string | null; + added_by_users: string[]; + last_added_on: string; + item_type: string | null; + item_group: string | null; + zone: string | null; + } | null = null; + let currentClassification: { + item_type: string | null; + item_group: string | null; + zone: string | null; + } | null = null; + let classificationRequestMode: "success" | "error" = "success"; + + await page.route("**/households", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([ + { id: 1, name: "Classification House", role: "admin", invite_code: "ABCD1234" }, + ]), + }); + }); + + await page.route("**/stores/household/1", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([ + { id: 10, name: "Costco", location: "Warehouse", is_default: true }, + ]), + }); + }); + + await page.route("**/households/1/members", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([ + { id: 1, username: "owner", name: "Owner User", display_name: "Owner User", role: "owner" }, + ]), + }); + }); + + await page.route("**/households/1/stores/10/list/recent", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([]), + }); + }); + + await page.route("**/households/1/stores/10/list/suggestions**", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([]), + }); + }); + + await page.route("**/households/1/stores/10/list/classification**", async (route) => { + const request = route.request(); + + if (request.method() === "GET") { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ classification: currentClassification }), + }); + return; + } + + const body = request.postDataJSON() as { + classification?: string | { item_type?: string | null; item_group?: string | null; zone?: string | null }; + }; + + if (classificationRequestMode === "error") { + await route.fulfill({ + status: 400, + contentType: "application/json", + body: JSON.stringify({ + error: { message: "Invalid zone" }, + }), + }); + return; + } + + const payload = typeof body.classification === "string" + ? { item_type: body.classification, item_group: null, zone: null } + : { + item_type: body.classification?.item_type ?? null, + item_group: body.classification?.item_group ?? null, + zone: body.classification?.zone ?? null, + }; + + currentClassification = payload; + if (currentItem) { + currentItem = { + ...currentItem, + item_type: payload.item_type, + item_group: payload.item_group, + zone: payload.zone, + }; + } + + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + message: "Classification set", + classification: payload, + }), + }); + }); + + await page.route("**/households/1/stores/10/list/item**", async (route) => { + const request = route.request(); + + if (request.method() === "PUT") { + const body = request.postDataJSON() as { item_name?: string; quantity?: number }; + if (currentItem) { + currentItem = { + ...currentItem, + item_name: String(body.item_name || currentItem.item_name).toLowerCase(), + quantity: Number(body.quantity || currentItem.quantity), + }; + } + + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + message: "Item updated", + item: { + id: currentItem?.id || 201, + item_name: currentItem?.item_name || "yogurt", + quantity: currentItem?.quantity || 1, + }, + }), + }); + return; + } + + const url = new URL(request.url()); + const itemName = (url.searchParams.get("item_name") || "").toLowerCase(); + const itemMatches = currentItem && currentItem.item_name === itemName; + + await route.fulfill({ + status: itemMatches ? 200 : 404, + contentType: "application/json", + body: JSON.stringify(itemMatches ? currentItem : { message: "Item not found" }), + }); + }); + + await page.route("**/households/1/stores/10/list/add", async (route) => { + currentItem = { + id: 201, + item_id: 501, + item_name: "yogurt", + quantity: 1, + bought: false, + item_image: null, + image_mime_type: null, + added_by_users: ["Owner User"], + last_added_on: "2026-03-28T12:00:00.000Z", + item_type: currentClassification?.item_type ?? null, + item_group: currentClassification?.item_group ?? null, + zone: currentClassification?.zone ?? null, + }; + + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + message: "Item added", + item: { + id: 201, + item_name: "yogurt", + quantity: 1, + bought: false, + }, + }), + }); + }); + + await page.route("**/households/1/stores/10/list", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + items: currentItem ? [currentItem] : [], + }), + }); + }); + + return { + setClassificationRequestMode(mode: "success" | "error") { + classificationRequestMode = mode; + }, + }; +} + +async function openEditModal(itemRow: ReturnType, page: import("@playwright/test").Page) { + await itemRow.dispatchEvent("mousedown"); + await page.waitForTimeout(650); + await itemRow.dispatchEvent("mouseup"); + await expect(page.locator(".edit-modal-content")).toBeVisible(); +} + +test("add-details modal validates with toasts and persists classification details", async ({ page }) => { + await seedAuthStorage(page); + await mockConfig(page); + await setupGroceryListRoutes(page); + + let dialogSeen = false; + page.on("dialog", async (dialog) => { + dialogSeen = true; + await dialog.dismiss(); + }); + + await page.goto("/"); + + await page.getByPlaceholder("Enter item name").fill("yogurt"); + await page.getByRole("button", { name: "Create + Add" }).click(); + + const addDetailsModal = page.locator(".add-item-details-modal"); + await expect(addDetailsModal).toBeVisible(); + + await addDetailsModal.locator(".add-item-details-select").nth(0).selectOption("dairy"); + await addDetailsModal.getByRole("button", { name: "Add Item" }).click(); + + await expect(page.locator(".action-toast.action-toast-error")).toContainText("Select an item group"); + expect(dialogSeen).toBe(false); + + await addDetailsModal.locator(".add-item-details-select").nth(1).selectOption("Milk"); + await addDetailsModal.locator(".add-item-details-select").nth(2).selectOption("Dairy & Refrigerated"); + await addDetailsModal.getByRole("button", { name: "Add Item" }).click(); + + const yogurtRow = page.locator(".glist-li").filter({ hasText: "yogurt" }); + await expect(yogurtRow).toBeVisible(); + await expect(page.locator(".action-toast.action-toast-success")).toContainText("Added item"); + + await openEditModal(yogurtRow, page); + + const editModal = page.locator(".edit-modal-content"); + await expect(editModal.locator(".edit-modal-select").nth(0)).toHaveValue("dairy"); + await expect(editModal.locator(".edit-modal-select").nth(1)).toHaveValue("Milk"); + await expect(editModal.locator(".edit-modal-select").nth(2)).toHaveValue("Dairy & Refrigerated"); +}); + +test("edit modal supports zone-only updates and shows API error toasts", async ({ page }) => { + await seedAuthStorage(page); + await mockConfig(page); + const routes = await setupGroceryListRoutes(page); + + await page.goto("/"); + + await page.getByPlaceholder("Enter item name").fill("yogurt"); + await page.getByRole("button", { name: "Create + Add" }).click(); + await page.locator(".add-item-details-modal").getByRole("button", { name: "Skip All" }).click(); + + const yogurtRow = page.locator(".glist-li").filter({ hasText: "yogurt" }); + await expect(yogurtRow).toBeVisible(); + + await openEditModal(yogurtRow, page); + + let editModal = page.locator(".edit-modal-content"); + await editModal.locator(".edit-modal-select").nth(0).selectOption(""); + await editModal.locator(".edit-modal-select").nth(1).selectOption("Checkout Area"); + await editModal.getByRole("button", { name: "Save Changes" }).click(); + + await expect(page.locator(".action-toast.action-toast-success")).toContainText("Updated item"); + await expect(editModal).toBeHidden(); + + await openEditModal(yogurtRow, page); + editModal = page.locator(".edit-modal-content"); + await expect(editModal.locator(".edit-modal-select").nth(0)).toHaveValue(""); + await expect(editModal.locator(".edit-modal-select").nth(1)).toHaveValue("Checkout Area"); + + routes.setClassificationRequestMode("error"); + await editModal.locator(".edit-modal-select").nth(1).selectOption("Bakery"); + await editModal.getByRole("button", { name: "Save Changes" }).click(); + + await expect(page.locator(".action-toast.action-toast-error")).toContainText("Invalid zone"); +});