From 41d08f1286aa6eb611e56dfe738b0931da295b69 Mon Sep 17 00:00:00 2001 From: Nico Date: Sat, 28 Mar 2026 23:00:30 -0700 Subject: [PATCH] fix(ui): keep available item management out of grocery flow --- backend/controllers/lists.controller.v2.js | 15 +- backend/models/list.model.v2.js | 26 ---- backend/tests/list.model.v2.test.js | 42 ----- backend/tests/lists.controller.v2.test.js | 7 - frontend/src/components/forms/AddItemForm.jsx | 67 +------- .../modals/AvailableItemsPickerModal.jsx | 92 ----------- frontend/src/pages/GroceryList.jsx | 78 +--------- .../src/styles/components/AddItemForm.css | 22 --- .../components/AvailableItemsPickerModal.css | 144 ------------------ .../tests/available-items-catalog.spec.ts | 122 ++------------- 10 files changed, 22 insertions(+), 593 deletions(-) delete mode 100644 frontend/src/components/modals/AvailableItemsPickerModal.jsx delete mode 100644 frontend/src/styles/components/AvailableItemsPickerModal.css diff --git a/backend/controllers/lists.controller.v2.js b/backend/controllers/lists.controller.v2.js index 6053dfb..1d6cd0a 100644 --- a/backend/controllers/lists.controller.v2.js +++ b/backend/controllers/lists.controller.v2.js @@ -1,5 +1,4 @@ const List = require("../models/list.model.v2"); -const AvailableItems = require("../models/available-item.model"); const householdModel = require("../models/household.model"); const { isValidItemType, isValidItemGroup, isValidZone } = require("../constants/classifications"); const { sendError } = require("../utils/http"); @@ -120,18 +119,8 @@ exports.addItem = async (req, res) => { } // Get processed image if uploaded - let imageBuffer = req.processedImage?.buffer || null; - let mimeType = req.processedImage?.mimeType || null; - - if (!imageBuffer) { - const catalogItem = await AvailableItems.getAvailableItemImageByName( - householdId, - storeId, - item_name - ); - imageBuffer = catalogItem?.custom_image || null; - mimeType = catalogItem?.custom_image_mime_type || null; - } + const imageBuffer = req.processedImage?.buffer || null; + const mimeType = req.processedImage?.mimeType || null; const result = await List.addOrUpdateItem( householdId, diff --git a/backend/models/list.model.v2.js b/backend/models/list.model.v2.js index 82e775d..ed33d32 100644 --- a/backend/models/list.model.v2.js +++ b/backend/models/list.model.v2.js @@ -269,32 +269,6 @@ exports.addHistoryRecord = async (listId, quantity, userId) => { * @returns {Promise} Suggestions */ exports.getSuggestions = async (query, householdId, storeId) => { - const hasCatalogResult = await pool.query( - `SELECT 1 - FROM household_store_available_items - WHERE household_id = $1 - AND store_id = $2 - LIMIT 1`, - [householdId, storeId] - ); - - if (hasCatalogResult.rowCount > 0) { - const catalogSuggestions = await pool.query( - `SELECT - i.name as item_name, - 0 as sort_order - FROM household_store_available_items hsai - JOIN items i ON i.id = hsai.item_id - WHERE hsai.household_id = $2 - AND hsai.store_id = $3 - AND i.name ILIKE $1 - ORDER BY i.name - LIMIT 10`, - [`%${query}%`, householdId, storeId] - ); - return catalogSuggestions.rows; - } - // Get items from both master catalog and household history const result = await pool.query( `SELECT DISTINCT diff --git a/backend/tests/list.model.v2.test.js b/backend/tests/list.model.v2.test.js index 925eb0c..53d4d11 100644 --- a/backend/tests/list.model.v2.test.js +++ b/backend/tests/list.model.v2.test.js @@ -132,45 +132,3 @@ describe("list.model.v2 classification helpers", () => { ); }); }); - -describe("list.model.v2 suggestions", () => { - beforeEach(() => { - pool.query.mockReset(); - }); - - test("returns catalog suggestions when a household-store catalog exists", async () => { - pool.query - .mockResolvedValueOnce({ rowCount: 1, rows: [{ "?column?": 1 }] }) - .mockResolvedValueOnce({ - rowCount: 1, - rows: [{ item_name: "milk", sort_order: 0 }], - }); - - const result = await List.getSuggestions("mi", 1, 2); - - expect(result).toEqual([{ item_name: "milk", sort_order: 0 }]); - expect(pool.query).toHaveBeenNthCalledWith( - 1, - expect.stringContaining("FROM household_store_available_items"), - [1, 2] - ); - }); - - test("falls back to legacy suggestions when catalog is empty", async () => { - pool.query - .mockResolvedValueOnce({ rowCount: 0, rows: [] }) - .mockResolvedValueOnce({ - rowCount: 1, - rows: [{ item_name: "milk", sort_order: 1 }], - }); - - const result = await List.getSuggestions("mi", 1, 2); - - expect(result).toEqual([{ item_name: "milk", sort_order: 1 }]); - expect(pool.query).toHaveBeenNthCalledWith( - 2, - expect.stringContaining("LEFT JOIN household_lists"), - ["%mi%", 1, 2] - ); - }); -}); diff --git a/backend/tests/lists.controller.v2.test.js b/backend/tests/lists.controller.v2.test.js index 5d6a927..f267645 100644 --- a/backend/tests/lists.controller.v2.test.js +++ b/backend/tests/lists.controller.v2.test.js @@ -9,16 +9,11 @@ jest.mock("../models/household.model", () => ({ isHouseholdMember: jest.fn(), })); -jest.mock("../models/available-item.model", () => ({ - getAvailableItemImageByName: jest.fn(), -})); - jest.mock("../utils/logger", () => ({ logError: jest.fn(), })); const List = require("../models/list.model.v2"); -const AvailableItems = require("../models/available-item.model"); const householdModel = require("../models/household.model"); const controller = require("../controllers/lists.controller.v2"); @@ -41,7 +36,6 @@ describe("lists.controller.v2 addItem", () => { List.addHistoryRecord.mockResolvedValue(undefined); List.getItemByName.mockResolvedValue({ id: 42, item_id: 99, item_name: "milk" }); List.upsertClassification.mockResolvedValue(undefined); - AvailableItems.getAvailableItemImageByName.mockResolvedValue(null); householdModel.isHouseholdMember.mockResolvedValue(true); }); @@ -180,7 +174,6 @@ describe("lists.controller.v2 setClassification", () => { itemName: "milk", isNew: true, }); - AvailableItems.getAvailableItemImageByName.mockResolvedValue(null); }); test("accepts object classification with type, group, and zone", async () => { diff --git a/frontend/src/components/forms/AddItemForm.jsx b/frontend/src/components/forms/AddItemForm.jsx index 0aaf45b..3e5b934 100644 --- a/frontend/src/components/forms/AddItemForm.jsx +++ b/frontend/src/components/forms/AddItemForm.jsx @@ -6,7 +6,6 @@ import SuggestionList from "../items/SuggestionList"; export default function AddItemForm({ onAdd, - onOpenCatalog, onSuggest, suggestions, buttonText = "Add", @@ -19,7 +18,6 @@ export default function AddItemForm({ const [assignmentMode, setAssignmentMode] = useState("me"); const [assignedUserId, setAssignedUserId] = useState(null); const [showAssignModal, setShowAssignModal] = useState(false); - const [pendingAction, setPendingAction] = useState(null); const numericCurrentUserId = currentUserId == null ? null : Number.parseInt(String(currentUserId), 10); @@ -35,31 +33,24 @@ export default function AddItemForm({ return member ? (member.display_name || member.name || member.username || `User ${member.id}`) : ""; }, [assignmentMode, assignedUserId, otherMembers]); - const resetForm = () => { - setItemName(""); - setQuantity(1); - setAssignmentMode("me"); - setAssignedUserId(null); - setShowAssignModal(false); - setPendingAction(null); - }; - const handleSubmit = (e) => { e.preventDefault(); if (!itemName.trim()) return; if (assignmentMode === "others" && assignedUserId == null) { if (otherMembers.length > 0) { - setPendingAction("submit"); setShowAssignModal(true); } return; } - setPendingAction(null); const targetUserId = assignmentMode === "others" ? Number(assignedUserId) : null; onAdd(itemName, quantity, targetUserId); - resetForm(); + setItemName(""); + setQuantity(1); + setAssignmentMode("me"); + setAssignedUserId(null); + setShowAssignModal(false); }; const handleInputChange = (text) => { @@ -103,48 +94,12 @@ export default function AddItemForm({ setShowAssignModal(false); setAssignmentMode("me"); setAssignedUserId(null); - setPendingAction(null); }; const handleAssignConfirm = (memberId) => { setShowAssignModal(false); setAssignmentMode("others"); - const parsedMemberId = Number(memberId); - setAssignedUserId(parsedMemberId); - - if (pendingAction === "submit" && itemName.trim()) { - onAdd(itemName, quantity, parsedMemberId); - resetForm(); - return; - } - - if (pendingAction === "catalog" && onOpenCatalog) { - onOpenCatalog({ - quantity, - addedForUserId: parsedMemberId, - resetForm, - }); - setPendingAction(null); - } - }; - - const handleCatalogOpen = () => { - if (!onOpenCatalog) return; - - if (assignmentMode === "others" && assignedUserId == null) { - if (otherMembers.length > 0) { - setPendingAction("catalog"); - setShowAssignModal(true); - } - return; - } - - setPendingAction(null); - onOpenCatalog({ - quantity, - addedForUserId: assignmentMode === "others" ? Number(assignedUserId) : null, - resetForm, - }); + setAssignedUserId(Number(memberId)); }; const isDisabled = !itemName.trim(); @@ -172,16 +127,6 @@ export default function AddItemForm({ )} - {onOpenCatalog ? ( - - ) : null} - -
event.stopPropagation()}> -
-
-

Store Items

-

- Pick from your household's available items for this store. -

-
- -
- - onQueryChange(event.target.value)} - placeholder="Search available items" - /> - -
- {loading ? ( -

Loading store items...

- ) : items.length === 0 ? ( -

No matching store items found.

- ) : ( - items.map((item) => { - const imageSrc = itemImageSource(item); - return ( - - ); - }) - )} -
-
- - ); -} diff --git a/frontend/src/pages/GroceryList.jsx b/frontend/src/pages/GroceryList.jsx index d0bfaf0..15f22d2 100644 --- a/frontend/src/pages/GroceryList.jsx +++ b/frontend/src/pages/GroceryList.jsx @@ -10,13 +10,11 @@ import { markBought, updateItemWithClassification } from "../api/list"; -import { getAvailableItems } from "../api/availableItems"; import { getHouseholdMembers } from "../api/households"; import SortDropdown from "../components/common/SortDropdown"; import AddItemForm from "../components/forms/AddItemForm"; -import GroceryListItem from "../components/items/GroceryListItem"; +import GroceryListItem from "../components/items/GroceryListItem"; import AddItemWithDetailsModal from "../components/modals/AddItemWithDetailsModal"; -import AvailableItemsPickerModal from "../components/modals/AvailableItemsPickerModal"; import ConfirmAddExistingModal from "../components/modals/ConfirmAddExistingModal"; import EditItemModal from "../components/modals/EditItemModal"; import SimilarItemModal from "../components/modals/SimilarItemModal"; @@ -59,16 +57,11 @@ export default function GroceryList() { const [loading, setLoading] = useState(true); const [buttonText, setButtonText] = useState("Add Item"); const [pendingItem, setPendingItem] = useState(null); - const [showAddDetailsModal, setShowAddDetailsModal] = useState(false); - const [showSimilarModal, setShowSimilarModal] = useState(false); - const [similarItemSuggestion, setSimilarItemSuggestion] = useState(null); + const [showAddDetailsModal, setShowAddDetailsModal] = useState(false); + const [showSimilarModal, setShowSimilarModal] = useState(false); + const [similarItemSuggestion, setSimilarItemSuggestion] = useState(null); const [showEditModal, setShowEditModal] = useState(false); const [editingItem, setEditingItem] = useState(null); - const [showAvailableItemsPicker, setShowAvailableItemsPicker] = useState(false); - const [availableItemsQuery, setAvailableItemsQuery] = useState(""); - const [availableItems, setAvailableItems] = useState([]); - const [availableItemsLoading, setAvailableItemsLoading] = useState(false); - const [availableItemsContext, setAvailableItemsContext] = useState(null); const [recentlyBoughtCollapsed, setRecentlyBoughtCollapsed] = useState(settings.recentlyBoughtCollapsed); const [collapsedZones, setCollapsedZones] = useState({}); const [showConfirmAddExisting, setShowConfirmAddExisting] = useState(false); @@ -132,34 +125,6 @@ export default function GroceryList() { loadHouseholdMembers(); }, [activeHousehold?.id]); - useEffect(() => { - const loadAvailableStoreItems = async () => { - if (!showAvailableItemsPicker) return; - if (!activeHousehold?.id || !activeStore?.id) return; - - setAvailableItemsLoading(true); - try { - const response = await getAvailableItems(activeHousehold.id, activeStore.id, availableItemsQuery); - setAvailableItems(response.data.items || []); - } catch (error) { - console.error("Failed to load available store items:", error); - const message = getApiErrorMessage(error, "Failed to load store items"); - toast.error("Load store items failed", `Load store items failed: ${message}`); - setAvailableItems([]); - } finally { - setAvailableItemsLoading(false); - } - }; - - loadAvailableStoreItems(); - }, [ - activeHousehold?.id, - activeStore?.id, - availableItemsQuery, - showAvailableItemsPicker, - toast, - ]); - useEffect(() => { const handleUploadSuccess = async (event) => { const detail = event?.detail || {}; @@ -333,25 +298,6 @@ export default function GroceryList() { } }, [activeHousehold?.id, activeStore?.id, items, recentlyBoughtItems, buttonText]); - const handleOpenAvailableItemsPicker = useCallback((context) => { - setAvailableItemsContext(context); - setAvailableItemsQuery(""); - setAvailableItems([]); - setShowAvailableItemsPicker(true); - }, []); - - const handleAvailableItemSelect = useCallback(async (item) => { - setShowAvailableItemsPicker(false); - setAvailableItems([]); - setAvailableItemsQuery(""); - - const context = availableItemsContext || {}; - context.resetForm?.(); - setAvailableItemsContext(null); - - await handleAdd(item.item_name, context.quantity || 1, context.addedForUserId || null); - }, [availableItemsContext, handleAdd]); - const processItemAddition = useCallback(async (itemName, quantity, options = {}) => { if (!activeHousehold?.id || !activeStore?.id) return; @@ -811,7 +757,6 @@ export default function GroceryList() { {householdRole && householdRole !== 'viewer' && ( )} - - { - setShowAvailableItemsPicker(false); - setAvailableItemsContext(null); - setAvailableItemsQuery(""); - setAvailableItems([]); - }} - onSelect={handleAvailableItemSelect} - /> ); } diff --git a/frontend/src/styles/components/AddItemForm.css b/frontend/src/styles/components/AddItemForm.css index 4a5bda6..5c455e9 100644 --- a/frontend/src/styles/components/AddItemForm.css +++ b/frontend/src/styles/components/AddItemForm.css @@ -37,23 +37,6 @@ margin: 0; } -.add-item-form-catalog-btn { - flex: 0 0 auto; - min-width: 108px; - border: var(--border-width-thin) solid var(--color-primary); - background: var(--color-bg-surface); - color: var(--color-primary); - border-radius: var(--button-border-radius); - padding: 0 var(--spacing-sm); - font-weight: var(--font-weight-semibold); - cursor: pointer; - transition: var(--transition-base); -} - -.add-item-form-catalog-btn:hover { - background: var(--color-primary-light); -} - .add-item-form-assignee-hint { margin: 0; font-size: var(--font-size-xs); @@ -221,11 +204,6 @@ width: 100px; } - .add-item-form-catalog-btn { - min-width: 96px; - font-size: var(--font-size-sm); - } - .add-item-form-quantity-control { height: 36px; } diff --git a/frontend/src/styles/components/AvailableItemsPickerModal.css b/frontend/src/styles/components/AvailableItemsPickerModal.css deleted file mode 100644 index f5b5c05..0000000 --- a/frontend/src/styles/components/AvailableItemsPickerModal.css +++ /dev/null @@ -1,144 +0,0 @@ -.available-items-picker-overlay { - position: fixed; - inset: 0; - background: var(--modal-backdrop-bg); - display: flex; - align-items: center; - justify-content: center; - z-index: var(--z-modal); - padding: var(--spacing-md); -} - -.available-items-picker-modal { - width: min(680px, 100%); - max-height: 90vh; - overflow: hidden; - display: flex; - flex-direction: column; - background: var(--modal-bg); - border-radius: var(--border-radius-xl); - box-shadow: var(--shadow-xl); - padding: var(--spacing-lg); -} - -.available-items-picker-header { - display: flex; - align-items: flex-start; - justify-content: space-between; - gap: var(--spacing-md); -} - -.available-items-picker-title { - margin: 0; - color: var(--color-text-primary); - font-size: var(--font-size-xl); -} - -.available-items-picker-subtitle { - margin: var(--spacing-xs) 0 0; - color: var(--color-text-secondary); -} - -.available-items-picker-close { - border: none; - background: transparent; - color: var(--color-text-secondary); - font-size: var(--font-size-xl); - cursor: pointer; -} - -.available-items-picker-search { - margin-top: var(--spacing-md); - width: 100%; - box-sizing: border-box; - padding: var(--input-padding-y) var(--input-padding-x); - border: var(--border-width-thin) solid var(--input-border-color); - border-radius: var(--input-border-radius); - background: var(--color-bg-surface); - color: var(--color-text-primary); -} - -.available-items-picker-search:focus { - outline: none; - border-color: var(--input-focus-border-color); - box-shadow: var(--input-focus-shadow); -} - -.available-items-picker-list { - margin-top: var(--spacing-md); - overflow-y: auto; - display: flex; - flex-direction: column; - gap: var(--spacing-sm); -} - -.available-items-picker-item { - display: flex; - align-items: center; - gap: var(--spacing-md); - width: 100%; - text-align: left; - background: var(--color-bg-surface); - border: var(--border-width-thin) solid var(--color-border-light); - border-radius: var(--border-radius-lg); - padding: var(--spacing-md); - cursor: pointer; - transition: var(--transition-base); -} - -.available-items-picker-item:hover { - border-color: var(--color-primary); - box-shadow: var(--shadow-md); - transform: translateY(-1px); -} - -.available-items-picker-thumb { - width: 56px; - height: 56px; - border-radius: var(--border-radius-md); - object-fit: cover; - flex-shrink: 0; - background: var(--color-bg-muted); -} - -.available-items-picker-thumb-placeholder { - display: inline-flex; - align-items: center; - justify-content: center; - color: var(--color-text-secondary); - font-weight: var(--font-weight-semibold); -} - -.available-items-picker-copy { - display: flex; - flex-direction: column; - gap: var(--spacing-xs); - min-width: 0; -} - -.available-items-picker-name { - color: var(--color-text-primary); - font-weight: var(--font-weight-semibold); -} - -.available-items-picker-meta { - color: var(--color-text-secondary); - font-size: var(--font-size-sm); -} - -.available-items-picker-empty { - margin: 0; - padding: var(--spacing-lg) 0; - text-align: center; - color: var(--color-text-secondary); -} - -@media (max-width: 640px) { - .available-items-picker-modal { - padding: var(--spacing-md); - } - - .available-items-picker-item { - padding: var(--spacing-sm); - } -} diff --git a/frontend/tests/available-items-catalog.spec.ts b/frontend/tests/available-items-catalog.spec.ts index 891b36e..229c96c 100644 --- a/frontend/tests/available-items-catalog.spec.ts +++ b/frontend/tests/available-items-catalog.spec.ts @@ -179,57 +179,18 @@ test("manage stores lets admins import and curate available items", async ({ pag await expect(page.locator(".action-toast.action-toast-success")).toContainText("Removed store item"); }); -test("grocery picker uses available items and preserves quantity and assignee", async ({ page }) => { +test("grocery page remains unchanged and does not show a store items picker", async ({ page }) => { await seedAuthStorage(page); await mockConfig(page); await mockHouseholdAndStoreShell(page); - const members = [ - { id: 1, username: "owner", name: "Owner User", display_name: "Owner User", role: "owner" }, - { id: 2, username: "casey", name: "Casey Client", display_name: "Casey Client", role: "member" }, - ]; - - let lastAddBody = ""; - let currentItems: Array<{ - 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; - }> = []; - await page.route("**/households/1/members", async (route) => { await route.fulfill({ status: 200, contentType: "application/json", - body: JSON.stringify(members), - }); - }); - - await page.route("**/households/1/stores/10/available-items*", async (route) => { - await route.fulfill({ - status: 200, - contentType: "application/json", - body: JSON.stringify({ - items: [ - { - item_id: 600, - item_name: "bananas", - item_image: null, - image_mime_type: null, - item_type: "produce", - item_group: "Fresh Fruit", - zone: "Fresh Produce", - }, - ], - }), + body: JSON.stringify([ + { id: 1, username: "owner", name: "Owner User", display_name: "Owner User", role: "owner" }, + ]), }); }); @@ -245,59 +206,15 @@ test("grocery picker uses available items and preserves quantity and assignee", await route.fulfill({ status: 200, contentType: "application/json", - body: JSON.stringify([{ item_name: "bananas" }]), + body: JSON.stringify([]), }); }); await page.route("**/households/1/stores/10/list/item**", async (route) => { - const request = route.request(); - const url = new URL(request.url()); - const itemName = (url.searchParams.get("item_name") || "").toLowerCase(); - const item = currentItems.find((candidate) => candidate.item_name === itemName); - - if (request.method() === "GET") { - await route.fulfill({ - status: item ? 200 : 404, - contentType: "application/json", - body: JSON.stringify(item || { message: "Item not found" }), - }); - return; - } - - await route.fulfill({ status: 500 }); - }); - - await page.route("**/households/1/stores/10/list/add", async (route) => { - lastAddBody = route.request().postData() || ""; - currentItems = [ - { - id: 201, - item_id: 600, - item_name: "bananas", - quantity: 3, - bought: false, - item_image: null, - image_mime_type: null, - added_by_users: ["Casey Client"], - last_added_on: "2026-03-28T12:00:00.000Z", - item_type: "produce", - item_group: "Fresh Fruit", - zone: "Fresh Produce", - }, - ]; - await route.fulfill({ - status: 200, + status: 404, contentType: "application/json", - body: JSON.stringify({ - message: "Item added", - item: { - id: 201, - item_name: "bananas", - quantity: 3, - bought: false, - }, - }), + body: JSON.stringify({ message: "Item not found" }), }); }); @@ -305,31 +222,12 @@ test("grocery picker uses available items and preserves quantity and assignee", await route.fulfill({ status: 200, contentType: "application/json", - body: JSON.stringify({ items: currentItems }), + body: JSON.stringify({ items: [] }), }); }); await page.goto("/"); - await page.getByRole("button", { name: "Others" }).click(); - const assignModal = page.locator(".assign-item-for-modal"); - await assignModal.getByRole("button", { name: "Select member" }).click(); - await page.locator("body > .assign-item-for-dropdown-menu").getByRole("option", { name: "Casey Client" }).click(); - await assignModal.getByRole("button", { name: "Confirm" }).click(); - - await page.getByRole("button", { name: "+" }).click(); - await page.getByRole("button", { name: "+" }).click(); - await expect(page.locator(".add-item-form-quantity-input")).toHaveValue("3"); - - await page.getByRole("button", { name: "Store Items" }).click(); - const pickerModal = page.locator(".available-items-picker-modal"); - await expect(pickerModal).toBeVisible(); - await pickerModal.getByRole("button", { name: /bananas/i }).click(); - - await page.getByRole("button", { name: "Skip All" }).click(); - await expect(page.locator(".glist-li").filter({ hasText: "bananas" })).toContainText("Casey Client"); - expect(lastAddBody).toContain('name="quantity"'); - expect(lastAddBody).toContain("3"); - expect(lastAddBody).toContain('name="added_for_user_id"'); - expect(lastAddBody).toContain("2"); + await expect(page.getByRole("button", { name: "Store Items" })).toHaveCount(0); + await expect(page.locator(".available-items-picker-modal")).toHaveCount(0); });