Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement thumbnails for multidimensional data pages #4475

Merged
merged 4 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 69 additions & 7 deletions functions/_common/grapherTools.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { generateGrapherImageSrcSet, Grapher } from "@ourworldindata/grapher"
import {
GrapherInterface,
MultiDimDataPageConfigEnriched,
R2GrapherConfigDirectory,
} from "@ourworldindata/types"
import { excludeUndefined, Bounds } from "@ourworldindata/utils"
import {
excludeUndefined,
Bounds,
searchParamsToMultiDimView,
} from "@ourworldindata/utils"
import { StatusError } from "itty-router"
import { Env } from "./env.js"
import { fetchFromR2, grapherBaseUrl } from "./grapherRenderer.js"
Expand Down Expand Up @@ -83,11 +88,30 @@ export async function fetchUnparsedGrapherConfig(
return fetchFromR2(requestUrl, etag, fallbackUrl)
}

export async function fetchGrapherConfig(
identifier: GrapherIdentifier,
env: Env,
async function fetchMultiDimGrapherConfig(
multiDimConfig: MultiDimDataPageConfigEnriched,
searchParams: URLSearchParams,
env: Env
) {
const view = searchParamsToMultiDimView(multiDimConfig, searchParams)
const response = await fetchUnparsedGrapherConfig(
{ type: "uuid", id: view.fullConfigId },
env
)
return await response.json()
}

export async function fetchGrapherConfig({
identifier,
env,
etag,
searchParams,
}: {
identifier: GrapherIdentifier
env: Env
etag?: string
): Promise<FetchGrapherConfigResult> {
searchParams?: URLSearchParams
}): Promise<FetchGrapherConfigResult> {
const fetchResponse = await fetchUnparsedGrapherConfig(
identifier,
env,
Expand All @@ -113,7 +137,17 @@ export async function fetchGrapherConfig(
}
}

const grapherConfig: GrapherInterface = await fetchResponse.json()
const config = await fetchResponse.json()
let grapherConfig: GrapherInterface
if (identifier.type === "multi-dim-slug") {
grapherConfig = await fetchMultiDimGrapherConfig(
config as MultiDimDataPageConfigEnriched,
searchParams,
env
)
} else {
grapherConfig = config
}
console.log("grapher title", grapherConfig.title)
return {
grapherConfig,
Expand All @@ -127,7 +161,35 @@ export async function initGrapher(
searchParams: URLSearchParams,
env: Env
): Promise<Grapher> {
const grapherConfigResponse = await fetchGrapherConfig(identifier, env)
let grapherConfigResponse: FetchGrapherConfigResult
try {
grapherConfigResponse = await fetchGrapherConfig({
identifier,
env,
searchParams,
})
} catch (e) {
if (
identifier.type === "slug" &&
e instanceof StatusError &&
e.status === 404
) {
// Normal graphers and multi-dims have the same URL namespace, but
// we have no way of knowing which of them was requested, so we try
// again with a multi-dim identifier.
const multiDimId: MultiDimSlug = {
type: "multi-dim-slug",
id: identifier.id,
}
grapherConfigResponse = await fetchGrapherConfig({
identifier: multiDimId,
env,
searchParams,
})
} else {
throw e
}
}

if (grapherConfigResponse.status === 404) {
// we throw 404 errors instad of returning a 404 response so that the router
Expand Down
8 changes: 4 additions & 4 deletions functions/grapher/by-uuid/[uuid].ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ async function handleConfigRequest(
const shouldCache = searchParams.get("nocache") === null
console.log("Preparing json response for uuid ", uuid)

const grapherPageResp = await fetchGrapherConfig(
{ type: "uuid", id: uuid },
const grapherPageResp = await fetchGrapherConfig({
identifier: { type: "uuid", id: uuid },
env,
etag
)
etag,
})

if (grapherPageResp.status === 304) {
return new Response(null, { status: 304 })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,5 @@ export interface MultiDimDataPageProps {
primaryTopic?: PrimaryTopic
relatedResearchCandidates: DataPageRelatedResearch[]
imageMetadata: Record<string, ImageMetadata>
initialQueryStr?: string
isPreviewing?: boolean
}
24 changes: 23 additions & 1 deletion packages/@ourworldindata/utils/src/MultiDimDataPageConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
DimensionProperty,
MultiDimDataPageConfigEnriched,
} from "@ourworldindata/types"
import { groupBy, keyBy, pick } from "./Util.js"
import { groupBy, isEmpty, keyBy, pick } from "./Util.js"
import { Url } from "./urls/Url.js"

interface FilterToAvailableResult {
Expand Down Expand Up @@ -217,3 +217,25 @@ export const extractMultiDimChoicesFromQueryStr = (

return dimensionChoices
}

export function searchParamsToMultiDimView(
config: MultiDimDataPageConfigEnriched,
searchParams: URLSearchParams
): ViewEnriched {
const mdimConfig = MultiDimDataPageConfig.fromObject(config)
let dimensions = extractMultiDimChoicesFromQueryStr(
searchParams.toString(),
mdimConfig
)
if (isEmpty(dimensions)) {
// Get the default dimensions.
dimensions = mdimConfig.filterToAvailableChoices({}).selectedChoices
}
const view = mdimConfig.findViewByDimensions(dimensions)
if (!view) {
throw new Error(
`No view found for dimensions ${JSON.stringify(dimensions)}`
)
}
return view
}
1 change: 1 addition & 0 deletions packages/@ourworldindata/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,5 @@ export {
MultiDimDataPageConfig,
extractMultiDimChoicesFromQueryStr,
multiDimStateToQueryStr,
searchParamsToMultiDimView,
} from "./MultiDimDataPageConfig.js"
9 changes: 0 additions & 9 deletions site/DataPageV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,6 @@ export const DataPageV2 = (props: {
}).plaintext
} else pageDesc = "An interactive visualization from Our World in Data."

// Due to thumbnails not taking into account URL parameters, they are often inaccurate on
// social media. We decided to remove them and use a single thumbnail for all charts.
// See https://github.com/owid/owid-grapher/issues/1086
//
// const imageUrl = urljoin(
// baseGrapherUrl,
// "exports",
// `${grapher.slug}.png?v=${grapher.version}`
// )
const imageUrl: string = urljoin(baseUrl, "default-grapher-thumbnail.png")
const imageWidth = "1200"
const imageHeight = "628"
Expand Down
12 changes: 7 additions & 5 deletions site/multiDim/MultiDimDataPage.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import urljoin from "url-join"
import { Head } from "../Head.js"
import { IFrameDetector } from "../IframeDetector.js"
import { SiteHeader } from "../SiteHeader.js"
Expand All @@ -18,7 +19,6 @@ export function MultiDimDataPage({
primaryTopic,
relatedResearchCandidates,
imageMetadata,
initialQueryStr,
isPreviewing,
}: MultiDimDataPageProps) {
const canonicalUrl = `${baseGrapherUrl}/${slug}`
Expand All @@ -31,19 +31,21 @@ export function MultiDimDataPage({
relatedResearchCandidates,
imageMetadata,
tagToSlugMap,
initialQueryStr,
}
const imageUrl: string = urljoin(baseUrl, "default-grapher-thumbnail.png")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't getting overwritten correctly in staging it seems

image

I can't debug it correctly because my local version of the MDD previews don't render when served through the wrangler. Also, even when served by the admin server I get "No table loaded yet" - do you know what's going on there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to rebake the page first (since it was missing due to a bug I'm fixing here), but I see it rewritten there:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some ideas on what might be going on with your local dev setup, and it would be easiest if we could have a call to debug it together. Short summary:

  • Make sure you have the latest code and data in the DB

  • To see the normal mdim page at /grapher (not the admin preview) you need to set published to 1 in multi_dim_data_pages and run the migration script, so it propagates to chart configs with:

    yarn tsx --tsconfig tsconfig.tsx.json devTools/migrateMultiDimConfigs/migrateMultiDimConfigs.ts
    

    (I'll be adding an admin for publishing mdims later this cycle.)

const imageWidth = "1200"
const imageHeight = "628"
return (
<Html>
<Head
canonicalUrl={canonicalUrl}
// pageTitle={pageTitle}
// pageDesc={pageDesc}
// imageUrl={imageUrl}
imageUrl={imageUrl}
baseUrl={baseUrl}
>
{/* <meta property="og:image:width" content={imageWidth} />
<meta property="og:image:height" content={imageHeight} /> */}
<meta property="og:image:width" content={imageWidth} />
<meta property="og:image:height" content={imageHeight} />
<IFrameDetector />
<noscript>
<style>{`
Expand Down
18 changes: 5 additions & 13 deletions site/multiembedder/MultiEmbedder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ import {
Url,
GRAPHER_TAB_OPTIONS,
merge,
MultiDimDataPageConfig,
extractMultiDimChoicesFromQueryStr,
fetchWithRetry,
ChartViewInfo,
searchParamsToMultiDimView,
} from "@ourworldindata/utils"
import { action } from "mobx"
import ReactDOM from "react-dom"
Expand Down Expand Up @@ -238,20 +237,13 @@ class MultiEmbedder {
const { queryStr, slug } = embedUrl

const mdimConfigUrl = `${MULTI_DIM_DYNAMIC_CONFIG_URL}/${slug}.json`
const mdimJsonConfig = await fetchWithRetry(mdimConfigUrl).then((res) =>
const multiDimConfig = await fetchWithRetry(mdimConfigUrl).then((res) =>
res.json()
)
const mdimConfig = MultiDimDataPageConfig.fromObject(mdimJsonConfig)
const dimensions = extractMultiDimChoicesFromQueryStr(
queryStr,
mdimConfig
const view = searchParamsToMultiDimView(
multiDimConfig,
new URLSearchParams(queryStr)
)
const view = mdimConfig.findViewByDimensions(dimensions)
if (!view) {
throw new Error(
`No view found for dimensions ${JSON.stringify(dimensions)}`
)
}

const configUrl = `${GRAPHER_DYNAMIC_CONFIG_URL}/by-uuid/${view.fullConfigId}.config.json`

Expand Down