Make shopping items editable: tap to rename, move store/section, delete

Tapping an item outside shopping mode now opens an edit sheet. User-added items
can be renamed, moved between stores and sections, and deleted. PlanToEat items
show why they cannot be edited — the client is read-only, so presenting fields
that silently would not save is worse than saying so — and offer removal, which
marks them checked, the same mechanism shopping mode already uses.

This needed a store registry first. Stores previously existed only because some
item referenced them by name, with Trello's list names quietly acting as the
registry; removing Trello as a source made the gap visible. Without a registry
you cannot move an item into an empty store, and moving the last item out of a
store makes it vanish mid-edit. Migration 034 adds shopping_stores and
shopping_sections, seeded from the stores currently in use, and gives items a
section column — sections previously existed only as PlanToEat category
strings, so a user item could not be filed under one at all.

Registered stores and sections now appear in the list even when empty, which is
the whole point of the registry.

Two deliberate choices about destructive edges:
  * Renaming a store renames it and moves its items in one transaction. Items
    reference the store by name, so doing either alone orphans them.
  * Deleting a store or section keeps the items, moving them to Unassigned or
    clearing the section. Losing the list is far worse than re-filing an item.
  * An UPDATE matching no rows is an error, not a silent success, so a stale id
    cannot look like a saved edit.

Section options are rebuilt per store in the edit sheet rather than shipped as
one flat list, so a Costco item cannot be filed under a Safeway aisle.

18 new store tests, 11 handler tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

1 files changed, 302 insertions, 0 deletions

diff --git a/internal/store/shopping_registry_test.go b/internal/store/shopping_registry_test.go
new file mode 100644
index 0000000..b8ed79e
--- /dev/null
+++ b/internal/store/shopping_registry_test.go
@@ -0,0 +1,302 @@
+package store
+
+import (
+	"database/sql"
+	"errors"
+	"path/filepath"
+	"testing"
+
+	_ "github.com/mattn/go-sqlite3"
+)
+
+func setupRegistryStore(t *testing.T) *Store {
+	t.Helper()
+	db, err := sql.Open("sqlite3", filepath.Join(t.TempDir(), "reg.db"))
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+	db.SetMaxOpenConns(1)
+	if _, err := db.Exec(`
+		PRAGMA foreign_keys = ON;
+		CREATE TABLE user_shopping_items (
+			id INTEGER PRIMARY KEY AUTOINCREMENT,
+			name TEXT NOT NULL,
+			store TEXT NOT NULL,
+			section TEXT NOT NULL DEFAULT '',
+			checked INTEGER DEFAULT 0,
+			created_at DATETIME DEFAULT CURRENT_TIMESTAMP
+		);
+		CREATE TABLE shopping_stores (
+			id INTEGER PRIMARY KEY AUTOINCREMENT,
+			name TEXT NOT NULL UNIQUE,
+			sort_order INTEGER NOT NULL DEFAULT 0,
+			created_at DATETIME DEFAULT CURRENT_TIMESTAMP
+		);
+		CREATE TABLE shopping_sections (
+			id INTEGER PRIMARY KEY AUTOINCREMENT,
+			store_id INTEGER NOT NULL REFERENCES shopping_stores(id) ON DELETE CASCADE,
+			name TEXT NOT NULL,
+			sort_order INTEGER NOT NULL DEFAULT 0,
+			created_at DATETIME DEFAULT CURRENT_TIMESTAMP,
+			UNIQUE (store_id, name)
+		);`); err != nil {
+		t.Fatalf("schema: %v", err)
+	}
+	return &Store{db: db}
+}
+
+// The registry's whole purpose: a store exists whether or not anything is in
+// it, so you can move an item *to* an empty store.
+func TestShoppingStores_ExistWithoutItems(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	if err := s.AddShoppingStore("Hardware Store"); err != nil {
+		t.Fatalf("add: %v", err)
+	}
+	names, err := s.ShoppingStoreNames()
+	if err != nil {
+		t.Fatalf("names: %v", err)
+	}
+	if len(names) != 1 || names[0] != "Hardware Store" {
+		t.Errorf("got %v, want [Hardware Store]", names)
+	}
+}
+
+func TestShoppingStores_AddIsIdempotent(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.AddShoppingStore("Costco")
+	if err := s.AddShoppingStore("Costco"); err != nil {
+		t.Fatalf("re-add should be a no-op, got: %v", err)
+	}
+	names, _ := s.ShoppingStoreNames()
+	if len(names) != 1 {
+		t.Errorf("got %d stores, want 1", len(names))
+	}
+}
+
+func TestShoppingStores_AddRejectsBlank(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+	if err := s.AddShoppingStore("   "); err == nil {
+		t.Error("expected an error for a blank store name")
+	}
+}
+
+// Items reference a store by name, so a rename must carry them along or every
+// item in that store is orphaned.
+func TestShoppingStores_RenameMovesItsItems(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.AddShoppingStore("Hardware")
+	_ = s.SaveUserShoppingItem("Nails", "Hardware")
+	_ = s.SaveUserShoppingItem("Milk", "Costco")
+
+	stores, _ := s.GetShoppingStores()
+	if err := s.RenameShoppingStore(stores[0].ID, "Hardware Store"); err != nil {
+		t.Fatalf("rename: %v", err)
+	}
+
+	items, _ := s.GetUserShoppingItems()
+	for _, it := range items {
+		if it.Name == "Nails" && it.Store != "Hardware Store" {
+			t.Errorf("item stayed on the old store name: %+v", it)
+		}
+		if it.Name == "Milk" && it.Store != "Costco" {
+			t.Errorf("an unrelated item was moved: %+v", it)
+		}
+	}
+}
+
+func TestShoppingStores_RenameUnknownStore(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+	if err := s.RenameShoppingStore(999, "Nope"); !errors.Is(err, ErrStoreNotFound) {
+		t.Errorf("got %v, want ErrStoreNotFound", err)
+	}
+}
+
+// Deleting a store must not take its items with it — losing the shopping list
+// is a much worse outcome than an item needing to be re-filed.
+func TestShoppingStores_DeleteKeepsItems(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.AddShoppingStore("Hardware")
+	_ = s.SaveUserShoppingItemInSection("Nails", "Hardware", "Fasteners")
+
+	stores, _ := s.GetShoppingStores()
+	if err := s.DeleteShoppingStore(stores[0].ID); err != nil {
+		t.Fatalf("delete: %v", err)
+	}
+
+	items, _ := s.GetUserShoppingItems()
+	if len(items) != 1 {
+		t.Fatalf("got %d items, want the item kept", len(items))
+	}
+	if items[0].Store != "Unassigned" || items[0].Section != "" {
+		t.Errorf("got %+v, want it moved to Unassigned with no section", items[0])
+	}
+}
+
+func TestShoppingSections_AddedAndReturnedWithTheirStore(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.AddShoppingStore("Costco")
+	stores, _ := s.GetShoppingStores()
+	if err := s.AddShoppingSection(stores[0].ID, "Produce"); err != nil {
+		t.Fatalf("add section: %v", err)
+	}
+	_ = s.AddShoppingSection(stores[0].ID, "Frozen")
+
+	stores, _ = s.GetShoppingStores()
+	if len(stores[0].Sections) != 2 {
+		t.Fatalf("got %d sections, want 2", len(stores[0].Sections))
+	}
+}
+
+// The same section name in two different stores is normal ("Produce" exists in
+// both) and must not collide.
+func TestShoppingSections_SameNameInDifferentStores(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.AddShoppingStore("Costco")
+	_ = s.AddShoppingStore("Safeway")
+	stores, _ := s.GetShoppingStores()
+	for _, st := range stores {
+		if err := s.AddShoppingSection(st.ID, "Produce"); err != nil {
+			t.Fatalf("add section to %s: %v", st.Name, err)
+		}
+	}
+	stores, _ = s.GetShoppingStores()
+	for _, st := range stores {
+		if len(st.Sections) != 1 {
+			t.Errorf("%s has %d sections, want 1", st.Name, len(st.Sections))
+		}
+	}
+}
+
+func TestShoppingSections_DeleteClearsItFromItems(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.AddShoppingStore("Costco")
+	stores, _ := s.GetShoppingStores()
+	_ = s.AddShoppingSection(stores[0].ID, "Produce")
+	_ = s.SaveUserShoppingItemInSection("Kale", "Costco", "Produce")
+
+	stores, _ = s.GetShoppingStores()
+	if err := s.DeleteShoppingSection(stores[0].Sections[0].ID); err != nil {
+		t.Fatalf("delete section: %v", err)
+	}
+
+	items, _ := s.GetUserShoppingItems()
+	if len(items) != 1 {
+		t.Fatalf("item was deleted along with its section")
+	}
+	if items[0].Section != "" {
+		t.Errorf("Section = %q, want cleared", items[0].Section)
+	}
+	if items[0].Store != "Costco" {
+		t.Errorf("Store = %q, want unchanged", items[0].Store)
+	}
+}
+
+// Deleting a store's section must cascade, not leave orphaned rows pointing at
+// a store that no longer exists.
+func TestShoppingSections_CascadeWhenStoreDeleted(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.AddShoppingStore("Costco")
+	stores, _ := s.GetShoppingStores()
+	_ = s.AddShoppingSection(stores[0].ID, "Produce")
+
+	if err := s.DeleteShoppingStore(stores[0].ID); err != nil {
+		t.Fatalf("delete store: %v", err)
+	}
+	var n int
+	if err := s.db.QueryRow(`SELECT COUNT(*) FROM shopping_sections`).Scan(&n); err != nil {
+		t.Fatalf("count: %v", err)
+	}
+	if n != 0 {
+		t.Errorf("%d orphaned sections remain", n)
+	}
+}
+
+// ── item editing ─────────────────────────────────────────────────────────────
+
+func TestUpdateUserShoppingItem_RenamesAndMoves(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.SaveUserShoppingItemInSection("Mlik", "Costco", "Dairy")
+	items, _ := s.GetUserShoppingItems()
+
+	if err := s.UpdateUserShoppingItem(items[0].ID, "Milk", "Safeway", "Refrigerated"); err != nil {
+		t.Fatalf("update: %v", err)
+	}
+
+	got, err := s.GetUserShoppingItem(items[0].ID)
+	if err != nil {
+		t.Fatalf("get: %v", err)
+	}
+	if got.Name != "Milk" || got.Store != "Safeway" || got.Section != "Refrigerated" {
+		t.Errorf("got %+v, want renamed and moved", got)
+	}
+}
+
+func TestUpdateUserShoppingItem_ClearingTheSection(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.SaveUserShoppingItemInSection("Kale", "Costco", "Produce")
+	items, _ := s.GetUserShoppingItems()
+
+	if err := s.UpdateUserShoppingItem(items[0].ID, "Kale", "Costco", ""); err != nil {
+		t.Fatalf("update: %v", err)
+	}
+	got, _ := s.GetUserShoppingItem(items[0].ID)
+	if got.Section != "" {
+		t.Errorf("Section = %q, want cleared", got.Section)
+	}
+}
+
+// A SQL UPDATE that matches nothing still succeeds, so a stale id would look
+// like a successful edit and the user would think the change stuck.
+func TestUpdateUserShoppingItem_UnknownIDIsAnError(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	if err := s.UpdateUserShoppingItem(999, "Ghost", "Costco", ""); !errors.Is(err, ErrItemNotFound) {
+		t.Errorf("got %v, want ErrItemNotFound", err)
+	}
+}
+
+func TestUpdateUserShoppingItem_RejectsBlankNameOrStore(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+
+	_ = s.SaveUserShoppingItem("Milk", "Costco")
+	items, _ := s.GetUserShoppingItems()
+
+	if err := s.UpdateUserShoppingItem(items[0].ID, "  ", "Costco", ""); err == nil {
+		t.Error("expected an error for a blank name")
+	}
+	if err := s.UpdateUserShoppingItem(items[0].ID, "Milk", "  ", ""); err == nil {
+		t.Error("expected an error for a blank store")
+	}
+}
+
+func TestGetUserShoppingItem_NotFound(t *testing.T) {
+	s := setupRegistryStore(t)
+	defer func() { _ = s.Close() }()
+	if _, err := s.GetUserShoppingItem(999); !errors.Is(err, ErrItemNotFound) {
+		t.Errorf("got %v, want ErrItemNotFound", err)
+	}
+}