Allow store deletion when only archived lists reference it
Archived lists are no longer part of anyone''s workflow, so they shouldn''t block a store from being deleted. The handler now blocks only on non-archived lists and purges archived ones (items cascade) in the same transaction. Also grooms BACKLOG.md to remove items already shipped (Family/multi- tenant epic, password reset, email invites, auth rate limiting, picked-up-vs-removed, per-store sections base feature, add-recipe-to- list, store delete confirm + duplicate-name 409).
This commit is contained in:
+7
-73
@@ -6,27 +6,10 @@ Speculative or longer-term ideas live in `ideas.md`.
|
|||||||
|
|
||||||
## Foundations
|
## 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
|
### 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 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 (per `Program.cs`) likely 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.
|
- **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.
|
- **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.
|
||||||
- **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.
|
|
||||||
|
|
||||||
### Real-time conflict resolution
|
### 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).
|
- 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.
|
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.
|
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
|
#### Data model sketch
|
||||||
- `Product` (global catalog, read-only): `Id`, `Name`, `DefaultUnit?`, `Brand?`, `Notes?`, search keys.
|
- `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.
|
- `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").
|
- 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.
|
- 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
|
## Shopping list items
|
||||||
|
|
||||||
### Structured quantities + unit of measure
|
### 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.
|
- 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.
|
- "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
|
### Per-store sections — remaining polish
|
||||||
- Each store has its own section layout (Produce, Meat/Seafood, Condiments, Frozen, Bakery, Dairy, etc.) — sections are properties of the store, not global.
|
The base feature is shipped (entity, default seed on store create, list view groups by section). Remaining nice-to-haves:
|
||||||
- Schema:
|
- **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.
|
||||||
- New `StoreSection` entity: `Id`, `StoreId`, `Name`, `SortOrder` (controls walk order through the store). Unique on `(StoreId, Name)`.
|
- **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).
|
||||||
- `ShoppingListItem.SectionId` nullable FK to `StoreSection` — null means "uncategorized".
|
- **Section drag-to-reorder** in the store edit UI — section walk order matters, but reordering today only works by editing `SortOrder` numbers manually.
|
||||||
- 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.
|
|
||||||
|
|
||||||
## Lists
|
## 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
|
### 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.
|
- 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.
|
- 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.
|
- 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)?
|
- 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).
|
|
||||||
|
|||||||
@@ -70,7 +70,7 @@ public class StoreEndpointsTests : AuthenticatedIntegrationTest
|
|||||||
}
|
}
|
||||||
|
|
||||||
[Test]
|
[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();
|
var store = await Data.CreateStoreAsync();
|
||||||
await Data.CreateListAsync(b => b.ForStore(store).CreatedBy(User));
|
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();
|
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]
|
[Test]
|
||||||
public async Task Create_returns_409_for_duplicate_name()
|
public async Task Create_returns_409_for_duplicate_name()
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -71,8 +71,16 @@ public static class StoreEndpoints
|
|||||||
var store = await db.Stores.FirstOrDefaultAsync(s => s.Id == id && s.FamilyId == familyId);
|
var store = await db.Stores.FirstOrDefaultAsync(s => s.Id == id && s.FamilyId == familyId);
|
||||||
if (store is null) return Results.NotFound();
|
if (store is null) return Results.NotFound();
|
||||||
|
|
||||||
var hasLists = await db.ShoppingLists.AnyAsync(l => l.StoreId == id && l.FamilyId == familyId);
|
var hasActiveLists = await db.ShoppingLists
|
||||||
if (hasLists) return Results.Conflict(new { error = "Store has shopping lists. Remove them first." });
|
.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);
|
db.Stores.Remove(store);
|
||||||
await db.SaveChangesAsync();
|
await db.SaveChangesAsync();
|
||||||
|
|||||||
Reference in New Issue
Block a user