diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index f929396..9a9f4b5 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -309,7 +309,8 @@ func (s *Service) List(ctx context.Context, status string) ([]Dinner, error) { // clears its recipe_url, raises its grounding_revision and enqueues a fresh // KindGroundDinner job that grounds the new name: a job queued or running // for the old name or link carries the old revision, so it can no longer -// write the dinner, and one running is stopped. +// write the dinner, and one running is stopped. Renaming any dinner drops +// the candidates offered for its old name: none of them grounds it. func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy string) (Dinner, error) { if updatedBy == "" { return Dinner{}, errors.New("dinners: update: no member to record as updated_by") @@ -320,7 +321,7 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri } var row db.Dinner - var regrounding bool + var renamed, regrounding bool err = s.st.WithTx(ctx, func(tx *sql.Tx) error { q := db.New(tx) cur, err := q.GetDinner(ctx, id) @@ -374,12 +375,16 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri if err != nil { return fmt.Errorf("dinners: update %s: %w", id, err) } + renamed = row.NormalizedName != cur.NormalizedName regrounding = row.GroundingRevision != cur.GroundingRevision return nil }) if err != nil { return Dinner{}, err } + if renamed { + s.clearOffers(row.ID) + } if regrounding { s.stopGroundings(row.ID, row.GroundingRevision) jobID, err := s.enqueueGrounding(ctx, row.ID, row.GroundingRevision) diff --git a/internal/dinners/grounding.go b/internal/dinners/grounding.go index 60fe2f0..5fff6f4 100644 --- a/internal/dinners/grounding.go +++ b/internal/dinners/grounding.go @@ -304,10 +304,12 @@ var offerLifetime = 30 * time.Minute var now = time.Now // offerSet is the candidates last offered for a dinner: the page URL of -// each candidate id. They ground the dinner until deadline; expiry drops -// the set from offers once it has passed. +// each candidate id, found by searching the dinner's name, normalized as +// name. They ground the dinner until deadline, and only while it still has +// that name; expiry drops the set from offers once deadline has passed. type offerSet struct { urls map[string]string + name string deadline time.Time expiry *time.Timer } @@ -322,11 +324,11 @@ type offerSearch struct { // recipe pages to ground it in, best first; the list may be empty. They // replace the dinner's earlier candidates: one not offered again no longer // grounds it. They ground it for offerLifetime, and not at all once it is -// grounded by hand or archived, even while this search runs. A search that -// fails wraps recipes.ErrSearchFailed. +// grounded by hand, renamed or archived, even while this search runs. A +// search that fails wraps recipes.ErrSearchFailed. func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error) { - // Tracked before the dinner is read: grounding or archiving it after - // the read makes this search stale. + // Tracked before the dinner is read: grounding, renaming or archiving + // it after the read makes this search stale. keep, done := s.trackOffers(id) defer done() row, err := s.get(ctx, id) @@ -349,15 +351,16 @@ func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error } } if row.Status != StatusArchived { - keep(urls) + keep(urls, row.NormalizedName) } return out, nil } // trackOffers registers a Candidates search for dinner id. It returns keep, -// which stores urls as the dinner's offers for offerLifetime unless -// clearOffers has run for it since, and done, to call when the search ends. -func (s *Service) trackOffers(id string) (keep func(urls map[string]string), done func()) { +// which stores urls, found for the normalized name, as the dinner's offers +// for offerLifetime unless clearOffers has run for it since, and done, to +// call when the search ends. +func (s *Service) trackOffers(id string) (keep func(urls map[string]string, name string), done func()) { search := &offerSearch{} s.offersMu.Lock() if s.offerSearches[id] == nil { @@ -365,14 +368,14 @@ func (s *Service) trackOffers(id string) (keep func(urls map[string]string), don } s.offerSearches[id][search] = struct{}{} s.offersMu.Unlock() - keep = func(urls map[string]string) { + keep = func(urls map[string]string, name string) { s.offersMu.Lock() defer s.offersMu.Unlock() if search.stale { return } s.dropOffers(id) - set := &offerSet{urls: urls, deadline: now().Add(offerLifetime)} + set := &offerSet{urls: urls, name: name, deadline: now().Add(offerLifetime)} // expiry waits for offersMu, so it finds set stored even when it // fires at once, and drops set only, never offers stored later. set.expiry = time.AfterFunc(offerLifetime, func() { @@ -397,7 +400,7 @@ func (s *Service) trackOffers(id string) (keep func(urls map[string]string), don // clearOffers drops dinner id's offers and makes every Candidates search // running for it stale, so no candidate offered before grounds it. Call -// it once the dinner is grounded by hand or archived. +// it once the dinner is grounded by hand, renamed or archived. func (s *Service) clearOffers(id string) { s.offersMu.Lock() defer s.offersMu.Unlock() @@ -425,15 +428,15 @@ func (s *Service) candidateID(id, rawURL string) string { return base64.RawURLEncoding.EncodeToString(mac.Sum(nil)[:16]) } -// candidateURL is the page URL of candidate cid last offered for dinner id. +// candidateURL is the page URL of candidate cid last offered for dinner d. // A cid not among the dinner's live offers, or whose page is not a valid -// recipe link, is a *ValidationError. Offers past their deadline are not -// live, even before their expiry drops them. -func (s *Service) candidateURL(id, cid string) (string, error) { +// recipe link, is a *ValidationError. Offers past their deadline, or found +// for a name d no longer has, are not live, even before they are dropped. +func (s *Service) candidateURL(d db.Dinner, cid string) (string, error) { var raw string var ok bool s.offersMu.Lock() - if set := s.offers[id]; set != nil && now().Before(set.deadline) { + if set := s.offers[d.ID]; set != nil && set.name == d.NormalizedName && now().Before(set.deadline) { raw, ok = set.urls[cid] } s.offersMu.Unlock() @@ -499,7 +502,7 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string return Dinner{}, ErrArchived } if f.CandidateID != nil { - if link, err = s.candidateURL(cur.ID, *f.CandidateID); err != nil { + if link, err = s.candidateURL(cur, *f.CandidateID); err != nil { return Dinner{}, err } } diff --git a/internal/dinners/grounding_test.go b/internal/dinners/grounding_test.go index 4604419..520730b 100644 --- a/internal/dinners/grounding_test.go +++ b/internal/dinners/grounding_test.go @@ -804,8 +804,8 @@ func wantStaleCandidate(t *testing.T, e *env, id, cid string) { } // TestGroundingOrArchivingClearsCandidates: once a dinner is grounded by -// hand — by candidate, link or manual recipe — or archived, the service -// holds none of its candidates' URLs and none of them grounds it. +// hand — by candidate, link or manual recipe — renamed or archived, the +// service holds none of its candidates' URLs and none of them grounds it. func TestGroundingOrArchivingClearsCandidates(t *testing.T) { t.Parallel() tests := []struct { @@ -834,6 +834,11 @@ func TestGroundingOrArchivingClearsCandidates(t *testing.T) { t.Fatal(err) } }, false}, + {"rename", func(t *testing.T, e *env, id, _ string) { + if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Name: ptr("Beef stew")}, member); err != nil { + t.Fatal(err) + } + }, false}, {"archive", func(t *testing.T, e *env, id, _ string) { if _, err := e.svc.Archive(t.Context(), id, member); err != nil { t.Fatal(err) @@ -860,9 +865,56 @@ func TestGroundingOrArchivingClearsCandidates(t *testing.T) { } } +// TestRenamePendingRefusesOldCandidates: candidates offered for a pending +// dinner's old name no longer ground it once it is renamed — Ground refuses +// them and leaves the old recipe off it — while candidates for the new name +// do. +func TestRenamePendingRefusesOldCandidates(t *testing.T) { + t.Parallel() + // Unstarted, so the dinner stays pending with its jobs queued. + e := newUnstartedEnv(t) + e.search.Results["Beef stew"] = []recipes.Candidate{{URL: stew}} + c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry")}, member) + if err != nil { + t.Fatal(err) + } + old := candidateID(t, e, c.ID, "Chicken Curry") + + d, err := e.svc.Update(t.Context(), c.ID, dinners.Patch{Name: ptr("Beef stew")}, member) + if err != nil { + t.Fatal(err) + } + if d.GroundingStatus != dinners.GroundingPending { + t.Fatalf("renamed = %+v, want still pending", d) + } + if e.svc.HasOffers(c.ID) { + t.Errorf("service still holds the old name's candidates of dinner %s", c.ID) + } + wantStaleCandidate(t, e, c.ID, old) + if row := e.dinner(t, c.ID); row.RecipeID.Valid || row.GroundingStatus != dinners.GroundingPending { + t.Fatalf("dinner = %+v, want pending with no recipe", row) + } + + e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150)) + g, err := e.svc.Ground(t.Context(), c.ID, dinners.Fix{CandidateID: ptr(candidateID(t, e, c.ID, "Beef Stew"))}, member) + if err != nil { + t.Fatal(err) + } + if g.GroundingStatus != dinners.GroundingGrounded || g.RecipeID == nil { + t.Fatalf("dinner = %+v, want grounded for the new name", g) + } + rec, err := db.New(e.st.Reader).GetRecipe(t.Context(), *g.RecipeID) + if err != nil { + t.Fatal(err) + } + if rec.SourceUrl != stew { + t.Errorf("recipe source = %s, want %s", rec.SourceUrl, stew) + } +} + // TestCandidatesRunningWhenCleared: a candidate search still running when -// its dinner is grounded by hand or archived stores none of the candidates -// it returns, so none of them grounds the dinner. +// its dinner is grounded by hand, renamed or archived stores none of the +// candidates it returns, so none of them grounds the dinner. func TestCandidatesRunningWhenCleared(t *testing.T) { t.Parallel() tests := []struct { @@ -877,6 +929,11 @@ func TestCandidatesRunningWhenCleared(t *testing.T) { t.Fatal(err) } }, false}, + {"rename", func(t *testing.T, e *env, id string) { + if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Name: ptr("Beef stew")}, member); err != nil { + t.Fatal(err) + } + }, false}, {"archive", func(t *testing.T, e *env, id string) { if _, err := e.svc.Archive(t.Context(), id, member); err != nil { t.Fatal(err) diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index e06992c..3e25759 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -1,4 +1,4 @@ -import { act, render, screen } from '@testing-library/react' +import { act, cleanup, render, screen } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createApiClient, createVerifiedSession } from './api/index.ts' import { createNetworkStatus } from './pwa/networkStatus.ts' @@ -236,6 +236,13 @@ describe('App offline bootstrap', () => { vi.spyOn(navigator, 'onLine', 'get').mockImplementation(() => onLine) }) + // A test that emits an offline event leaves TanStack Query's online + // manager, shared by every test, offline until an online event. + afterEach(() => { + cleanup() + window.dispatchEvent(new Event('online')) + }) + it.each([ ['/plan', 'Plan'], ['/dinner/01J9ZK', 'Dinner'], @@ -250,6 +257,31 @@ describe('App offline bootstrap', () => { expect(screen.queryByRole('alert')).not.toBeInTheDocument() }) + it('opens the saved page when the browser has emitted an offline event', async () => { + await visitOnline() + // /me is still loading, with the app mounted, when the browser emits + // its offline event; only then does /me fail. + let failMe!: () => void + vi.stubGlobal( + 'fetch', + vi.fn().mockImplementationOnce( + () => new Promise((_, reject) => (failMe = () => reject(new TypeError('Failed to fetch')))), + ).mockRejectedValue(new TypeError('Failed to fetch')), + ) + renderAt('/plan') + await vi.waitFor(() => expect(failMe).toBeDefined()) + onLine = false + act(() => { + window.dispatchEvent(new Event('offline')) + }) + act(() => failMe()) + + expect(await screen.findByRole('heading', { level: 1, name: 'Plan' })).toBeInTheDocument() + expect(screen.queryByRole('main', { name: 'Loading MealQuest' })).not.toBeInTheDocument() + expect(screen.getByRole('status')).toHaveTextContent('Offline — showing saved data') + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + it('opens the saved pages when the server is unreachable but the browser reports online', async () => { await visitOnline() vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))) diff --git a/web/src/onboarding/OnboardingGate.tsx b/web/src/onboarding/OnboardingGate.tsx index 473b33d..1e5c853 100644 --- a/web/src/onboarding/OnboardingGate.tsx +++ b/web/src/onboarding/OnboardingGate.tsx @@ -19,7 +19,8 @@ function inOnboarding(pathname: string): boolean { // The service worker never caches the settings, so offline on the saved // pages (see AuthBootstrap) they cannot be read: the saved pages open as they // are, since onboarding needs the server anyway, and the settings are read -// again when the browser is back online. +// again when the browser is back online. The read runs even while the +// browser reports offline, so that case fails fast instead of waiting. export function OnboardingGate() { const api = useApi() const { pathname } = useLocation() diff --git a/web/src/settings/api.ts b/web/src/settings/api.ts index be27f9d..058caf0 100644 --- a/web/src/settings/api.ts +++ b/web/src/settings/api.ts @@ -367,12 +367,16 @@ function parseHouseholdSettings(where: string, body: unknown): HouseholdSettings export const HOUSEHOLD_SETTINGS_KEY = ['settings', 'household'] as const +// useHouseholdSettings runs even while the browser reports offline, like +// /me: OnboardingGate reads it before every page, so offline it must fail +// into the saved pages instead of waiting for the network. export function useHouseholdSettings() { const { apiFetch } = useApi() return useQuery({ queryKey: HOUSEHOLD_SETTINGS_KEY, queryFn: async () => parseHouseholdSettings(`GET ${HOUSEHOLD_SETTINGS_PATH}`, await apiFetch(HOUSEHOLD_SETTINGS_PATH)), + networkMode: 'always', }) }