diff --git a/BACKLOG.md b/BACKLOG.md index 1a44d35..2602966 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -6,27 +6,10 @@ Speculative or longer-term ideas live in `ideas.md`. ## Foundations -### Family entity + multi-tenant migration (epic) -Prerequisite for the product catalog and unit catalog work — both rely on a `FamilyId` for scoping. Tracked separately so it doesn't get buried inside the catalog item. -- Introduce a `Family` entity. Every existing user/store/list/recipe/section/item row gets a `FamilyId` and an API filter applies it everywhere. -- Per-family invite codes replace the env-var `FAMILY_CODE`. Decide: rotating? expiring? single-use? Recommend regenerable per-family codes that admins control. -- Member roles: at minimum `Admin` vs `Member`. Admins can manage stores, archive lists, invite/remove members, regenerate the family code. Members can do everyday list/recipe work. -- Member removal flow: what happens to their `CreatedByUserId`, `CheckedByUserId` references? Likely keep the FK (history) but tombstone the user record so they can't log in. -- **Bootstrap migration for the existing single-family deployment:** at upgrade time, auto-create one `Family` from the current `FAMILY_CODE` env value, assign all existing rows to it, and migrate the env var off. Flag explicitly in the runbook so deployment isn't a surprise. -- Auth changes: JWT carries `FamilyId` claim. `OnTokenValidated` rejects tokens whose user has been removed from the family. - ### Account & session lifecycle -- **Password reset.** Currently no flow, no email infrastructure. Required before multi-family launches publicly. Needs SMTP config, email template, single-use token table, rate limiting on the reset endpoint. -- **JWT refresh tokens.** Today's tokens are signed with HS256 and (per `Program.cs`) likely have no refresh path; they expire and the user gets bounced to login. Add refresh tokens with rotation + revocation list. +- **JWT refresh tokens.** Today's tokens are signed with HS256 and have no refresh path; they expire and the user gets bounced to login. Add refresh tokens with rotation + revocation list. - **Account deletion / data export.** GDPR-style request handling. Family-scoped: deleting a user shouldn't delete the family's data, but should redact PII. -- **Rate limiting on `/api/auth/register` and `/api/auth/login`.** Currently unlimited — with multi-family this becomes a real abuse vector. -- **Email-based invites (admin-sent) doubling as email confirmation.** Today the `InviteCode` is a shareable secret an admin distributes out-of-band; new users self-register with it and there's no proof they own a real email address. Build an invite flow where a family admin enters an email, the backend issues a single-use, time-limited invite token and sends a templated email containing a join link. Clicking the link drops the user into a registration form pre-bound to that family + email; on submit, both the invite is consumed and the email is marked confirmed in one step (no second "confirm your email" round-trip needed). Implications: - - New table for invites: `(Id, FamilyId, Email, TokenHash, IssuedByUserId, IssuedAt, ExpiresAt, ConsumedAt, ConsumedByUserId)`. Hash the token (don't store raw) so a DB leak doesn't leak active invites. - - `User` gains an `Email` (unique) and `EmailConfirmedAt` column. Migration backfill: existing users have null email until they (or an admin) attach one. - - SMTP infrastructure shared with the password-reset item above — design both together; same templating, same rate limits, same outbound queue if we add one. - - Admin UI on `/family`: list pending invites, resend, revoke. Pair with the existing role/membership management. - - Keep the old `InviteCode` flow available initially (or as an admin-only "generate shareable code" fallback) so we don't strand families that prefer the link-share workflow. Revisit deprecation once email invites are battle-tested. - - Open question: do we allow registering without an invite at all once email invites land? Recommend yes-for-now (existing `InviteCode` path stays) so this can ship incrementally. +- **Member removal (tombstone) flow.** When a family admin removes a member, keep the `CheckedByUserId` / `RemovedByUserId` FKs intact for history but tombstone the user so they can no longer authenticate, and ensure `OnTokenValidated` rejects tokens for removed users. ### Real-time conflict resolution - The deferred SignalR-versioning plan in memory addresses connect-window races. It does **not** address edit conflicts (two family members renaming the same store, unchecking-then-checking the same item simultaneously). @@ -71,11 +54,6 @@ A `Product` becomes the canonical thing being bought. Shopping list items and re 4. **Custom products and edits are scoped to the family that made them** — never visible to other families. 5. Long-term: track family-level edits/additions to surface promotion candidates for the global catalog. -#### Architectural prerequisite — multi-tenant model -- Today the app is single-tenant per deployment (one `FAMILY_CODE` per deployment, no `Family` entity). The "isolated per family" requirement implies a `Family` entity and family-scoped tenant boundaries. -- **Decision (2026-05-06):** going with option (a) — one deployment, many families. Existing `FAMILY_CODE` becomes a per-family invite mechanism (each family has its own code), and cross-family promotion analytics become meaningful. -- Either way: introduce `Family` entity, every existing user/store/list/recipe/section row gets a `FamilyId`, and an API filter applies it everywhere. This is a substantial migration — separate epic. - #### Data model sketch - `Product` (global catalog, read-only): `Id`, `Name`, `DefaultUnit?`, `Brand?`, `Notes?`, search keys. - `FamilyProduct` (family-owned): `Id`, `FamilyId`, `Name`, plus the same detail fields. Family-only products live here. @@ -118,20 +96,6 @@ A `Product` becomes the canonical thing being bought. Shopping list items and re - Should recipes use the same product catalog, or stay free-form? Big consistency win if they share, but recipe ingredients tend to be more abstract ("flour") than shopping items ("King Arthur AP Flour 5lb"). - How does this interact with the existing `Store.Name` uniqueness — moot if we add `FamilyId` to everything. -## Shopping list item actions - -### Distinguish "picked up" from "removed" -- Today `ShoppingListItem` only has `IsChecked` (+ `CheckedByUserId`). The UI conflates two genuinely different intents: - - **Picked up** — the item was acquired at the store. Belongs to the shopping history. Should remain visible (greyed out / struck through) so the rest of the family knows it was grabbed and by whom. Reversible (un-check). This is what `IsChecked` already represents. - - **Removed** — the item shouldn't be on the list at all (typo, changed mind, duplicate, decided to skip). Disappears from the active list. Probably soft-deleted so it can be undone within the session. -- Schema: add `RemovedAt` (nullable timestamp) and `RemovedByUserId` (nullable FK) to `ShoppingListItem`. Active list = items where `RemovedAt IS NULL`. Keep the row so we can track "who removed what" and offer undo. -- API: a separate `DELETE /api/lists/{id}/items/{itemId}` for soft-remove (sets `RemovedAt`), distinct from the existing toggle-checked endpoint. A hard-purge can run on list archive. -- UX: - - Tap (or check the checkbox) → marks picked up. Item stays visible, struck through, with the picker's name. - - Swipe-left / explicit trash icon → removes. Show a snackbar with "Undo" for ~5 seconds. - - Don't ever surface a destructive remove behind the same gesture as pick-up — too easy to lose data. -- Reporting: when a list is completed/archived, "what got bought" = items with `IsChecked=true AND RemovedAt IS NULL`. This is also the natural input for the future per-store ingredient-section memory and any "frequently bought" suggestions. - ## Shopping list items ### Structured quantities + unit of measure @@ -192,32 +156,14 @@ Replace free-form `Quantity` strings with a structured `(Quantity, UnitOfMeasure - How aggressively do we pre-populate `ProductAllowedUnit` for the curated catalog? At minimum, common-sense defaults per category (produce → lb/each; dairy → gallon/quart/oz; etc.). Could ship with sensible defaults and let families extend. - "A few", "to taste", "some" — these are real recipe quantities that don't fit `(decimal, unit)`. Probably modeled as a special `IsApproximate` flag with an optional `QuantityNote` rather than forcing them into the structured shape. -### Per-store sections / departments for items -- Each store has its own section layout (Produce, Meat/Seafood, Condiments, Frozen, Bakery, Dairy, etc.) — sections are properties of the store, not global. -- Schema: - - New `StoreSection` entity: `Id`, `StoreId`, `Name`, `SortOrder` (controls walk order through the store). Unique on `(StoreId, Name)`. - - `ShoppingListItem.SectionId` nullable FK to `StoreSection` — null means "uncategorized". -- API: CRUD for sections under a store (`/api/stores/{id}/sections`). Item create/update accepts an optional `sectionId`. -- **List view MUST group items by section in the UI** — this is the core UX payoff. Items render under collapsible section headers, ordered by the store's section `SortOrder` (so the list reads top-to-bottom in walk order through the store). An "Uncategorized" bucket holds items without a section (place at the end). -- UX for assigning section: dropdown on the item row, scoped to the list's store's sections. -- Open questions: - - **Seed defaults?** When a new store is created, should we seed a default section list (Produce, Meat/Seafood, Dairy, Bakery, Frozen, Pantry, Condiments, Beverages, Other) that the user can edit, or start empty? Recommend seeding — saves setup friction. - - **Per-store ingredient memory?** Should the app remember "last time `Bananas` was bought at Kroger it was in Produce" and auto-assign on next add? Big UX win, but adds an `IngredientSection` mapping table per store. Probably v2. - - **Recipes → sections?** When pulling recipe ingredients into a list, do we try to map them to sections? Same answer as above — depends on whether we add the per-store ingredient memory. - - **Section reordering UI?** Drag-to-reorder on the store edit page is the natural fit since section walk order matters. +### Per-store sections — remaining polish +The base feature is shipped (entity, default seed on store create, list view groups by section). Remaining nice-to-haves: +- **Per-store ingredient memory:** remember "last time `Bananas` was bought at Kroger it was in Produce" and auto-assign on next add at that store. Adds an `IngredientSection` mapping table per store. Pairs naturally with the product catalog. +- **Recipes → sections:** when pulling recipe ingredients into a list, map them to the list's store's sections (only meaningful once the per-store ingredient memory or product catalog lands). +- **Section drag-to-reorder** in the store edit UI — section walk order matters, but reordering today only works by editing `SortOrder` numbers manually. ## Lists -### Add recipe to shopping list -- From a recipe page, "Add to shopping list" → user picks an existing list (filtered to lists for the same family). Each `RecipeIngredient` becomes a `ShoppingListItem`: - - If the ingredient has a `ProductId`, the new list item references the same product → its store-section assignment for that list's store auto-applies, and the item lands in the right group. - - If the ingredient is free-form text, the new list item carries the text in `Name` with a null `SectionId` (lands in "Uncategorized"). - - `Quantity` copies through. - - `RecipeId` on the new list item links back to the source recipe (already supported by `ShoppingListItem.RecipeId`). -- UX: confirmation step showing the resolved items + their target sections, with the option to deselect anything before adding. -- Edge case: if the same product is already on the list (and unchecked), prompt to merge quantities vs. add a duplicate row. Probably default to add-duplicate to keep things simple. -- Depends on: product catalog feature, sections feature, quantities on list items. - ### Block create-list flow when no stores exist - A shopping list requires a `Store` (see `ListSummary.store` and the `newStoreId` state in `lists/+page.svelte`), so the create-list flow shouldn't be available until at least one store exists. - Behavior: if user tries to open the create-list UI (or hits the create-list page directly via URL) with zero stores, surface a `toast.warning()` (or modal) that says "You need to create a store first" with a CTA linking to `/stores`. Don't render the empty/broken create form. @@ -233,15 +179,3 @@ Replace free-form `Quantity` strings with a structured `(Quantity, UnitOfMeasure - Starter set of types? Suggested: Grocery, Home Improvement, Big Box, Pharmacy, Specialty, Other. - Does store type affect anything beyond display (e.g., filtering recipes/ingredients to grocery-only stores)? -### Confirm-before-delete + block delete when in use -- Add a modal confirmation prompt before deleting a store (currently one click → gone, no undo). -- Disallow deletion of a store that has any active shopping lists associated with it. -- Backend status: `StoreEndpoints.cs` DELETE already returns 400 with `{error: "Store has shopping lists. Remove them first."}` if any list references the store. Consider switching to 409 Conflict for semantic correctness. -- Frontend status: `stores/+page.svelte` `deleteStore` currently surfaces the backend message via `toast.error()`. Replace with the new confirmation modal that also surfaces this error inline (or as a toast on confirm-action failure). -- Open question: what counts as "active"? Backend currently blocks on *any* list. Decide whether archived/completed lists should still block deletion. - -### Duplicate store name → 500 + silent frontend (bug) -- Adding a store with a name that already exists triggers the unique constraint on `Store.Name`, which leaks out as a 500 from `StoreEndpoints.cs` POST (no exception handling). -- Frontend `addStore` in `stores/+page.svelte` swallows the error — the form just sits there with no feedback. -- Fix: backend should pre-check (or catch the unique-violation) and return 409 with a helpful message; frontend should display a `toast.error()` (e.g. "A store named 'Kroger' already exists") rather than failing silently. -- Same pattern likely affects PUT (rename to an existing name). diff --git a/src/backend/YesChef.Api.IntegrationTests/Features/StoreEndpointsTests.cs b/src/backend/YesChef.Api.IntegrationTests/Features/StoreEndpointsTests.cs index 882b949..b8ea1a5 100644 --- a/src/backend/YesChef.Api.IntegrationTests/Features/StoreEndpointsTests.cs +++ b/src/backend/YesChef.Api.IntegrationTests/Features/StoreEndpointsTests.cs @@ -70,7 +70,7 @@ public class StoreEndpointsTests : AuthenticatedIntegrationTest } [Test] - public async Task Delete_blocks_when_store_has_lists() + public async Task Delete_blocks_when_store_has_active_lists() { var store = await Data.CreateStoreAsync(); await Data.CreateListAsync(b => b.ForStore(store).CreatedBy(User)); @@ -81,6 +81,19 @@ public class StoreEndpointsTests : AuthenticatedIntegrationTest await Assert.That(await UseDbAsync(db => db.Stores.AnyAsync(s => s.Id == store.Id))).IsTrue(); } + [Test] + public async Task Delete_succeeds_when_store_only_has_archived_lists() + { + var store = await Data.CreateStoreAsync(); + await Data.CreateListAsync(b => b.ForStore(store).CreatedBy(User).Archived()); + + var response = await Client.DeleteAsync($"/api/stores/{store.Id}"); + + await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NoContent); + await Assert.That(await UseDbAsync(db => db.Stores.AnyAsync(s => s.Id == store.Id))).IsFalse(); + await Assert.That(await UseDbAsync(db => db.ShoppingLists.AnyAsync(l => l.StoreId == store.Id))).IsFalse(); + } + [Test] public async Task Create_returns_409_for_duplicate_name() { diff --git a/src/backend/YesChef.Api/Features/Stores/StoreEndpoints.cs b/src/backend/YesChef.Api/Features/Stores/StoreEndpoints.cs index 83588e9..f8d8472 100644 --- a/src/backend/YesChef.Api/Features/Stores/StoreEndpoints.cs +++ b/src/backend/YesChef.Api/Features/Stores/StoreEndpoints.cs @@ -71,8 +71,16 @@ public static class StoreEndpoints var store = await db.Stores.FirstOrDefaultAsync(s => s.Id == id && s.FamilyId == familyId); if (store is null) return Results.NotFound(); - var hasLists = await db.ShoppingLists.AnyAsync(l => l.StoreId == id && l.FamilyId == familyId); - if (hasLists) return Results.Conflict(new { error = "Store has shopping lists. Remove them first." }); + var hasActiveLists = await db.ShoppingLists + .AnyAsync(l => l.StoreId == id && l.FamilyId == familyId && !l.IsArchived); + if (hasActiveLists) return Results.Conflict(new { error = "Store has active shopping lists. Archive or remove them first." }); + + // Archived lists no longer belong to anyone's workflow — purge them (and their items, via cascade) + // so the store can be deleted without leaving FK-orphaned history rows. + var archivedLists = await db.ShoppingLists + .Where(l => l.StoreId == id && l.FamilyId == familyId && l.IsArchived) + .ToListAsync(); + if (archivedLists.Count > 0) db.ShoppingLists.RemoveRange(archivedLists); db.Stores.Remove(store); await db.SaveChangesAsync();