diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3227230..d50ac13 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -194,7 +194,9 @@ automatic** (D-021). `GET /api/v1/dinners?status=regular|quest_log` returns `{dinners: [...]}` ordered by normalized name, each with `photo_url` (`/media/photos/{photo_id}-400.jpg`, or null) and `grounding_status`; - any other status is 400. `PATCH /api/v1/dinners/{id}` edits `name` + any other status is 400. The Quest log (`quest_log`) is the dinners + accepted and not yet scheduled: one on the plan for the household's + current week (by its timezone) or a later week is left out. `PATCH /api/v1/dinners/{id}` edits `name` (deduplicated against the other unarchived dinners), `method` (`stovetop` / `oven` / `crockpot` / `grill` / `other`), `minutes_total` (1–1440) and `difficulty` (`easy` / `medium` / `hard`); invalid values diff --git a/internal/db/dinners.sql.go b/internal/db/dinners.sql.go index 74c929d..e8bb637 100644 --- a/internal/db/dinners.sql.go +++ b/internal/db/dinners.sql.go @@ -501,6 +501,63 @@ func (q *Queries) ListUnarchivedDinners(ctx context.Context) ([]ListUnarchivedDi return items, nil } +const listUnplannedQuestLogDinners = `-- name: ListUnplannedQuestLogDinners :many +SELECT id, name, normalized_name, status, origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, photo_id, grounding_status, updated_by, created_at, updated_at, recipe_url, grounding_error, grounding_revision FROM dinners +WHERE status = 'quest_log' + AND NOT EXISTS ( + SELECT 1 FROM plan_days + JOIN week_plans ON week_plans.id = plan_days.plan_id + WHERE plan_days.dinner_id = dinners.id + AND week_plans.week_start >= ?1 + ) +ORDER BY normalized_name, id +` + +// The Quest log: dinners with status quest_log that no plan for the week +// starting from_week_start or later has on any day. +func (q *Queries) ListUnplannedQuestLogDinners(ctx context.Context, fromWeekStart string) ([]Dinner, error) { + rows, err := q.db.QueryContext(ctx, listUnplannedQuestLogDinners, fromWeekStart) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Dinner + for rows.Next() { + var i Dinner + if err := rows.Scan( + &i.ID, + &i.Name, + &i.NormalizedName, + &i.Status, + &i.Origin, + &i.Method, + &i.MinutesTotal, + &i.Difficulty, + &i.Cuisine, + &i.IsFavorite, + &i.RecipeID, + &i.PhotoID, + &i.GroundingStatus, + &i.UpdatedBy, + &i.CreatedAt, + &i.UpdatedAt, + &i.RecipeUrl, + &i.GroundingError, + &i.GroundingRevision, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const setDinnerFavorite = `-- name: SetDinnerFavorite :one UPDATE dinners SET is_favorite = ?, diff --git a/internal/db/querier.go b/internal/db/querier.go index b6094eb..4765334 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -124,6 +124,9 @@ type Querier interface { // Every dinner a new or renamed dinner must not repeat, with the // source_url of the recipe it is grounded in (NULL while it has none). ListUnarchivedDinners(ctx context.Context) ([]ListUnarchivedDinnersRow, error) + // The Quest log: dinners with status quest_log that no plan for the week + // starting from_week_start or later has on any day. + ListUnplannedQuestLogDinners(ctx context.Context, fromWeekStart string) ([]Dinner, error) // Moves a dinner still suggested to status (quest_log or archived), // recording updated_by; a dinner no longer suggested is left as it is // (0 rows). diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index ae2a20b..f68c771 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -16,10 +16,12 @@ import ( "slices" "strings" "sync" + "time" "unicode/utf8" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" @@ -273,12 +275,20 @@ func (s *Service) createRegular(ctx context.Context, r NewRegular, updatedBy str } // List returns the dinners with status, which must be regular or -// quest_log, ordered by normalized name. +// quest_log, ordered by normalized name. The Quest log is the dinners +// accepted and not yet scheduled, so a quest_log dinner on the plan for the +// household's current week or a later one is left out of it. func (s *Service) List(ctx context.Context, status string) ([]Dinner, error) { if !slices.Contains(listable, status) { return nil, invalid("status", "must be one of "+strings.Join(listable, ", ")) } - rows, err := db.New(s.st.Reader).ListDinnersByStatus(ctx, status) + var rows []db.Dinner + var err error + if status == StatusQuestLog { + rows, err = s.listQuestLog(ctx) + } else { + rows, err = db.New(s.st.Reader).ListDinnersByStatus(ctx, status) + } if err != nil { return nil, fmt.Errorf("dinners: list %s: %w", status, err) } @@ -289,6 +299,29 @@ func (s *Service) List(ctx context.Context, status string) ([]Dinner, error) { return out, nil } +// listQuestLog reads the Quest log in one read transaction, so the +// household's current week and the plans come from the same snapshot. +func (s *Service) listQuestLog(ctx context.Context) (rows []db.Dinner, err error) { + tx, err := s.st.Reader.BeginTx(ctx, nil) + if err != nil { + return nil, fmt.Errorf("begin read: %w", err) + } + defer func() { + err = errors.Join(err, tx.Rollback()) + }() + q := db.New(tx) + settings, err := q.GetHouseholdSettings(ctx) + if err != nil { + return nil, fmt.Errorf("read household settings: %w", err) + } + loc, err := household.LoadZone(settings.Timezone) + if err != nil { + return nil, fmt.Errorf("household settings: %w", err) + } + thisWeek := household.WeekStart(household.Today(time.Now(), loc)) + return q.ListUnplannedQuestLogDinners(ctx, thisWeek.String()) +} + // Update applies p to dinner id, recording updatedBy. An invalid patch is a // *ValidationError and a new name repeating another unarchived dinner a // *DuplicateError; either changes nothing. Renaming a dinner still pending diff --git a/internal/server/dinners_test.go b/internal/server/dinners_test.go index a8c312f..2d7861b 100644 --- a/internal/server/dinners_test.go +++ b/internal/server/dinners_test.go @@ -11,9 +11,11 @@ import ( "sync" "testing" "testing/fstest" + "time" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest" @@ -66,6 +68,10 @@ func (b *logBuffer) String() string { func newDinnersEnv(t *testing.T) *dinnersEnv { t.Helper() st := newTestStore(t) + // The Quest log is read against the household's current week. + if err := household.SeedSettings(context.Background(), st, "America/New_York"); err != nil { + t.Fatal(err) + } e := &dinnersEnv{ st: st, search: &recipestest.Search{Results: map[string][]recipes.Candidate{}}, @@ -414,6 +420,61 @@ func TestListDinnersFiltersByStatus(t *testing.T) { } } +// The Quest log leaves out a dinner on the plan for the household's current +// week or a later one: it is scheduled already. A dinner on an earlier +// week's plan only, or on none, is listed. +func TestQuestLogLeavesOutPlannedDinners(t *testing.T) { + t.Parallel() + router, st := newDinnersRouter(t) + ctx := context.Background() + names := []string{"Bibimbap", "Gumbo", "Pho", "Ramen", "Tagine"} + ids := map[string]string{} + for _, name := range names { + id := createDinner(t, router, `{"name":"`+name+`"}`) + if _, err := st.Writer.ExecContext(ctx, `UPDATE dinners SET status = 'quest_log', origin = 'side_quest' WHERE id = ?`, id); err != nil { + t.Fatal(err) + } + ids[name] = id + } + + loc, err := time.LoadLocation("America/New_York") + if err != nil { + t.Fatal(err) + } + thisWeek := household.WeekStart(household.Today(time.Now(), loc)) + // day is the date days after the Monday weeks after this week's. + day := func(weeks, days int) string { + y, m, d := thisWeek.Date() + return time.Date(y, m, d+7*weeks+days, 0, 0, 0, 0, time.UTC).Format(time.DateOnly) + } + // Gumbo is on last week's plan only; Pho on this week's; Ramen on the + // week after next; Tagine on last week's and this week's. + plan := func(weeks int, dinners ...string) { + t.Helper() + weekStart := day(weeks, 0) + planID := "plan-" + weekStart + if _, err := st.Writer.ExecContext(ctx, + `INSERT INTO week_plans (id, week_start, status, created_at, updated_at) VALUES (?, ?, 'draft', 't', 't')`, + planID, weekStart); err != nil { + t.Fatal(err) + } + for i, name := range dinners { + if _, err := st.Writer.ExecContext(ctx, + `INSERT INTO plan_days (id, plan_id, date, dinner_id, is_new, created_at, updated_at) VALUES (?, ?, ?, ?, 1, 't', 't')`, + planID+"-"+name, planID, day(weeks, i), ids[name]); err != nil { + t.Fatal(err) + } + } + } + plan(-1, "Gumbo", "Tagine") + plan(0, "Pho", "Tagine") + plan(2, "Ramen") + + if got := strings.Join(listNames(t, router, "quest_log"), ","); got != "Bibimbap,Gumbo" { + t.Errorf("quest_log = %s, want Bibimbap,Gumbo", got) + } +} + func TestPatchDinner(t *testing.T) { t.Parallel() router, st := newDinnersRouter(t) diff --git a/migrations/queries/dinners.sql b/migrations/queries/dinners.sql index 43e96ae..ecd95bd 100644 --- a/migrations/queries/dinners.sql +++ b/migrations/queries/dinners.sql @@ -19,6 +19,19 @@ SELECT * FROM dinners WHERE status = ? ORDER BY normalized_name, id; +-- name: ListUnplannedQuestLogDinners :many +-- The Quest log: dinners with status quest_log that no plan for the week +-- starting from_week_start or later has on any day. +SELECT * FROM dinners +WHERE status = 'quest_log' + AND NOT EXISTS ( + SELECT 1 FROM plan_days + JOIN week_plans ON week_plans.id = plan_days.plan_id + WHERE plan_days.dinner_id = dinners.id + AND week_plans.week_start >= sqlc.arg(from_week_start) + ) +ORDER BY normalized_name, id; + -- name: CountRegularDinners :one -- How many Regulars the household has; archived dinners are not counted. SELECT COUNT(*) FROM dinners diff --git a/web/src/dinners/api.ts b/web/src/dinners/api.ts index dc5a556..06e040c 100644 --- a/web/src/dinners/api.ts +++ b/web/src/dinners/api.ts @@ -106,7 +106,8 @@ export function photoId(where: string, value: unknown): string | undefined { return decodeURIComponent(match[1]) } -function parseDinner(where: string, body: unknown): Regular { +// parseDinner checks a dinner as the dinners API sends it. +export function parseDinner(where: string, body: unknown): Regular { if (!isRecord(body) || typeof body.id !== 'string' || typeof body.name !== 'string') { throw new Error(`${where}: response is not a dinner`) } @@ -141,7 +142,7 @@ function parseCandidate(where: string, body: unknown): RecipeCandidate { } } -function dinnerPath(id: string): string { +export function dinnerPath(id: string): string { return `${DINNERS_PATH}/${encodeURIComponent(id)}` } diff --git a/web/src/discover/DeckCard.tsx b/web/src/discover/DeckCard.tsx index 9c4f119..26758be 100644 --- a/web/src/discover/DeckCard.tsx +++ b/web/src/discover/DeckCard.tsx @@ -7,7 +7,7 @@ import styles from './DiscoverPage.module.css' // DECK_DEPTH is the depths the deck shows, top card first. export const DECK_DEPTH = [0, 1, 2] as const -const DIFFICULTY_LABEL: Record = { easy: 'Easy', medium: 'Medium', hard: 'Hard' } +export const DIFFICULTY_LABEL: Record = { easy: 'Easy', medium: 'Medium', hard: 'Hard' } // SwipeHandlers are the pointer handlers that drag the top card. export type SwipeHandlers = Pick< diff --git a/web/src/discover/DiscoverPage.module.css b/web/src/discover/DiscoverPage.module.css index a8a98d4..ae35eb2 100644 --- a/web/src/discover/DiscoverPage.module.css +++ b/web/src/discover/DiscoverPage.module.css @@ -59,26 +59,6 @@ font-weight: 700; } -/* The Quest log's dinner names, under the header while the pill is open. */ -.questLogList { - flex: 1 0 100%; - font-family: var(--font-body); - font-size: 14px; -} - -.questLogNames { - margin: 0; - padding-left: 20px; - display: flex; - flex-direction: column; - gap: 6px; -} - -.questLogEmpty { - margin: 0; - color: var(--color-neutral-700); -} - /* The deck: three cards stacked in the same box, the top one in front. */ .deck { position: relative; diff --git a/web/src/discover/DiscoverPage.test.tsx b/web/src/discover/DiscoverPage.test.tsx index 66d941c..f1f73f0 100644 --- a/web/src/discover/DiscoverPage.test.tsx +++ b/web/src/discover/DiscoverPage.test.tsx @@ -1,10 +1,12 @@ import { QueryClientProvider } from '@tanstack/react-query' import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react' -import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' +import { createMemoryRouter, RouterProvider, useParams } from 'react-router' +import { afterEach, beforeEach, describe, expect, it, onTestFinished, vi, type Mock } from 'vitest' import { ApiProvider, createApiClient, createQueryClient, createVerifiedSession } from '../api/index.ts' import { createNetworkStatus } from '../pwa/networkStatus.ts' import { stubAnimate, type FakeAnimation } from '../test/animate.ts' import { declaredStyle } from '../test/cssRules.ts' +import { dinnerBody, type DinnerBody } from '../test/dinners.ts' import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' import { expectMinHitArea } from '../test/hitArea.ts' import { mockViewport } from '../test/matchMedia.ts' @@ -14,6 +16,12 @@ import { DiscoverPage } from './DiscoverPage.tsx' const DECK = '/api/v1/suggestions/deck' const QUEST_LOG = '/api/v1/dinners?status=quest_log' +// questLogBody is a Quest-log dinner as GET /api/v1/dinners?status=quest_log +// sends it. +function questLogBody(id: string, name: string, overrides: Partial = {}) { + return dinnerBody(id, name, { status: 'quest_log', origin: 'side_quest', ...overrides }) +} + // SuggestionBody is a suggestion as GET /api/v1/suggestions/deck sends it. function suggestionBody(id: string, name: string, overrides: Record = {}) { return { @@ -51,7 +59,7 @@ function stubServer(deck: unknown[], responders: Record = {}, 'GET /api/v1/me': () => jsonResponse(200, ME), [`GET ${DECK}`]: () => jsonResponse(200, { suggestions: server.deck }), [`GET ${QUEST_LOG}`]: () => - jsonResponse(200, { dinners: Array.from({ length: server.questLog }, (_, i) => ({ id: `q${i}`, name: `Quest ${i}` })) }), + jsonResponse(200, { dinners: Array.from({ length: server.questLog }, (_, i) => questLogBody(`q${i}`, `Quest ${i}`)) }), ...responders, } server.fetchMock = vi.fn((input, init) => { @@ -102,15 +110,27 @@ async function finishRunning(animations: FakeAnimation[]) { }) } +// DinnerProbe stands in for the dinner screen, showing the id it opened. +function DinnerProbe() { + return

Dinner screen for {useParams().id}

+} + // renderPage loads /me (for the CSRF token every write needs) and renders -// the Discover page with queryClient. +// the Discover page with queryClient at /discover, with /dinner/:id beside it. async function renderPage(queryClient = createQueryClient()) { const api = createApiClient(fakeLocation('/discover'), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() + const router = createMemoryRouter( + [ + { path: '/discover', element: }, + { path: '/dinner/:id', element: }, + ], + { initialEntries: ['/discover'] }, + ) return render( - + , ) @@ -359,34 +379,6 @@ describe('DiscoverPage', () => { await screen.findByRole('article', { name: 'Korean beef bowls' }) }) - it('opens and closes the Quest log from its pill, a keyboard-reachable button', async () => { - stubServer([BOWLS], {}, 2) - await renderPage() - const pill = await screen.findByRole('button', { name: 'Quest log · 2' }) - expect(pill).toHaveAttribute('aria-expanded', 'false') - expect(screen.queryByRole('list', { name: 'Quest log' })).not.toBeInTheDocument() - expectMinHitArea(pill) - pill.focus() - expect(pill).toHaveFocus() - - fireEvent.click(pill) - expect(pill).toHaveAttribute('aria-expanded', 'true') - const list = screen.getByRole('list', { name: 'Quest log' }) - expect(pill).toHaveAttribute('aria-controls', list.parentElement!.id) - expect(within(list).getAllByRole('listitem').map((item) => item.textContent)).toEqual(['Quest 0', 'Quest 1']) - - fireEvent.click(pill) - expect(pill).toHaveAttribute('aria-expanded', 'false') - expect(screen.queryByRole('list', { name: 'Quest log' })).not.toBeInTheDocument() - }) - - it('says so when the Quest log it opens is empty', async () => { - stubServer([BOWLS], {}, 0) - await renderPage() - fireEvent.click(await screen.findByRole('button', { name: 'Quest log · 0' })) - expect(screen.getByText('No dinners in the Quest log yet. Try it on a Side quest to add one.')).toBeInTheDocument() - }) - it('sends one decision for presses made before the page re-renders', async () => { const accept = deferred() const server = stubServer([BOWLS, TIKKA], { 'POST /api/v1/suggestions/s1/accept': () => accept.promise }) @@ -511,6 +503,254 @@ describe('DiscoverPage', () => { expect(screen.getByRole('heading', { name: 'All caught up' })).toBeInTheDocument() }) + describe('Quest log sheet', () => { + const PAD_THAI = questLogBody('q1', 'Pad thai', { photo_url: '/media/photos/p9-400.jpg', difficulty: 'medium', minutes_total: 35 }) + const GUMBO = questLogBody('q2', 'Shrimp gumbo', { difficulty: 'hard', minutes_total: null }) + const RISOTTO = questLogBody('q3', 'Mushroom risotto') + + // stubQuestLog serves the deck and server.dinners as the Quest log. + function stubQuestLog(dinners: unknown[], responders: Record = {}) { + const questLog = { dinners } + const server = stubServer([BOWLS], { [`GET ${QUEST_LOG}`]: () => jsonResponse(200, { dinners: questLog.dinners }), ...responders }) + return Object.assign(server, { questLogDinners: questLog }) + } + + async function openSheet(count: number) { + fireEvent.click(await screen.findByRole('button', { name: `Quest log · ${count}` })) + return screen.getByRole('dialog', { name: 'Quest log' }) + } + + function rows(sheet: HTMLElement) { + return within(within(sheet).getByRole('list', { name: 'Quest log' })).getAllByRole('listitem') + } + + // names is the dinners the sheet lists, read off their remove buttons. + function names(sheet: HTMLElement) { + return rows(sheet).map((row) => within(row).getByRole('button', { name: /^Remove / }).getAttribute('aria-label')!.slice('Remove '.length)) + } + + it('opens from the "Quest log · N" pill and lists each dinner: 48 px washed photo, name, difficulty and minutes', async () => { + stubQuestLog([PAD_THAI, GUMBO, RISOTTO]) + await renderPage() + const pill = await screen.findByRole('button', { name: 'Quest log · 3' }) + expect(pill).toHaveAttribute('aria-haspopup', 'dialog') + expect(pill).toHaveAttribute('aria-expanded', 'false') + expectMinHitArea(pill) + pill.focus() + + fireEvent.click(pill) + const sheet = screen.getByRole('dialog', { name: 'Quest log' }) + expect(pill).toHaveAttribute('aria-expanded', 'true') + const [padThai, gumbo, risotto] = rows(sheet) + expect(names(sheet)).toEqual(['Pad thai', 'Shrimp gumbo', 'Mushroom risotto']) + expect(within(padThai!).getByText('Medium · 35 min')).toBeInTheDocument() + expect(within(gumbo!).getByText('Hard')).toBeInTheDocument() + // Nothing known, no meta line. + expect(within(risotto!).getByRole('button', { name: /^Mushroom risotto/ })).toHaveTextContent(/^dish photoMushroom risotto$/) + + // The photo is the stored one, through the washed treatment, at 48 px. + const photo = padThai!.querySelector('img')! + expect(photo).toHaveAttribute('src', '/media/photos/p9-400.jpg') + expect(declaredStyle(photo).filter).toBe('saturate(0.6) contrast(0.85) brightness(1.1) opacity(0.94)') + expect(declaredStyle(photo).width).toBe('48px') + expect(declaredStyle(photo).height).toBe('48px') + // Without a photo, the striped placeholder stands in. + expect(gumbo!.querySelector('img')).toBeNull() + expect(within(gumbo!).getByText('dish photo')).toBeInTheDocument() + + for (const row of [padThai!, gumbo!, risotto!]) { + for (const button of within(row).getAllByRole('button')) expectMinHitArea(button) + } + + fireEvent.keyDown(sheet, { key: 'Escape' }) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + expect(pill).toHaveAttribute('aria-expanded', 'false') + expect(pill).toHaveFocus() + }) + + it('lists only the dinners the Quest log read returns, so ones already on the current or a future plan stay out', async () => { + // The server leaves out dinners on the current or a later week's plan; + // the sheet adds none and works out nothing of its own. + const server = stubQuestLog([RISOTTO]) + await renderPage() + const sheet = await openSheet(1) + expect(names(sheet)).toEqual(['Mushroom risotto']) + expect(server.fetchMock.mock.calls.map(([input]) => String(input)).filter((path) => path.startsWith('/api/v1/plans'))).toEqual([]) + }) + + it('opens /dinner/:id from a row', async () => { + stubQuestLog([PAD_THAI, GUMBO]) + await renderPage() + const sheet = await openSheet(2) + fireEvent.click(within(sheet).getByRole('button', { name: /^Shrimp gumbo/ })) + expect(await screen.findByText('Dinner screen for q2')).toBeInTheDocument() + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + }) + + it('removes a dinner only once confirmed, archiving it and taking one off the count', async () => { + const server = stubQuestLog([PAD_THAI, GUMBO], { + 'DELETE /api/v1/dinners/q1': () => { + server.questLogDinners.dinners = [GUMBO] + return jsonResponse(200, { ...PAD_THAI, status: 'archived' }) + }, + }) + await renderPage() + const sheet = await openSheet(2) + + fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' }) + expect(confirm).toHaveTextContent('It leaves your Quest log for good.') + fireEvent.click(within(confirm).getByRole('button', { name: 'Keep it' })) + expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument() + expect(rows(sheet)).toHaveLength(2) + expect(sent(server.fetchMock)).toEqual([]) + + fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + fireEvent.click(within(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).getByRole('button', { name: 'Remove' })) + await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument()) + expect(sent(server.fetchMock)).toEqual(['DELETE /api/v1/dinners/q1']) + expect(names(sheet)).toEqual(['Shrimp gumbo']) + expect(screen.getByRole('button', { name: 'Quest log · 1' })).toBeInTheDocument() + // The Quest log is read again after the removal, and agrees. + await waitFor(() => + expect(server.fetchMock.mock.calls.filter(([input]) => String(input) === QUEST_LOG)).toHaveLength(2), + ) + expect(screen.getByRole('button', { name: 'Quest log · 1' })).toBeInTheDocument() + }) + + it('keeps focus in the open Quest log after a removal: on the next row, the one before the last, then the sheet', async () => { + const archive = (id: string) => () => { + server.questLogDinners.dinners = server.questLogDinners.dinners.filter((dinner) => dinner !== byId[id]) + return jsonResponse(200, { ...byId[id]!, status: 'archived' }) + } + const byId: Record = { q1: PAD_THAI, q2: GUMBO, q3: RISOTTO } + const server = stubQuestLog([PAD_THAI, GUMBO, RISOTTO], { + 'DELETE /api/v1/dinners/q1': archive('q1'), + 'DELETE /api/v1/dinners/q2': archive('q2'), + 'DELETE /api/v1/dinners/q3': archive('q3'), + }) + await renderPage() + const sheet = await openSheet(3) + + async function removeAndExpectFocus(name: string, expected: () => HTMLElement) { + within(sheet).getByRole('button', { name: `Remove ${name}` }).focus() + fireEvent.click(within(sheet).getByRole('button', { name: `Remove ${name}` })) + fireEvent.click(within(screen.getByRole('dialog', { name: `Remove ${name}?` })).getByRole('button', { name: 'Remove' })) + await waitFor(() => expect(screen.queryByRole('dialog', { name: `Remove ${name}?` })).not.toBeInTheDocument()) + expect(expected()).toHaveFocus() + expect(sheet).toContainElement(document.activeElement as HTMLElement) + } + + // A middle row: focus goes to the row now in its place. + await removeAndExpectFocus('Shrimp gumbo', () => within(sheet).getByRole('button', { name: 'Remove Mushroom risotto' })) + // The last row: focus goes to the row before it. + await removeAndExpectFocus('Mushroom risotto', () => within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + // The only row: focus goes to the sheet itself. + await removeAndExpectFocus('Pad thai', () => sheet) + expect(within(sheet).getByText('Your Quest log is empty — swipe right on a side quest to add one.')).toBeInTheDocument() + + // From there Escape still reaches the Quest log, and closes it. + fireEvent.keyDown(document.activeElement!, { key: 'Escape' }) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + }) + + it.each([ + ['Keep it', (confirm: HTMLElement) => fireEvent.click(within(confirm).getByRole('button', { name: 'Keep it' }))], + ['Escape', (confirm: HTMLElement) => fireEvent.keyDown(confirm, { key: 'Escape' })], + ['the scrim', (confirm: HTMLElement) => fireEvent.click(confirm.previousElementSibling!)], + ])('keeps the confirmation and sends one removal once Remove is activated: %s and a second Remove at once do nothing', async (_, dismiss) => { + const archive = deferred() + const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => archive.promise }) + await renderPage() + const sheet = await openSheet(2) + + fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' }) + const removeButton = within(confirm).getByRole('button', { name: 'Remove' }) + // Straight after Remove, without waiting for the pending request to render. + fireEvent.click(removeButton) + fireEvent.click(removeButton) + dismiss(confirm) + expect(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).toBe(confirm) + + expect(await within(confirm).findByRole('button', { name: 'Removing…' })).toBeDisabled() + expect(within(confirm).getByRole('button', { name: 'Keep it' })).toBeDisabled() + dismiss(confirm) + expect(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).toBe(confirm) + + server.questLogDinners.dinners = [GUMBO] + await act(async () => archive.resolve(jsonResponse(200, { ...PAD_THAI, status: 'archived' }))) + await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument()) + expect(names(sheet)).toEqual(['Shrimp gumbo']) + expect(sent(server.fetchMock)).toEqual(['DELETE /api/v1/dinners/q1']) + }) + + it.each([ + ['keeps a disabled button focused', false], + ['moves focus off a disabled button to the page', true], + ])('keeps focus in the confirmation, off its disabled buttons, while a removal is under way when the browser %s', async (_, fixup) => { + // Some browsers move focus from a control to the page as it is disabled. + const setAttribute = Element.prototype.setAttribute + if (fixup) { + vi.spyOn(Element.prototype, 'setAttribute').mockImplementation(function (this: Element, name, value) { + // jsdom ignores blur() on a disabled control, so focus leaves just before. + if (name === 'disabled' && this === document.activeElement && this instanceof HTMLElement) this.blur() + setAttribute.call(this, name, value) + }) + } + onTestFinished(() => { + vi.restoreAllMocks() + }) + const archive = deferred() + const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => archive.promise }) + await renderPage() + const sheet = await openSheet(2) + + fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' }) + const removeButton = within(confirm).getByRole('button', { name: 'Remove' }) + removeButton.focus() + fireEvent.click(removeButton) + expect(removeButton).toBeDisabled() + expect(confirm).toHaveFocus() + + expect(await within(confirm).findByRole('button', { name: 'Removing…' })).toBeDisabled() + expect(confirm).toHaveFocus() + + server.questLogDinners.dinners = [GUMBO] + await act(async () => archive.resolve(jsonResponse(200, { ...PAD_THAI, status: 'archived' }))) + await waitFor(() => expect(screen.queryByRole('dialog', { name: 'Remove Pad thai?' })).not.toBeInTheDocument()) + expect(within(sheet).getByRole('button', { name: 'Remove Shrimp gumbo' })).toHaveFocus() + }) + + it('keeps a dinner whose removal the server refuses, saying why', async () => { + const server = stubQuestLog([PAD_THAI, GUMBO], { 'DELETE /api/v1/dinners/q1': () => errorResponse(500, 'internal') }) + await renderPage() + const sheet = await openSheet(2) + + fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + const confirm = screen.getByRole('dialog', { name: 'Remove Pad thai?' }) + fireEvent.click(within(confirm).getByRole('button', { name: 'Remove' })) + expect(await within(confirm).findByRole('alert')).toHaveTextContent("Couldn't remove it. internal message") + expect(sent(server.fetchMock)).toEqual(['DELETE /api/v1/dinners/q1']) + expect(rows(sheet)).toHaveLength(2) + expect(screen.getByRole('button', { name: 'Quest log · 2' })).toBeInTheDocument() + + // Keep it closes the confirmation and clears the error. + fireEvent.click(within(confirm).getByRole('button', { name: 'Keep it' })) + fireEvent.click(within(sheet).getByRole('button', { name: 'Remove Pad thai' })) + expect(within(screen.getByRole('dialog', { name: 'Remove Pad thai?' })).queryByRole('alert')).not.toBeInTheDocument() + }) + + it('says how to fill an empty Quest log', async () => { + stubQuestLog([]) + await renderPage() + const sheet = await openSheet(0) + expect(within(sheet).getByText('Your Quest log is empty — swipe right on a side quest to add one.')).toBeInTheDocument() + expect(within(sheet).queryByRole('list')).not.toBeInTheDocument() + }) + }) + describe('optimistic decisions', () => { const PAELLA = suggestionBody('s4', 'Sheet pan paella') const SOUP = suggestionBody('s5', 'Tortellini soup') @@ -733,8 +973,8 @@ describe('DiscoverPage', () => { [`GET ${QUEST_LOG}`]: () => jsonResponse(200, { dinners: [ - { id: 'q0', name: 'Quest 0' }, - { id: 'dinner-s1', name: 'Korean beef bowls' }, + questLogBody('q0', 'Quest 0'), + questLogBody('dinner-s1', 'Korean beef bowls'), ], }), }) @@ -754,7 +994,7 @@ describe('DiscoverPage', () => { it('takes out of the Quest log only the entry a refused Try it added, after a read of the Quest log lands during it', async () => { const accept = deferred() - const questLog = [{ id: 'q0', name: 'Quest 0' }] + const questLog = [questLogBody('q0', 'Quest 0')] const server = stubServer([BOWLS, TIKKA, ORZO, PAELLA], { 'POST /api/v1/suggestions/s1/accept': () => accept.promise, [`GET ${QUEST_LOG}`]: () => jsonResponse(200, { dinners: questLog }), @@ -770,7 +1010,7 @@ describe('DiscoverPage', () => { // The decision is saved, and a read of the Quest log lands with its // dinner in it; then the answer to the decision is lost. - questLog.push({ id: 'dinner-s1', name: 'Korean beef bowls' }) + questLog.push(questLogBody('dinner-s1', 'Korean beef bowls')) await act(async () => await queryClient.invalidateQueries({ queryKey: ['dinners', 'quest_log'] })) accept.resolve(errorResponse(500, 'internal')) await screen.findByRole('alert') diff --git a/web/src/discover/DiscoverPage.tsx b/web/src/discover/DiscoverPage.tsx index 078286b..5e9e010 100644 --- a/web/src/discover/DiscoverPage.tsx +++ b/web/src/discover/DiscoverPage.tsx @@ -1,8 +1,7 @@ import { useQueryClient } from '@tanstack/react-query' import { Heart, X } from 'lucide-react' -import { useCallback, useEffect, useId, useLayoutEffect, useMemo, useRef, useState, type PointerEvent, type Ref } from 'react' +import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, type PointerEvent, type Ref } from 'react' import { useJob } from '../api/index.ts' -import { Card } from '../ui/Card.tsx' import { EmptyState } from '../ui/EmptyState.tsx' import { ErrorBanner } from '../ui/ErrorBanner.tsx' import { Icon } from '../ui/Icon.tsx' @@ -19,6 +18,7 @@ import { useQuestLog, useStartBatch, type Choice, + type QuestLogDinner, type SideQuest, } from './api.ts' import { DeckCard, DECK_DEPTH, type SwipeHandlers } from './DeckCard.tsx' @@ -35,6 +35,7 @@ import { unfollow, type Exit, } from './motion.ts' +import { QuestLogSheet } from './QuestLogSheet.tsx' import styles from './DiscoverPage.module.css' // SWIPE_THRESHOLD is how far, in px, the top card must be dragged left or @@ -54,36 +55,22 @@ function isEditable(target: EventTarget | null): boolean { ) } -// QuestLogPill is the "Quest log · N" pill. It shows and hides the names -// of the dinners in the Quest log; the Quest log sheet replaces this list. -// ref is the visible pill, which Try it bumps. -function QuestLogPill({ dinners, ref }: { dinners: { id: string; name: string }[]; ref: Ref }) { +// QuestLogPill is the "Quest log · N" pill, which opens the Quest log +// sheet. ref is the visible pill, which Try it bumps. +function QuestLogPill({ dinners, ref }: { dinners: QuestLogDinner[]; ref: Ref }) { const [open, setOpen] = useState(false) - const listId = useId() return ( <> - {open && ( - - {dinners.length === 0 ? ( -

No dinners in the Quest log yet. Try it on a Side quest to add one.

- ) : ( -
    - {dinners.map((dinner) => ( -
  • {dinner.name}
  • - ))} -
- )} -
- )} + setOpen(false)} /> ) } diff --git a/web/src/discover/QuestLogSheet.module.css b/web/src/discover/QuestLogSheet.module.css new file mode 100644 index 0000000..75fce9c --- /dev/null +++ b/web/src/discover/QuestLogSheet.module.css @@ -0,0 +1,38 @@ +/* The Quest log sheet (mock 1a's "Quest log · N" pill opens it). */ +.sheet { + display: flex; + flex-direction: column; + gap: 14px; + max-width: 560px; + margin: 0 auto; +} + +.title { + margin: 0; + font-family: var(--font-heading); + font-size: 22px; + font-weight: 400; +} + +.rows { + margin: 0; + padding: 0; + list-style: none; + display: flex; + flex-direction: column; + gap: var(--space-1); +} + +/* The 48 px washed photo; Photo applies the washed filter. */ +.photo { + width: 48px; + height: 48px; +} + +.empty { + margin: 0; + font-family: var(--font-body); + font-size: 14px; + line-height: 1.45; + color: var(--color-neutral-700); +} diff --git a/web/src/discover/QuestLogSheet.tsx b/web/src/discover/QuestLogSheet.tsx new file mode 100644 index 0000000..cafdfa7 --- /dev/null +++ b/web/src/discover/QuestLogSheet.tsx @@ -0,0 +1,147 @@ +import { Trash2 } from 'lucide-react' +import { useEffect, useId, useRef, useState } from 'react' +import { useNavigate } from 'react-router' +import { Confirm } from '../settings/LeaveGuard.tsx' +import { Icon } from '../ui/Icon.tsx' +import { IconButton } from '../ui/IconButton.tsx' +import { ListRow } from '../ui/ListRow.tsx' +import { Photo } from '../ui/Photo.tsx' +import { Sheet } from '../ui/Sheet.tsx' +import { useRemoveFromQuestLog, type QuestLogDinner } from './api.ts' +import { DIFFICULTY_LABEL } from './DeckCard.tsx' +import styles from './QuestLogSheet.module.css' + +export type QuestLogSheetProps = { + open: boolean + // The Quest log, as useQuestLog reads it. + dinners: QuestLogDinner[] + onClose: () => void +} + +// details is a Quest-log dinner's "difficulty · minutes" line, or null when +// neither is known. +function details(dinner: QuestLogDinner): string | null { + const parts = [ + dinner.difficulty !== null && DIFFICULTY_LABEL[dinner.difficulty], + dinner.minutes_total !== null && `${dinner.minutes_total} min`, + ].filter((part): part is string => typeof part === 'string') + return parts.length === 0 ? null : parts.join(' · ') +} + +// QuestLogSheet is the Quest log as a bottom sheet: each dinner with its +// washed photo, name, difficulty and minutes. A row opens the dinner; its +// remove button, once confirmed, archives the dinner. +export function QuestLogSheet({ open, dinners, onClose }: QuestLogSheetProps) { + const titleId = useId() + const navigate = useNavigate() + const remove = useRemoveFromQuestLog() + // The dinner whose removal is being confirmed. + const [removing, setRemoving] = useState(null) + // The dinner just removed and its place in the list, until focus has + // moved on from it. + const removed = useRef<{ id: string; index: number } | null>(null) + const removeButtons = useRef(new Map()) + const sheetRef = useRef(null) + // Whether a removal has been sent and not yet settled. The ref is set as + // Remove is activated, so a dismissal or a second Remove that arrives before + // the next render is already refused; the state renders it. + const sending = useRef(false) + const [sendingShown, setSendingShown] = useState(false) + + // Once a removal's confirmation has closed and the dinner has left the list, + // focus goes to the remove button of the dinner now in its place, or of the + // one before it when it was the last, or to the Quest log sheet itself when + // none is left. Otherwise it would be left on the removed dinner's button, + // which the confirmation hands it back to, and which is then gone. + useEffect(() => { + const done = removed.current + if (removing !== null || done === null || dinners.some((d) => d.id === done.id)) return + removed.current = null + const next = dinners[Math.min(done.index, dinners.length - 1)] + if (next) removeButtons.current.get(next.id)!.focus() + else sheetRef.current!.closest('[role="dialog"]')!.focus() + }, [removing, dinners]) + + function setSending(value: boolean) { + sending.current = value + setSendingShown(value) + } + + function cancelRemove() { + if (sending.current) return + remove.reset() + setRemoving(null) + } + + function confirmRemove() { + if (sending.current) return + setSending(true) + remove.mutate(removing!, { + onSuccess: (_data, dinner) => { + removed.current = { id: dinner.id, index: dinners.findIndex((d) => d.id === dinner.id) } + setRemoving(null) + }, + onSettled: () => setSending(false), + }) + } + + return ( + <> + +
+

+ Quest log +

+ {dinners.length === 0 ? ( +

Your Quest log is empty — swipe right on a side quest to add one.

+ ) : ( +
    + {dinners.map((dinner) => ( +
  • +
  • + ))} +
+ )} +
+
+ + It leaves your Quest log for good. + + + ) +} diff --git a/web/src/discover/api.ts b/web/src/discover/api.ts index c233de3..84a1917 100644 --- a/web/src/discover/api.ts +++ b/web/src/discover/api.ts @@ -1,7 +1,7 @@ import { useMutation, useMutationState, useQuery, useQueryClient } from '@tanstack/react-query' import { useApi } from '../api/index.ts' import { isRecord } from '../api/errors.ts' -import { DIFFICULTIES, DINNERS_PATH, oneOf, photoId, type Difficulty } from '../dinners/api.ts' +import { DIFFICULTIES, DINNERS_PATH, dinnerPath, oneOf, parseDinner, photoId, type Difficulty, type Regular } from '../dinners/api.ts' export const SUGGESTIONS_PATH = '/api/v1/suggestions' export const DECK_PATH = `${SUGGESTIONS_PATH}/deck` @@ -39,8 +39,9 @@ export const BATCH_JOB_KIND = 'suggestion_batch' // BatchResult is the suggestion batch job's result, published as it goes. export type BatchResult = { batch_id: string; stage: string; suggestion_ids: string[]; dropped: number } -// QuestLogDinner is a dinner in the Quest log, as the Quest log pill lists it. -export type QuestLogDinner = { id: string; name: string } +// QuestLogDinner is a dinner in the Quest log, as the Quest log sheet lists +// it: photo, name, difficulty and minutes. +export type QuestLogDinner = Pick // DECISION_STATUS is the status a suggestion has once each choice is saved. const DECISION_STATUS: Record = { try: 'accepted', pass: 'declined' } @@ -97,7 +98,8 @@ export function useDeck() { } // useQuestLog is the dinners in the Quest log, in the server's order (GET -// /api/v1/dinners?status=quest_log). +// /api/v1/dinners?status=quest_log). The server leaves out a dinner already +// on the plan for the current week or a later one: it is scheduled. export function useQuestLog() { const { apiFetch } = useApi() return useQuery({ @@ -107,15 +109,30 @@ export function useQuestLog() { const body = await apiFetch(QUEST_LOG_PATH) if (!isRecord(body) || !Array.isArray(body.dinners)) throw new Error(`${where}: response has no dinners list`) return body.dinners.map((dinner) => { - if (!isRecord(dinner) || typeof dinner.id !== 'string' || typeof dinner.name !== 'string') { - throw new Error(`${where}: response is not a dinner`) - } - return { id: dinner.id, name: dinner.name } + const { id, name, photo_id, difficulty, minutes_total } = parseDinner(where, dinner) + return { id, name, photo_id, difficulty, minutes_total } }) }, }) } +// useRemoveFromQuestLog archives a Quest-log dinner (DELETE +// /api/v1/dinners/{id}). Once the server has archived it, it leaves the +// Quest log, and the Quest log is read again. +export function useRemoveFromQuestLog() { + const { apiFetch } = useApi() + const queryClient = useQueryClient() + return useMutation({ + mutationFn: async (dinner: QuestLogDinner) => { + await apiFetch(dinnerPath(dinner.id), { method: 'DELETE' }) + }, + onSuccess: async (_data, dinner) => { + queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => dinners?.filter((d) => d.id !== dinner.id)) + await queryClient.invalidateQueries({ queryKey: QUEST_LOG_KEY }) + }, + }) +} + // useStartBatch asks for more Side quests now (POST // /api/v1/suggestions/batches). The server answers with the batch already // running when there is one. @@ -215,7 +232,17 @@ export function useDecide() { if (choice !== 'try') return // A stale deck can offer a Side quest whose dinner the Quest log has. queryClient.setQueryData(QUEST_LOG_KEY, (dinners) => - dinners === undefined || dinners.some((d) => d.id === quest.dinner_id) ? dinners : dinners.concat({ id: quest.dinner_id, name: quest.name }), + dinners === undefined || dinners.some((d) => d.id === quest.dinner_id) + ? dinners + : // The deck does not say the minutes; the Quest log read after the + // decision does. + dinners.concat({ + id: quest.dinner_id, + name: quest.name, + photo_id: quest.photo_id, + difficulty: quest.difficulty, + minutes_total: null, + }), ) }, onSettled: async (_replenishJobId, error, { quest }) => { diff --git a/web/src/settings/LeaveGuard.tsx b/web/src/settings/LeaveGuard.tsx index 35ae711..26ded91 100644 --- a/web/src/settings/LeaveGuard.tsx +++ b/web/src/settings/LeaveGuard.tsx @@ -1,4 +1,4 @@ -import { useEffect, useId, type ReactNode } from 'react' +import { useEffect, useId, useLayoutEffect, useRef, type ReactNode } from 'react' import { useBlocker } from 'react-router' import { Button } from '../ui/Button.tsx' import { Sheet } from '../ui/Sheet.tsx' @@ -36,6 +36,9 @@ type ConfirmProps = { confirmLabel: string cancelLabel?: string busy?: boolean + // When false, the cancel button, Escape and the scrim do nothing, so the + // sheet stays until the action settles. + dismissible?: boolean error?: string | null onCancel: () => void onConfirm: () => void @@ -49,14 +52,28 @@ export function Confirm({ confirmLabel, cancelLabel = 'Cancel', busy = false, + dismissible = true, error = null, onCancel, onConfirm, }: ConfirmProps) { const titleId = useId() + const sheetRef = useRef(null) + // A focused button that becomes disabled either keeps focus while disabled or + // loses it to the page, depending on the browser; the dialog takes it instead. + useLayoutEffect(() => { + const dialog = sheetRef.current?.closest('[role="dialog"]') + if (!dialog) return + const active = document.activeElement + const lost = active === null || active === document.body || (dialog.contains(active) && active.matches(':disabled')) + if (lost) dialog.focus() + }, [busy, dismissible]) + function dismiss() { + if (dismissible) onCancel() + } return ( - -
+ +

{title}

@@ -67,7 +84,7 @@ export function Confirm({

)}
-