From e6d08c02a62c2fe5bca9583ee2489375c3af5e22 Mon Sep 17 00:00:00 2001 From: Brian Richter Date: Fri, 31 Jul 2026 01:38:19 -0700 Subject: [PATCH] feat(categories): make a list's item categories editable and its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The categorised view offered 16 hardcoded grocery aisles to every list. On a packing list that meant every item piled into "Other" (no grocery keyword matches "Travel pillow") while the only categories on offer were Produce, Deli and Meat & Seafood. Categories could be added but never renamed, reordered or deleted — removeCustomAisle had existed for a while with no UI at all. A list now owns its categories. Absent `itemCategories` means it is still on the grocery defaults; the first edit MATERIALISES the full set onto the list, folding in any existing customAisles, and from then on that set is the whole truth for the list and grocery aisles stop being offered. Auto-classification is kept and gated on reachability: a keyword guess is used only if the list's set still contains that category. So a grocery list keeps auto-filing bananas → Produce even after adding a category of its own, while a packing list that has dropped the grocery aisles stops guessing rather than filing "Sunglasses" into Produce. An explicit assignment always wins, and an assignment to a deleted category degrades to Other instead of vanishing the item into a group nothing renders. Every set keeps a permanent Other bucket: it cannot be renamed, deleted or reordered, so items always have somewhere to land. Deleting a category reassigns its items to Other rather than leaving a dangling id that would resurrect the group if the name were reused. The rules live in convex/lib/itemCategories so the mutations enforce exactly what the UI shows instead of a second, drifting copy; only the grocery keyword map stays client-side, since nothing on the server needs it. Editing is inline on the section header via a ⋯ menu — rename and emoji edit in place, so a one-word fix costs one tap and no navigation. Named itemCategories, not categories: `lists.categoryId` already means the user's folder for the LIST, a different concept on the same row. 32 new tests cover the rules and the mutations, including the regression that materialising must not silently switch grocery auto-filing off. Co-Authored-By: Claude Opus 5 (1M context) --- convex/_generated/api.d.ts | 4 + convex/itemCategories.ts | 117 ++++++++++ convex/lib/itemCategories.ts | 181 ++++++++++++++++ convex/schema.ts | 14 +- scripts/categories.test.mjs | 239 +++++++++++++++++++++ scripts/item-categories-mutations.test.mjs | 177 +++++++++++++++ src/components/CategoryHeaderMenu.tsx | 192 +++++++++++++++++ src/lib/categories.ts | 65 ++++++ src/pages/ListView.tsx | 61 ++++-- 9 files changed, 1031 insertions(+), 19 deletions(-) create mode 100644 convex/itemCategories.ts create mode 100644 convex/lib/itemCategories.ts create mode 100644 scripts/categories.test.mjs create mode 100644 scripts/item-categories-mutations.test.mjs create mode 100644 src/components/CategoryHeaderMenu.tsx create mode 100644 src/lib/categories.ts diff --git a/convex/_generated/api.d.ts b/convex/_generated/api.d.ts index 1232226..5b9ddbe 100644 --- a/convex/_generated/api.d.ts +++ b/convex/_generated/api.d.ts @@ -36,6 +36,7 @@ import type * as didResources from "../didResources.js"; import type * as didResourcesHttp from "../didResourcesHttp.js"; import type * as feedback from "../feedback.js"; import type * as http from "../http.js"; +import type * as itemCategories from "../itemCategories.js"; import type * as items from "../items.js"; import type * as itemsHttp from "../itemsHttp.js"; import type * as lib_actor from "../lib/actor.js"; @@ -45,6 +46,7 @@ import type * as lib_auth from "../lib/auth.js"; import type * as lib_authUser from "../lib/authUser.js"; import type * as lib_bucket from "../lib/bucket.js"; import type * as lib_httpResponses from "../lib/httpResponses.js"; +import type * as lib_itemCategories from "../lib/itemCategories.js"; import type * as lib_jwt from "../lib/jwt.js"; import type * as lib_listEnvelope from "../lib/listEnvelope.js"; import type * as lib_observability from "../lib/observability.js"; @@ -112,6 +114,7 @@ declare const fullApi: ApiFromModules<{ didResourcesHttp: typeof didResourcesHttp; feedback: typeof feedback; http: typeof http; + itemCategories: typeof itemCategories; items: typeof items; itemsHttp: typeof itemsHttp; "lib/actor": typeof lib_actor; @@ -121,6 +124,7 @@ declare const fullApi: ApiFromModules<{ "lib/authUser": typeof lib_authUser; "lib/bucket": typeof lib_bucket; "lib/httpResponses": typeof lib_httpResponses; + "lib/itemCategories": typeof lib_itemCategories; "lib/jwt": typeof lib_jwt; "lib/listEnvelope": typeof lib_listEnvelope; "lib/observability": typeof lib_observability; diff --git a/convex/itemCategories.ts b/convex/itemCategories.ts new file mode 100644 index 0000000..4d50717 --- /dev/null +++ b/convex/itemCategories.ts @@ -0,0 +1,117 @@ +/** + * Mutations for a list's item categories. + * + * Every edit follows the same shape: materialise the list's set (a no-op once it + * owns one), apply a pure operation from lib/itemCategories, and persist. The + * rules live in that shared module so the server enforces exactly what the UI + * shows rather than a second, drifting copy. + */ + +import { v } from "convex/values"; +import { mutation } from "./_generated/server"; +import type { MutationCtx } from "./_generated/server"; +import type { Id } from "./_generated/dataModel"; +import { canUserEditList } from "./lib/permissions"; +import { + type Category, + OTHER_CATEGORY_ID, + addCategory, + deleteCategory, + materialiseCategories, + moveCategory, + renameCategory, + setCategoryEmoji, +} from "./lib/itemCategories"; + +const editorArgs = { + listId: v.id("lists"), + userDid: v.string(), +}; + +/** + * Loads the list, checks edit rights, and returns its materialised set. Anyone + * who can edit the list can edit its categories — they can already add and + * reclassify items, so restricting this to the owner would be inconsistent. + */ +async function loadEditableSet( + ctx: MutationCtx, + listId: Id<"lists">, + userDid: string +): Promise { + const list = await ctx.db.get(listId); + if (!list) throw new Error("List not found"); + if (!(await canUserEditList(ctx, listId, userDid))) { + throw new Error("You do not have permission to edit this list"); + } + return materialiseCategories(list.itemCategories, list.customAisles); +} + +async function persist( + ctx: MutationCtx, + listId: Id<"lists">, + categories: Category[] +): Promise { + await ctx.db.patch(listId, { itemCategories: categories }); +} + +export const addListCategory = mutation({ + args: { ...editorArgs, name: v.string(), emoji: v.string() }, + handler: async (ctx, args) => { + const set = await loadEditableSet(ctx, args.listId, args.userDid); + await persist(ctx, args.listId, addCategory(set, args.name, args.emoji)); + }, +}); + +export const renameListCategory = mutation({ + args: { ...editorArgs, categoryId: v.string(), name: v.string() }, + handler: async (ctx, args) => { + const set = await loadEditableSet(ctx, args.listId, args.userDid); + await persist(ctx, args.listId, renameCategory(set, args.categoryId, args.name)); + }, +}); + +export const setListCategoryEmoji = mutation({ + args: { ...editorArgs, categoryId: v.string(), emoji: v.string() }, + handler: async (ctx, args) => { + const set = await loadEditableSet(ctx, args.listId, args.userDid); + await persist(ctx, args.listId, setCategoryEmoji(set, args.categoryId, args.emoji)); + }, +}); + +export const moveListCategory = mutation({ + args: { + ...editorArgs, + categoryId: v.string(), + direction: v.union(v.literal("up"), v.literal("down")), + }, + handler: async (ctx, args) => { + const set = await loadEditableSet(ctx, args.listId, args.userDid); + await persist(ctx, args.listId, moveCategory(set, args.categoryId, args.direction)); + }, +}); + +export const deleteListCategory = mutation({ + args: { ...editorArgs, categoryId: v.string() }, + handler: async (ctx, args) => { + const set = await loadEditableSet(ctx, args.listId, args.userDid); + const next = deleteCategory(set, args.categoryId); + + // Items explicitly filed here would otherwise point at a category that no + // longer exists. The client degrades those to Other on read, but leaving the + // stale id behind would resurrect the group if the name were ever reused. + const items = await ctx.db + .query("items") + .withIndex("by_list", (q) => q.eq("listId", args.listId)) + .collect(); + let reassigned = 0; + for (const item of items) { + if (item.groceryAisle === args.categoryId) { + await ctx.db.patch(item._id, { groceryAisle: OTHER_CATEGORY_ID }); + reassigned += 1; + } + } + + await persist(ctx, args.listId, next); + return { reassigned }; + }, +}); diff --git a/convex/lib/itemCategories.ts b/convex/lib/itemCategories.ts new file mode 100644 index 0000000..b7a8cc3 --- /dev/null +++ b/convex/lib/itemCategories.ts @@ -0,0 +1,181 @@ +/** + * Per-list item categories. + * + * A list either uses the built-in grocery set (the default, and what every list + * started as) or owns an explicit set of its own. The moment someone edits + * categories on a list, the set is MATERIALISED onto that list — from then on it + * is the complete truth for that list and grocery aisles are no longer offered. + * That is what lets a packing list stop being asked about Produce and Deli. + * + * Keyword auto-classification is kept, but gated on the guess being reachable: + * a guessed category is used only if the list's set still contains it. So a + * grocery list keeps auto-filing even after adding a category of its own, while + * a packing list that has removed the grocery aisles stops guessing rather than + * filing "Sunglasses" into Produce. + * + * Pure module — no Convex, no DOM, no keyword map — so both the client and the + * mutations share one implementation of the rules. + */ + + + +export interface Category { + id: string; + name: string; + emoji: string; + order: number; +} + +/** Every set ends with this bucket. It cannot be renamed away or deleted. */ +export const OTHER_CATEGORY_ID = "other"; + +export const MAX_CATEGORY_NAME_LENGTH = 40; + +/** The built-in grocery set, used until a list materialises its own. */ +export const DEFAULT_CATEGORIES: Category[] = [ + { id: "produce", name: "Produce", emoji: "🥬", order: 0 }, + { id: "bakery", name: "Bakery", emoji: "🍞", order: 1 }, + { id: "deli", name: "Deli", emoji: "🥪", order: 2 }, + { id: "meat", name: "Meat & Seafood", emoji: "🥩", order: 3 }, + { id: "dairy", name: "Dairy & Eggs", emoji: "🥛", order: 4 }, + { id: "frozen", name: "Frozen", emoji: "🧊", order: 5 }, + { id: "beverages", name: "Beverages", emoji: "🥤", order: 6 }, + { id: "snacks", name: "Snacks", emoji: "🍿", order: 7 }, + { id: "canned", name: "Canned & Jarred", emoji: "🥫", order: 8 }, + { id: "pasta", name: "Pasta, Rice & Grains", emoji: "🍝", order: 9 }, + { id: "condiments", name: "Condiments & Sauces", emoji: "🫙", order: 10 }, + { id: "baking", name: "Baking", emoji: "🧁", order: 11 }, + { id: "breakfast", name: "Breakfast & Cereal", emoji: "🥣", order: 12 }, + { id: "household", name: "Household", emoji: "🧹", order: 13 }, + { id: "health", name: "Health & Personal Care", emoji: "🧴", order: 14 }, + { id: "other", name: "Other", emoji: "🛒", order: 99 }, +]; + +/** True once a list owns its categories and should not be offered grocery aisles. */ +export function hasOwnCategories(categories: Category[] | undefined | null): boolean { + return Array.isArray(categories) && categories.length > 0; +} + +/** The set in effect for a list, always in display order. */ +export function resolveCategories(categories: Category[] | undefined | null): Category[] { + const set = hasOwnCategories(categories) ? categories! : DEFAULT_CATEGORIES; + return [...set].sort((a, b) => a.order - b.order); +} + +/** + * The set a list should be given the first time its categories are edited. + * Folds any legacy per-list aisles in after the defaults, preserving what the + * list already displayed. + */ +export function materialiseCategories( + categories: Category[] | undefined | null, + legacyCustomAisles?: Category[] | null +): Category[] { + if (hasOwnCategories(categories)) return resolveCategories(categories); + + const merged = [...DEFAULT_CATEGORIES]; + for (const aisle of legacyCustomAisles ?? []) { + if (!merged.some((c) => c.id === aisle.id)) merged.push(aisle); + } + return normaliseOrder(withOtherLast(merged)); +} + +/** Renumbers `order` to 0..n-1 so it never drifts or collides. */ +export function normaliseOrder(categories: Category[]): Category[] { + return categories.map((category, index) => ({ ...category, order: index })); +} + +/** Other always sorts last, wherever it ended up. */ +function withOtherLast(categories: Category[]): Category[] { + const rest = categories.filter((c) => c.id !== OTHER_CATEGORY_ID); + const other = categories.find((c) => c.id === OTHER_CATEGORY_ID); + return other ? [...rest, other] : [...rest, { id: OTHER_CATEGORY_ID, name: "Other", emoji: "🛒", order: rest.length }]; +} + +export class CategoryError extends Error {} + +function assertEditable(id: string, action: string): void { + if (id === OTHER_CATEGORY_ID) { + throw new CategoryError(`The Other category cannot be ${action} — items need somewhere to land.`); + } +} + +function cleanName(name: string): string { + const trimmed = name.trim(); + if (trimmed.length === 0) throw new CategoryError("Category name cannot be empty"); + if (trimmed.length > MAX_CATEGORY_NAME_LENGTH) { + throw new CategoryError(`Category name cannot exceed ${MAX_CATEGORY_NAME_LENGTH} characters`); + } + return trimmed; +} + +/** Slug derived from the name, kept unique within the set. */ +export function categoryIdFor(name: string, existing: Category[]): string { + const base = + name + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 32) || "category"; + + let candidate = base; + let n = 2; + while (existing.some((c) => c.id === candidate)) candidate = `${base}-${n++}`; + return candidate; +} + +export function addCategory(categories: Category[], name: string, emoji: string): Category[] { + const clean = cleanName(name); + if (categories.some((c) => c.name.toLowerCase() === clean.toLowerCase())) { + throw new CategoryError(`A category named "${clean}" already exists`); + } + const added: Category = { + id: categoryIdFor(clean, categories), + name: clean, + emoji: emoji.trim() || "🏷️", + order: categories.length, + }; + return normaliseOrder(withOtherLast([...categories, added])); +} + +export function renameCategory(categories: Category[], id: string, name: string): Category[] { + assertEditable(id, "renamed"); + const clean = cleanName(name); + if (!categories.some((c) => c.id === id)) throw new CategoryError(`No category ${id}`); + if (categories.some((c) => c.id !== id && c.name.toLowerCase() === clean.toLowerCase())) { + throw new CategoryError(`A category named "${clean}" already exists`); + } + return categories.map((c) => (c.id === id ? { ...c, name: clean } : c)); +} + +export function setCategoryEmoji(categories: Category[], id: string, emoji: string): Category[] { + if (!categories.some((c) => c.id === id)) throw new CategoryError(`No category ${id}`); + const clean = emoji.trim() || "🏷️"; + return categories.map((c) => (c.id === id ? { ...c, emoji: clean } : c)); +} + +/** + * Removes a category. Callers must reassign its items to OTHER_CATEGORY_ID — + * this module only owns the category list. + */ +export function deleteCategory(categories: Category[], id: string): Category[] { + assertEditable(id, "deleted"); + if (!categories.some((c) => c.id === id)) throw new CategoryError(`No category ${id}`); + return normaliseOrder(withOtherLast(categories.filter((c) => c.id !== id))); +} + +/** Moves a category one slot up or down. Other stays pinned last. */ +export function moveCategory(categories: Category[], id: string, direction: "up" | "down"): Category[] { + assertEditable(id, "reordered"); + const ordered = resolveCategories(categories); + const movable = ordered.filter((c) => c.id !== OTHER_CATEGORY_ID); + const index = movable.findIndex((c) => c.id === id); + if (index === -1) throw new CategoryError(`No category ${id}`); + + const target = direction === "up" ? index - 1 : index + 1; + if (target < 0 || target >= movable.length) return ordered; // already at the edge + + const next = [...movable]; + [next[index], next[target]] = [next[target], next[index]]; + return normaliseOrder(withOtherLast(next)); +} diff --git a/convex/schema.ts b/convex/schema.ts index d785836..6eca4d3 100644 --- a/convex/schema.ts +++ b/convex/schema.ts @@ -121,13 +121,25 @@ export default defineSchema({ }), proof: v.optional(v.string()), // JWT or linked data proof })), - // Custom grocery aisles created by users for this list + // Custom grocery aisles created by users for this list. + // Superseded by itemCategories; read only when materialising a list's set. customAisles: v.optional(v.array(v.object({ id: v.string(), name: v.string(), emoji: v.string(), order: v.number(), }))), + // This list's own item categories. Absent means it still uses the built-in + // grocery set; the first category edit materialises the full set here, after + // which it is the complete truth for the list. Named itemCategories, not + // categories, because `categoryId` above is the user's folder for the LIST — + // a different concept on the same row. + itemCategories: v.optional(v.array(v.object({ + id: v.string(), + name: v.string(), + emoji: v.string(), + order: v.number(), + }))), // Item view mode preference: "alphabetical" (flat A-Z) or "categorized" (grouped by category) itemViewMode: v.optional(v.union(v.literal("alphabetical"), v.literal("categorized"))), }) diff --git a/scripts/categories.test.mjs b/scripts/categories.test.mjs new file mode 100644 index 0000000..bc6b95a --- /dev/null +++ b/scripts/categories.test.mjs @@ -0,0 +1,239 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdir, rm } from "node:fs/promises"; +import { pathToFileURL } from "node:url"; +import { build } from "esbuild"; + +const outdir = "tmp/categories-test"; + +async function loadModule() { + await rm(outdir, { recursive: true, force: true }); + await mkdir(outdir, { recursive: true }); + await build({ + entryPoints: ["src/lib/categories.ts"], + outfile: `${outdir}/categories.mjs`, + bundle: true, + platform: "node", + format: "esm", + target: "node20", + }); + return import(pathToFileURL(`${process.cwd()}/${outdir}/categories.mjs`).href); +} + +const c = await loadModule(); + +/** A materialised packing-list set, as a list gets after its first edit. */ +function packingSet() { + return c.normaliseOrder([ + { id: "luggage", name: "Luggage", emoji: "🧳", order: 0 }, + { id: "clothes", name: "Clothes", emoji: "👕", order: 1 }, + { id: "electronics", name: "Electronics", emoji: "🔌", order: 2 }, + { id: "other", name: "Other", emoji: "🛒", order: 3 }, + ]); +} + +test("a list with no categories falls back to the grocery defaults", () => { + assert.equal(c.hasOwnCategories(undefined), false); + assert.equal(c.hasOwnCategories([]), false, "an empty array is not an owned set"); + + const resolved = c.resolveCategories(undefined); + assert.ok(resolved.length > 1); + assert.equal(resolved[0].id, "produce", "defaults keep store-walk order"); + assert.ok(resolved.some((x) => x.id === "other")); +}); + +test("materialising folds legacy custom aisles in after the defaults", () => { + const materialised = c.materialiseCategories(undefined, [ + { id: "pet", name: "Pet Supplies", emoji: "🐕", order: 99 }, + ]); + + assert.ok(materialised.some((x) => x.id === "produce"), "defaults are preserved"); + assert.ok(materialised.some((x) => x.id === "pet"), "existing custom aisles survive"); + assert.equal( + materialised[materialised.length - 1].id, + "other", + "Other is pinned last so items always have a home" + ); + assert.deepEqual( + materialised.map((x) => x.order), + materialised.map((_, i) => i), + "order is renumbered contiguously" + ); +}); + +test("materialising an already-owned set is a no-op", () => { + const set = packingSet(); + assert.deepEqual(c.materialiseCategories(set, []), set); +}); + +test("an owned set no longer offers grocery aisles", () => { + const resolved = c.resolveCategories(packingSet()); + assert.deepEqual(resolved.map((x) => x.id), ["luggage", "clothes", "electronics", "other"]); + assert.equal(resolved.some((x) => x.id === "produce"), false, "this is the whole point"); +}); + +test("addCategory appends, slugs the id, and keeps Other last", () => { + const next = c.addCategory(packingSet(), "Toiletries", "🧴"); + const added = next.find((x) => x.name === "Toiletries"); + + assert.equal(added.id, "toiletries"); + assert.equal(added.emoji, "🧴"); + assert.equal(next[next.length - 1].id, "other"); + assert.equal(next.find((x) => x.id === "other").order, next.length - 1); +}); + +test("addCategory rejects blank, overlong, and duplicate names", () => { + const set = packingSet(); + assert.throws(() => c.addCategory(set, " ", "🧴"), /cannot be empty/); + assert.throws(() => c.addCategory(set, "x".repeat(41), "🧴"), /exceed/); + assert.throws(() => c.addCategory(set, "clothes", "👕"), /already exists/, "match is case-insensitive"); +}); + +test("addCategory disambiguates ids when names slug identically", () => { + const once = c.addCategory(packingSet(), "Carry On", "🎒"); + const twice = c.addCategory(once, "Carry-On", "🎒"); + const ids = twice.filter((x) => x.id.startsWith("carry-on")).map((x) => x.id); + assert.deepEqual(ids, ["carry-on", "carry-on-2"], "ids must stay unique within the set"); +}); + +test("addCategory falls back to a default emoji", () => { + const next = c.addCategory(packingSet(), "Snacks", " "); + assert.equal(next.find((x) => x.name === "Snacks").emoji, "🏷️"); +}); + +test("renameCategory renames in place without reordering", () => { + const next = c.renameCategory(packingSet(), "clothes", "Outfits"); + assert.equal(next.find((x) => x.id === "clothes").name, "Outfits"); + assert.deepEqual(next.map((x) => x.id), ["luggage", "clothes", "electronics", "other"]); +}); + +test("renameCategory rejects a name another category already uses", () => { + assert.throws(() => c.renameCategory(packingSet(), "clothes", "Luggage"), /already exists/); + // Renaming to its own name differing only in case is fine. + assert.doesNotThrow(() => c.renameCategory(packingSet(), "clothes", "CLOTHES")); +}); + +test("deleteCategory removes it and renumbers", () => { + const next = c.deleteCategory(packingSet(), "clothes"); + assert.deepEqual(next.map((x) => x.id), ["luggage", "electronics", "other"]); + assert.deepEqual(next.map((x) => x.order), [0, 1, 2]); +}); + +test("the Other bucket cannot be renamed, deleted, or reordered", () => { + const set = packingSet(); + assert.throws(() => c.deleteCategory(set, "other"), /cannot be deleted/); + assert.throws(() => c.renameCategory(set, "other", "Misc"), /cannot be renamed/); + assert.throws(() => c.moveCategory(set, "other", "up"), /cannot be reordered/); +}); + +test("moveCategory swaps neighbours and is a no-op at the edges", () => { + const set = packingSet(); + + const down = c.moveCategory(set, "luggage", "down"); + assert.deepEqual(down.map((x) => x.id), ["clothes", "luggage", "electronics", "other"]); + + const up = c.moveCategory(down, "luggage", "up"); + assert.deepEqual(up.map((x) => x.id), ["luggage", "clothes", "electronics", "other"]); + + assert.deepEqual( + c.moveCategory(set, "luggage", "up").map((x) => x.id), + set.map((x) => x.id), + "already first" + ); + assert.deepEqual( + c.moveCategory(set, "electronics", "down").map((x) => x.id), + set.map((x) => x.id), + "last movable category cannot displace Other" + ); +}); + +test("moveCategory never lets a category slip past Other", () => { + const next = c.moveCategory(packingSet(), "electronics", "down"); + assert.equal(next[next.length - 1].id, "other"); +}); + +test("setCategoryEmoji changes only the emoji, including on Other", () => { + const next = c.setCategoryEmoji(packingSet(), "other", "📦"); + assert.equal(next.find((x) => x.id === "other").emoji, "📦"); + assert.equal(next.find((x) => x.id === "other").name, "Other", "name is untouched"); +}); + +test("editing an unknown category is an error, not a silent no-op", () => { + const set = packingSet(); + assert.throws(() => c.renameCategory(set, "nope", "X"), /No category/); + assert.throws(() => c.deleteCategory(set, "nope"), /No category/); + assert.throws(() => c.moveCategory(set, "nope", "up"), /No category/); + assert.throws(() => c.setCategoryEmoji(set, "nope", "X"), /No category/); +}); + +// --- classification against the list's own set --- + +test("a grocery list still auto-classifies", () => { + const set = c.resolveCategories(undefined); + assert.equal(c.resolveItemCategory({ name: "bananas" }, set), "produce"); + assert.equal(c.resolveItemCategory({ name: "milk" }, set), "dairy"); +}); + +test("a grocery list that added a category KEEPS auto-classifying", () => { + // The regression this guards: materialising on first edit must not silently + // switch grocery auto-filing off. + const set = c.addCategory(c.materialiseCategories(undefined, []), "Pet Supplies", "🐕"); + assert.equal(c.resolveItemCategory({ name: "bananas" }, set), "produce"); + assert.equal(c.resolveItemCategory({ name: "chicken breast" }, set), "meat"); +}); + +test("a packing list that dropped the grocery aisles stops guessing", () => { + const set = packingSet(); + // Would classify as "produce" against the grocery set; that category is gone. + assert.equal(c.resolveItemCategory({ name: "bananas" }, set), "other"); + assert.equal(c.resolveItemCategory({ name: "Sunglasses" }, set), "other"); +}); + +test("an explicit assignment beats the keyword guess", () => { + const set = c.resolveCategories(undefined); + assert.equal(c.resolveItemCategory({ name: "bananas", groceryAisle: "snacks" }, set), "snacks"); +}); + +test("an assignment to a deleted category degrades to Other", () => { + const set = c.deleteCategory(packingSet(), "clothes"); + assert.equal( + c.resolveItemCategory({ name: "T-shirt", groceryAisle: "clothes" }, set), + "other", + "the item must stay visible, not vanish into a group nothing renders" + ); +}); + +test("groupByCategory returns non-empty groups in display order", () => { + const groups = c.groupByCategory( + [ + { name: "Passport", groceryAisle: "luggage" }, + { name: "Laptop", groceryAisle: "electronics" }, + { name: "Socks", groceryAisle: "clothes" }, + { name: "Charger", groceryAisle: "electronics" }, + { name: "Mystery thing" }, + ], + packingSet() + ); + + assert.deepEqual(groups.map((g) => g.category.id), ["luggage", "clothes", "electronics", "other"]); + assert.deepEqual(groups.find((g) => g.category.id === "electronics").items.map((i) => i.name), [ + "Laptop", + "Charger", + ]); + assert.deepEqual(groups.find((g) => g.category.id === "other").items.map((i) => i.name), [ + "Mystery thing", + ]); +}); + +test("groupByCategory omits empty categories, including Other", () => { + const groups = c.groupByCategory([{ name: "Passport", groceryAisle: "luggage" }], packingSet()); + assert.deepEqual(groups.map((g) => g.category.id), ["luggage"]); +}); + +test("groupByCategory keeps every item exactly once", () => { + const items = Array.from({ length: 25 }, (_, i) => ({ name: `item-${i}` })); + const groups = c.groupByCategory(items, c.resolveCategories(undefined)); + const seen = groups.flatMap((g) => g.items.map((i) => i.name)); + assert.equal(seen.length, items.length); + assert.equal(new Set(seen).size, items.length); +}); diff --git a/scripts/item-categories-mutations.test.mjs b/scripts/item-categories-mutations.test.mjs new file mode 100644 index 0000000..fbc8942 --- /dev/null +++ b/scripts/item-categories-mutations.test.mjs @@ -0,0 +1,177 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdir, rm } from "node:fs/promises"; +import { pathToFileURL } from "node:url"; +import { build } from "esbuild"; + +const outdir = "tmp/item-categories-mutations-test"; + +async function loadModule() { + await rm(outdir, { recursive: true, force: true }); + await mkdir(outdir, { recursive: true }); + await build({ + entryPoints: ["./convex/itemCategories.ts"], + outfile: `${outdir}/itemCategories.mjs`, + bundle: true, + platform: "node", + format: "esm", + target: "node20", + external: ["convex/*"], + }); + return import( + `${pathToFileURL(`${process.cwd()}/${outdir}/itemCategories.mjs`).href}?t=${Date.now()}` + ); +} + +const mod = await loadModule(); +const unwrap = (fn) => fn._handler ?? fn.handler; + +const OWNER = "did:webvh:QmS:boop.ad:user-owner"; +const STRANGER = "did:webvh:QmS:boop.ad:user-stranger"; + +function makeCtx({ list, items = [] } = {}) { + const lists = [{ _id: "L1", ownerDid: OWNER, name: "Rachel's 40th", ...list }]; + const rows = { lists, items: items.map((i) => ({ listId: "L1", ...i })) }; + const byId = new Map(); + rows.lists.forEach((l) => byId.set(l._id, l)); + rows.items.forEach((i) => byId.set(i._id, i)); + + return { + rows, + db: { + get: async (id) => byId.get(id) ?? null, + patch: async (id, fields) => Object.assign(byId.get(id), fields), + query: (table) => { + const result = { + // Index filtering is irrelevant here: each fixture holds one list. + withIndex: () => result, + collect: async () => rows[table] ?? [], + first: async () => (rows[table] ?? [])[0] ?? null, + }; + return result; + }, + }, + }; +} + +const call = (fn, ctx, args) => unwrap(mod[fn])(ctx, { listId: "L1", userDid: OWNER, ...args }); + +test("the first edit materialises the grocery set onto the list", async () => { + const ctx = makeCtx(); + assert.equal(ctx.rows.lists[0].itemCategories, undefined); + + await call("addListCategory", ctx, { name: "Luggage", emoji: "🧳" }); + + const set = ctx.rows.lists[0].itemCategories; + assert.ok(set.some((c) => c.id === "produce"), "grocery aisles are folded in, not lost"); + assert.ok(set.some((c) => c.name === "Luggage")); + assert.equal(set[set.length - 1].id, "other"); +}); + +test("existing customAisles survive materialisation", async () => { + const ctx = makeCtx({ + list: { customAisles: [{ id: "pet", name: "Pet Supplies", emoji: "🐕", order: 99 }] }, + }); + await call("addListCategory", ctx, { name: "Luggage", emoji: "🧳" }); + + const set = ctx.rows.lists[0].itemCategories; + assert.ok(set.some((c) => c.id === "pet"), "a list's existing aisles must not disappear"); +}); + +test("rename, emoji and move persist to the list", async () => { + const ctx = makeCtx({ + list: { + itemCategories: [ + { id: "luggage", name: "Luggage", emoji: "🧳", order: 0 }, + { id: "clothes", name: "Clothes", emoji: "👕", order: 1 }, + { id: "other", name: "Other", emoji: "🛒", order: 2 }, + ], + }, + }); + + await call("renameListCategory", ctx, { categoryId: "clothes", name: "Outfits" }); + assert.equal(ctx.rows.lists[0].itemCategories.find((c) => c.id === "clothes").name, "Outfits"); + + await call("setListCategoryEmoji", ctx, { categoryId: "clothes", emoji: "🧥" }); + assert.equal(ctx.rows.lists[0].itemCategories.find((c) => c.id === "clothes").emoji, "🧥"); + + await call("moveListCategory", ctx, { categoryId: "clothes", direction: "up" }); + assert.deepEqual( + ctx.rows.lists[0].itemCategories.map((c) => c.id), + ["clothes", "luggage", "other"] + ); +}); + +test("deleting a category reassigns only its own items to Other", async () => { + const ctx = makeCtx({ + list: { + itemCategories: [ + { id: "luggage", name: "Luggage", emoji: "🧳", order: 0 }, + { id: "clothes", name: "Clothes", emoji: "👕", order: 1 }, + { id: "other", name: "Other", emoji: "🛒", order: 2 }, + ], + }, + items: [ + { _id: "i1", name: "T-shirt", groceryAisle: "clothes" }, + { _id: "i2", name: "Socks", groceryAisle: "clothes" }, + { _id: "i3", name: "Passport", groceryAisle: "luggage" }, + { _id: "i4", name: "Unfiled" }, + ], + }); + + const result = await call("deleteListCategory", ctx, { categoryId: "clothes" }); + + assert.equal(result.reassigned, 2); + assert.equal(ctx.rows.items.find((i) => i._id === "i1").groceryAisle, "other"); + assert.equal(ctx.rows.items.find((i) => i._id === "i2").groceryAisle, "other"); + assert.equal( + ctx.rows.items.find((i) => i._id === "i3").groceryAisle, + "luggage", + "another category's items must be untouched" + ); + assert.equal(ctx.rows.items.find((i) => i._id === "i4").groceryAisle, undefined); + assert.equal(ctx.rows.lists[0].itemCategories.some((c) => c.id === "clothes"), false); +}); + +test("the Other bucket is protected at the mutation layer too", async () => { + const ctx = makeCtx(); + await assert.rejects(() => call("deleteListCategory", ctx, { categoryId: "other" }), /cannot be deleted/); + await assert.rejects( + () => call("renameListCategory", ctx, { categoryId: "other", name: "Misc" }), + /cannot be renamed/ + ); +}); + +test("a non-editor cannot change categories", async () => { + const ctx = makeCtx(); + await assert.rejects( + () => unwrap(mod.addListCategory)(ctx, { listId: "L1", userDid: STRANGER, name: "X", emoji: "🏷️" }), + /permission/ + ); + assert.equal(ctx.rows.lists[0].itemCategories, undefined, "nothing persisted on refusal"); +}); + +test("a rejected edit leaves the stored set untouched", async () => { + const ctx = makeCtx({ + list: { + itemCategories: [ + { id: "luggage", name: "Luggage", emoji: "🧳", order: 0 }, + { id: "other", name: "Other", emoji: "🛒", order: 1 }, + ], + }, + }); + await assert.rejects(() => call("addListCategory", ctx, { name: " ", emoji: "🏷️" }), /empty/); + assert.deepEqual( + ctx.rows.lists[0].itemCategories.map((c) => c.id), + ["luggage", "other"], + "a failed validation must not partially write" + ); +}); + +test("a missing list is an error, not a silent no-op", async () => { + const ctx = makeCtx(); + await assert.rejects( + () => unwrap(mod.addListCategory)(ctx, { listId: "nope", userDid: OWNER, name: "X", emoji: "🏷️" }), + /List not found/ + ); +}); diff --git a/src/components/CategoryHeaderMenu.tsx b/src/components/CategoryHeaderMenu.tsx new file mode 100644 index 0000000..616ed28 --- /dev/null +++ b/src/components/CategoryHeaderMenu.tsx @@ -0,0 +1,192 @@ +/** + * The ⋯ menu on a category section header: rename, change emoji, reorder, delete. + * + * Rename and emoji edit inline in the header itself rather than opening a dialog, + * so a one-word fix costs one tap and no context switch. + */ + +import { useEffect, useRef, useState } from "react"; +import type { Category } from "../lib/categories"; +import { OTHER_CATEGORY_ID } from "../lib/categories"; + +interface CategoryHeaderMenuProps { + category: Category; + itemCount: number; + isFirst: boolean; + isLast: boolean; + onRename: (name: string) => void; + onSetEmoji: (emoji: string) => void; + onMove: (direction: "up" | "down") => void; + onDelete: () => void; + haptic: (style?: "light" | "medium" | "heavy") => void; +} + +export function CategoryHeaderMenu({ + category, + itemCount, + isFirst, + isLast, + onRename, + onSetEmoji, + onMove, + onDelete, + haptic, +}: CategoryHeaderMenuProps) { + const [open, setOpen] = useState(false); + const [editing, setEditing] = useState<"name" | "emoji" | null>(null); + const [draft, setDraft] = useState(""); + const containerRef = useRef(null); + + // Other exists so items always have a home; it is not user-editable except + // for its emoji, so it gets no menu at all. + const isProtected = category.id === OTHER_CATEGORY_ID; + + useEffect(() => { + if (!open) return; + const onPointerDown = (event: PointerEvent) => { + if (!containerRef.current?.contains(event.target as Node)) setOpen(false); + }; + const onKey = (event: KeyboardEvent) => { + if (event.key === "Escape") setOpen(false); + }; + document.addEventListener("pointerdown", onPointerDown); + document.addEventListener("keydown", onKey); + return () => { + document.removeEventListener("pointerdown", onPointerDown); + document.removeEventListener("keydown", onKey); + }; + }, [open]); + + const beginEdit = (mode: "name" | "emoji") => { + setDraft(mode === "name" ? category.name : category.emoji); + setEditing(mode); + setOpen(false); + }; + + const commit = () => { + const value = draft.trim(); + if (value) { + if (editing === "name" && value !== category.name) onRename(value); + if (editing === "emoji" && value !== category.emoji) onSetEmoji(value); + } + setEditing(null); + }; + + if (editing) { + return ( + setDraft(e.target.value)} + onBlur={commit} + onKeyDown={(e) => { + if (e.key === "Enter") commit(); + if (e.key === "Escape") setEditing(null); + }} + aria-label={editing === "name" ? "Category name" : "Category emoji"} + className={`${ + editing === "emoji" ? "w-12 text-center text-lg" : "flex-1 text-sm font-semibold" + } bg-white dark:bg-gray-800 border border-amber-400 dark:border-amber-600 rounded-lg px-2 py-1 text-gray-800 dark:text-gray-200`} + /> + ); + } + + return ( +
+ + {itemCount} {itemCount === 1 ? "item" : "items"} + + + + + {open && ( +
+ beginEdit("emoji")} /> + {!isProtected && beginEdit("name")} />} + {!isProtected && !isFirst && ( + { + haptic("light"); + onMove("up"); + setOpen(false); + }} + /> + )} + {!isProtected && !isLast && ( + { + haptic("light"); + onMove("down"); + setOpen(false); + }} + /> + )} + {!isProtected && ( + { + haptic("medium"); + onDelete(); + setOpen(false); + }} + /> + )} +
+ )} +
+ ); +} + +function MenuItem({ + icon, + label, + onClick, + destructive, +}: { + icon: string; + label: string; + onClick: () => void; + destructive?: boolean; +}) { + return ( + + ); +} diff --git a/src/lib/categories.ts b/src/lib/categories.ts new file mode 100644 index 0000000..47230f8 --- /dev/null +++ b/src/lib/categories.ts @@ -0,0 +1,65 @@ +/** + * Client-side category helpers: the shared rules plus keyword classification. + * + * The rules live in convex/lib/itemCategories so the mutations enforce exactly + * what the UI shows. Only classification is client-only — the grocery keyword + * map is large and nothing on the server needs it. + */ + +import { classifyItem } from "./groceryAisles"; +import { + type Category, + OTHER_CATEGORY_ID, + resolveCategories, +} from "../../convex/lib/itemCategories"; + +export * from "../../convex/lib/itemCategories"; + +export interface CategorisableItem { + name: string; + /** Explicit assignment (drag / item editor). Holds a category id. */ + groceryAisle?: string; +} + +/** + * Which category an item belongs to, given the list's set. + * + * Precedence: an explicit assignment wins, then the keyword guess, then Other. + * Both are checked against the set, so a category that has since been deleted + * degrades to Other instead of vanishing the item into a group nothing renders. + */ +export function resolveItemCategory(item: CategorisableItem, categories: Category[]): string { + const has = (id: string) => categories.some((c) => c.id === id); + + if (item.groceryAisle && has(item.groceryAisle)) return item.groceryAisle; + + const guess = classifyItem(item.name); + if (has(guess)) return guess; + + return OTHER_CATEGORY_ID; +} + +/** + * Groups items into the list's categories, in display order. Empty categories + * are dropped so a list is not padded with headers it never uses — except Other, + * which is also dropped when empty. + */ +export function groupByCategory( + items: T[], + categories: Category[] | undefined | null +): { category: Category; items: T[] }[] { + const resolved = resolveCategories(categories); + const buckets = new Map(); + + for (const item of items) { + const id = resolveItemCategory(item, resolved); + const bucket = buckets.get(id); + if (bucket) bucket.push(item); + else buckets.set(id, [item]); + } + + return resolved + .filter((category) => (buckets.get(category.id)?.length ?? 0) > 0) + .map((category) => ({ category, items: buckets.get(category.id)! })); +} + diff --git a/src/pages/ListView.tsx b/src/pages/ListView.tsx index 2d2c4c8..a75bf38 100644 --- a/src/pages/ListView.tsx +++ b/src/pages/ListView.tsx @@ -18,7 +18,9 @@ import { useSettings } from "../hooks/useSettings"; import { useTouchDrag } from "../hooks/useTouchDrag"; import { useNotifications } from "../hooks/useNotifications"; import { useKeyboardShortcuts, KeyboardShortcutsHelp, type Shortcut } from "../hooks/useKeyboardShortcuts"; -import { groupByAisle, classifyItem } from "../lib/groceryAisles"; +import { classifyItem } from "../lib/groceryAisles"; +import { groupByCategory, resolveCategories, type Category } from "../lib/categories"; +import { CategoryHeaderMenu } from "../components/CategoryHeaderMenu"; import { useCategories } from "../hooks/useCategories"; import { shareList } from "../lib/share"; import { recordLatencyMs, setGaugeMetric } from "../lib/observability"; @@ -142,9 +144,11 @@ export function ListView() { const updateItemMutation = useMutation(api.items.updateItem); // Custom aisle state - const addCustomAisleMutation = useMutation(api.lists.addCustomAisle); - const _removeCustomAisle = useMutation(api.lists.removeCustomAisle); - void _removeCustomAisle; // available for future delete-aisle UI + const addCategoryMutation = useMutation(api.itemCategories.addListCategory); + const renameCategoryMutation = useMutation(api.itemCategories.renameListCategory); + const setCategoryEmojiMutation = useMutation(api.itemCategories.setListCategoryEmoji); + const moveCategoryMutation = useMutation(api.itemCategories.moveListCategory); + const deleteCategoryMutation = useMutation(api.itemCategories.deleteListCategory); const [showAddAisle, setShowAddAisle] = useState(false); const [newAisleName, setNewAisleName] = useState(""); const [newAisleEmoji, setNewAisleEmoji] = useState("🏷️"); @@ -227,8 +231,15 @@ export function ListView() { if (itemViewMode !== "categorized") return null; const unchecked = sortedItems.filter(item => !item.checked && !item.parentId); const checked = sortedItems.filter(item => item.checked && !item.parentId); - const customAisles = (list as any)?.customAisles as { id: string; name: string; emoji: string; order: number }[] | undefined; - return { groups: groupByAisle(unchecked.map(item => ({ ...item, name: item.name ?? "" })), customAisles ?? undefined), checked, customAisles: customAisles ?? [] }; + const listRow = list as unknown as { itemCategories?: Category[]; customAisles?: Category[] } | undefined; + // A list that has not been edited yet has no itemCategories and falls back + // to the grocery defaults; customAisles are only folded in on first edit. + const categories = resolveCategories(listRow?.itemCategories); + return { + groups: groupByCategory(unchecked.map(item => ({ ...item, name: item.name ?? "" })), categories), + checked, + categories, + }; }, [itemViewMode, sortedItems, list]); // Look up live items by ID to avoid stale snapshots in modals @@ -391,7 +402,7 @@ export function ListView() { if (!aisleGroups) return null; const overId = groceryTouchDrag.state.dragOverId; if (!overId) return null; - for (const { aisle, items: aisleItems } of aisleGroups.groups) { + for (const { category: aisle, items: aisleItems } of aisleGroups.groups) { if (aisleItems.some(i => i._id === overId)) return aisle.id; } return null; @@ -539,7 +550,7 @@ export function ListView() { const item = sortedItems[focusedIndex]; if (item) { haptic('medium'); - removeItemMutation({ itemId: item._id, userDid: did, legacyDid: legacyDid ?? undefined }); + removeItemMutation({ itemId: item._id, userDid: did }); // Move focus up if at end of list if (focusedIndex >= sortedItems.length - 1) { setFocusedIndex(Math.max(0, sortedItems.length - 2)); @@ -555,7 +566,7 @@ export function ListView() { const item = sortedItems[focusedIndex]; if (item) { haptic('medium'); - removeItemMutation({ itemId: item._id, userDid: did, legacyDid: legacyDid ?? undefined }); + removeItemMutation({ itemId: item._id, userDid: did }); // Move focus up if at end of list if (focusedIndex >= sortedItems.length - 1) { setFocusedIndex(Math.max(0, sortedItems.length - 2)); @@ -571,7 +582,7 @@ export function ListView() { const item = sortedItems[focusedIndex]; if (item) { haptic('medium'); - removeItemMutation({ itemId: item._id, userDid: did, legacyDid: legacyDid ?? undefined }); + removeItemMutation({ itemId: item._id, userDid: did }); // Move focus up if at end of list if (focusedIndex >= sortedItems.length - 1) { setFocusedIndex(Math.max(0, sortedItems.length - 2)); @@ -789,7 +800,7 @@ export function ListView() { setViewMode("list"); if (itemViewMode !== "alphabetical") { setLocalItemViewMode("alphabetical"); - updateItemViewModeMutation({ listId, itemViewMode: "alphabetical", userDid: did, legacyDid: legacyDid ?? undefined }); + updateItemViewModeMutation({ listId, itemViewMode: "alphabetical", userDid: did }); } }} className={`p-1.5 sm:px-2.5 sm:py-1.5 rounded-full transition-all active:scale-95 ${ @@ -810,7 +821,7 @@ export function ListView() { setViewMode("list"); if (itemViewMode !== "categorized") { setLocalItemViewMode("categorized"); - updateItemViewModeMutation({ listId, itemViewMode: "categorized", userDid: did, legacyDid: legacyDid ?? undefined }); + updateItemViewModeMutation({ listId, itemViewMode: "categorized", userDid: did }); } }} className={`p-1.5 sm:px-2.5 sm:py-1.5 rounded-full transition-all active:scale-95 ${ @@ -979,7 +990,7 @@ export function ListView() { ✨ Drag items between aisles to reclassify - {aisleGroups.groups.map(({ aisle, items: aisleItems }) => ( + {aisleGroups.groups.map(({ category: aisle, items: aisleItems }, groupIndex) => (
{/* Aisle section header — highlights when dragging over */}
{aisle.emoji} {aisle.name} - - {aisleItems.length} {aisleItems.length === 1 ? "item" : "items"} - + {canUserEdit ? ( + renameCategoryMutation({ listId, categoryId: aisle.id, name, userDid: did })} + onSetEmoji={(emoji) => setCategoryEmojiMutation({ listId, categoryId: aisle.id, emoji, userDid: did })} + onMove={(direction) => moveCategoryMutation({ listId, categoryId: aisle.id, direction, userDid: did })} + onDelete={() => deleteCategoryMutation({ listId, categoryId: aisle.id, userDid: did })} + /> + ) : ( + + {aisleItems.length} {aisleItems.length === 1 ? "item" : "items"} + + )}
{aisleItems.map((item) => { @@ -1052,7 +1077,7 @@ export function ListView() { autoFocus onKeyDown={e => { if (e.key === "Enter" && newAisleName.trim()) { - addCustomAisleMutation({ listId, name: newAisleName.trim(), emoji: newAisleEmoji || "🏷️", userDid: did, legacyDid: legacyDid ?? undefined }); + addCategoryMutation({ listId, name: newAisleName.trim(), emoji: newAisleEmoji || "🏷️", userDid: did }); setNewAisleName(""); setNewAisleEmoji("🏷️"); setShowAddAisle(false); @@ -1065,7 +1090,7 @@ export function ListView() {