diff --git a/internal/dinners/detail.go b/internal/dinners/detail.go index 3764a7b..dfaf779 100644 --- a/internal/dinners/detail.go +++ b/internal/dinners/detail.go @@ -60,11 +60,12 @@ type Ingredient struct { // Source is the site a recipe was fetched from. SiteName is the name the // page itself gives its site, and null when it gives none or the stored -// name could have come from a link (see pageSiteName), so a client shows -// "Recipe from {site}" only when SiteName is not null. URL is the page's +// name could have come from a link (see pageSiteName). URL is the page's // link only when it is verifiably free of secrets (see safeSourceURL), and // null otherwise: a link pasted or offered as a candidate may carry a // personal token, so a client links the credit only when URL is not null. +// Source is set for every fetched recipe, so a client always credits it: +// by SiteName, else by URL's host, else without naming the site. type Source struct { SiteName *string `json:"site_name"` URL *string `json:"url"` @@ -189,8 +190,8 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) // stored. Nothing records which a stored name is, so any name that could // be a host — the link's own host, an IP address, or a dotted host name — // is treated as one and withheld. A site that names itself as a domain, -// such as "Food.com", loses its attribution with them: a recipe with no -// attribution is better than one that leaks a link. +// such as "Food.com", loses its name with them: a credit that does not +// name the site is better than one that leaks a link. func pageSiteName(stored sql.NullString, sourceURL string) (*string, error) { if !stored.Valid { return nil, nil diff --git a/web/src/dinners/DinnerDetailPage.test.tsx b/web/src/dinners/DinnerDetailPage.test.tsx index 756b9ab..021f39f 100644 --- a/web/src/dinners/DinnerDetailPage.test.tsx +++ b/web/src/dinners/DinnerDetailPage.test.tsx @@ -204,12 +204,25 @@ describe('DinnerDetailPage', () => { } }) - it('leaves the credit out when the server names no site', async () => { - serve(detail({ source: { site_name: null } })) + it("links the credit by the url's host when the server names no site", async () => { + serve(detail({ source: { site_name: null, url: 'https://Kitchen.Example:8443/korean-beef-bowls' } })) renderAt(['/dinner/d1']) - await screen.findByRole('heading', { level: 1, name: 'Korean beef bowls' }) - expect(screen.queryByText(/Recipe from/)).not.toBeInTheDocument() + const credit = await screen.findByRole('link', { name: 'Recipe from kitchen.example:8443' }) + expect(credit).toHaveAttribute('href', 'https://kitchen.example:8443/korean-beef-bowls') + expect(credit).toHaveAttribute('target', '_blank') + expect(credit).toHaveAttribute('rel', 'noopener noreferrer') + }) + + // The server withholds both when the stored name could be the link's + // host and the link could carry a token: the credit names neither. + it('credits the web in plain text when the server sends neither site nor url', async () => { + serve(detail({ source: { site_name: null, url: null } })) + renderAt(['/dinner/d1']) + + const credit = await screen.findByText('Recipe from the web') + expect(credit.closest('a')).toBeNull() + expect(screen.queryByRole('link', { name: /Recipe from/ })).not.toBeInTheDocument() }) it('holds Add to this week and the heart at the bottom', async () => { diff --git a/web/src/dinners/DinnerDetailPage.tsx b/web/src/dinners/DinnerDetailPage.tsx index 79b1928..9874787 100644 --- a/web/src/dinners/DinnerDetailPage.tsx +++ b/web/src/dinners/DinnerDetailPage.tsx @@ -67,10 +67,14 @@ function IngredientRow({ ingredient }: { ingredient: Ingredient }) { ) } -// SourceCredit is "Recipe from {site}": a link opening the recipe's page in -// a new tab when the server sends a url safe to open, else plain text. -function SourceCredit({ siteName, url }: RecipeSource & { siteName: string }) { - const credit = `Recipe from ${siteName}` +// SourceCredit is "Recipe from {site}" for a recipe fetched from the web: a +// link opening the recipe's page in a new tab when the server sends a url +// safe to open, else plain text. The site is the name the page gave, else +// the url's host — already in the link, so naming it shows nothing more — +// else, with neither to show, just "the web": a fetched recipe always +// credits where it came from, without a name the server withheld. +function SourceCredit({ siteName, url }: RecipeSource) { + const credit = `Recipe from ${siteName ?? (url === null ? 'the web' : new URL(url).host)}` return (

{url === null ? ( @@ -136,9 +140,7 @@ function Recipe({ dinner }: { dinner: DinnerDetail }) { )} - {dinner.source !== null && dinner.source.siteName !== null && ( - - )} + {dinner.source !== null && } ) } @@ -506,7 +508,7 @@ function KitchenReading({ dinner }: { dinner: DinnerDetail }) { {!recipe && } {recipe && dinner.steps.length > 0 && } - {recipe && dinner.source !== null && dinner.source.siteName !== null && ( + {recipe && dinner.source !== null && ( )} diff --git a/web/src/dinners/KitchenReading.test.tsx b/web/src/dinners/KitchenReading.test.tsx index d708fce..e393ae6 100644 --- a/web/src/dinners/KitchenReading.test.tsx +++ b/web/src/dinners/KitchenReading.test.tsx @@ -410,6 +410,36 @@ describe('Kitchen reading mode', () => { }) }) + describe('source credit', () => { + it.each([ + ['a site and a url', { site_name: 'Serious Eats', url: 'https://www.seriouseats.com/bowls' }, 'Recipe from Serious Eats'], + ['only a url', { site_name: null, url: 'https://www.seriouseats.com/bowls' }, 'Recipe from www.seriouseats.com'], + ])('links the recipe page in a new tab for %s', async (_, source, name) => { + serve({ [DINNER]: () => jsonResponse(200, detail({ source })) }) + await renderPage() + const credit = screen.getByRole('link', { name }) + expect(credit).toHaveAttribute('href', 'https://www.seriouseats.com/bowls') + expect(credit).toHaveAttribute('target', '_blank') + expect(credit).toHaveAttribute('rel', 'noopener noreferrer') + }) + + it.each([ + ['only a site', { site_name: 'Serious Eats', url: null }, 'Recipe from Serious Eats'], + ['neither site nor url', { site_name: null, url: null }, 'Recipe from the web'], + ])('is plain text for %s', async (_, source, text) => { + serve({ [DINNER]: () => jsonResponse(200, detail({ source })) }) + await renderPage() + expect(screen.getByText(text).closest('a')).toBeNull() + expect(screen.queryByRole('link', { name: /Recipe from/ })).not.toBeInTheDocument() + }) + + it('is absent for a recipe typed in', async () => { + serve({ [DINNER]: () => jsonResponse(200, detail({ source: null, grounding_status: 'manual' })) }) + await renderPage() + expect(screen.queryByText(/Recipe from/)).not.toBeInTheDocument() + }) + }) + it('shows what stands in for a recipe not found, with no step buttons', async () => { serve({ [DINNER]: () => diff --git a/web/src/dinners/detail.ts b/web/src/dinners/detail.ts index 48ef513..e5386b2 100644 --- a/web/src/dinners/detail.ts +++ b/web/src/dinners/detail.ts @@ -36,9 +36,8 @@ export type Ingredient = { // RecipeSource is where a fetched recipe came from. siteName is the site, // or null when there is none to name: a page that named no site, or a name // the server withheld because it could have come from the link. url is the -// recipe's page, or null: today's API never sends the stored link, which can -// carry a token, so the credit is text until the server sends a url it has -// checked is free of secrets. +// recipe's page, or null when the server withheld the stored link because +// it could carry a token; the credit then never names its host. export type RecipeSource = { siteName: string | null; url: string | null } // DinnerDetail is a dinner with its recipe (GET /api/v1/dinners/{id}). @@ -93,7 +92,7 @@ function detailPhotoId(where: string, value: unknown): string | undefined { return id } -// sourceUrl is source.url, which today's API does not send, or a link safe +// sourceUrl is source.url, null when the server withholds it, or a link safe // to open: https, with no user, password, query or fragment. function sourceUrl(where: string, value: unknown): string | null { if (value === undefined || value === null) return null