From 62d3afc416dd039d527de5cb89da552ff6b76a99 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Tue, 16 Jul 2024 12:43:30 +0000 Subject: [PATCH 01/14] =?UTF-8?q?=F0=9F=94=A8=20(grapher)=20let=20charts?= =?UTF-8?q?=20inherit=20defaults?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/ChartEditor.ts | 28 +- adminSiteClient/ChartEditorPage.tsx | 8 +- adminSiteClient/EditorHistoryTab.tsx | 11 +- adminSiteClient/GrapherConfigGridEditor.tsx | 1 + adminSiteServer/apiRouter.ts | 209 +++++++---- baker/GrapherBaker.tsx | 1 + baker/siteRenderers.tsx | 1 + ...1720600092980-MakeChartsInheritDefaults.ts | 59 +++ db/model/Variable.ts | 1 + .../generate-default-object-from-schema.ts | 22 +- devTools/svgTester/dump-data.ts | 10 +- explorer/GrapherGrammar.ts | 3 +- .../grapher/src/core/Grapher.tsx | 3 + packages/@ourworldindata/grapher/src/index.ts | 1 + .../grapher/src/schema/.gitignore | 1 + .../grapher/src/schema/README.md | 14 + .../src/schema/defaultGrapherConfig.ts | 81 ++++ .../src/schema/grapher-schema.004.yaml | 13 +- .../types/src/grapherTypes/GrapherTypes.ts | 13 +- packages/@ourworldindata/types/src/index.ts | 1 + .../@ourworldindata/utils/src/TimeBounds.ts | 6 +- packages/@ourworldindata/utils/src/Util.ts | 47 ++- .../utils/src/grapherConfigUtils.test.ts | 348 ++++++++++++++++++ .../utils/src/grapherConfigUtils.ts | 76 ++++ packages/@ourworldindata/utils/src/index.ts | 5 + site/DataPageV2.tsx | 11 +- site/ExplorerPage.tsx | 14 +- site/GrapherPage.tsx | 7 +- site/multiembedder/MultiEmbedder.tsx | 22 +- 29 files changed, 910 insertions(+), 107 deletions(-) create mode 100644 db/migration/1720600092980-MakeChartsInheritDefaults.ts create mode 100644 packages/@ourworldindata/grapher/src/schema/.gitignore create mode 100644 packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts create mode 100644 packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts create mode 100644 packages/@ourworldindata/utils/src/grapherConfigUtils.ts diff --git a/adminSiteClient/ChartEditor.ts b/adminSiteClient/ChartEditor.ts index d13047ff5e0..a566e50a7dd 100644 --- a/adminSiteClient/ChartEditor.ts +++ b/adminSiteClient/ChartEditor.ts @@ -14,6 +14,10 @@ import { ChartRedirect, DimensionProperty, Json, + GrapherInterface, + diffGrapherConfigs, + isEqual, + omit, } from "@ourworldindata/utils" import { computed, observable, runInAction, when } from "mobx" import { BAKED_GRAPHER_URL } from "../settings/clientSettings.js" @@ -97,6 +101,7 @@ export interface ChartEditorManager { admin: Admin grapher: Grapher database: EditorDatabase + parentGrapherConfig: GrapherInterface logs: Log[] references: References | undefined redirects: ChartRedirect[] @@ -124,7 +129,7 @@ export class ChartEditor { @observable.ref errorMessage?: { title: string; content: string } @observable.ref previewMode: "mobile" | "desktop" @observable.ref showStaticPreview = false - @observable.ref savedGrapherJson: string = "" + @observable.ref savedPatchConfig: GrapherInterface = {} // This gets set when we save a new chart for the first time // so the page knows to update the url @@ -138,12 +143,25 @@ export class ChartEditor { : "desktop" when( () => this.grapher.isReady, - () => (this.savedGrapherJson = JSON.stringify(this.grapher.object)) + () => (this.savedPatchConfig = this.patchConfig) ) } + @computed get fullConfig(): GrapherInterface { + return this.grapher.object + } + + @computed get patchConfig(): GrapherInterface { + const { parentGrapherConfig } = this.manager + if (!parentGrapherConfig) return this.fullConfig + return diffGrapherConfigs(this.fullConfig, parentGrapherConfig) + } + @computed get isModified(): boolean { - return JSON.stringify(this.grapher.object) !== this.savedGrapherJson + return !isEqual( + omit(this.patchConfig, "version"), + omit(this.savedPatchConfig, "version") + ) } @computed get grapher() { @@ -238,12 +256,12 @@ export class ChartEditor { if (isNewGrapher) { this.newChartId = json.chartId this.grapher.id = json.chartId - this.savedGrapherJson = JSON.stringify(this.grapher.object) + this.savedPatchConfig = json.savedPatch } else { runInAction(() => { grapher.version += 1 this.logs.unshift(json.newLog) - this.savedGrapherJson = JSON.stringify(currentGrapherObject) + this.savedPatchConfig = json.savedPatch }) } } else onError?.() diff --git a/adminSiteClient/ChartEditorPage.tsx b/adminSiteClient/ChartEditorPage.tsx index e1d1a49fc82..b16eeac4229 100644 --- a/adminSiteClient/ChartEditorPage.tsx +++ b/adminSiteClient/ChartEditorPage.tsx @@ -28,7 +28,7 @@ import { ChartRedirect, DimensionProperty, } from "@ourworldindata/types" -import { Grapher } from "@ourworldindata/grapher" +import { defaultGrapherConfig, Grapher } from "@ourworldindata/grapher" import { Admin } from "./Admin.js" import { ChartEditor, @@ -104,7 +104,7 @@ export class ChartEditorPage extends React.Component<{ grapherId?: number newGrapherIndex?: number - grapherConfig?: any + grapherConfig?: GrapherInterface }> implements ChartEditorManager { @@ -124,7 +124,9 @@ export class ChartEditorPage @observable simulateVisionDeficiency?: VisionDeficiency - fetchedGrapherConfig?: any + fetchedGrapherConfig?: GrapherInterface + // for now, every chart's parent config is the default layer + parentGrapherConfig = defaultGrapherConfig async fetchGrapher(): Promise { const { grapherId } = this.props diff --git a/adminSiteClient/EditorHistoryTab.tsx b/adminSiteClient/EditorHistoryTab.tsx index 0fa832b4acf..4c0e8baf5f8 100644 --- a/adminSiteClient/EditorHistoryTab.tsx +++ b/adminSiteClient/EditorHistoryTab.tsx @@ -129,7 +129,7 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> { @action.bound copyYamlToClipboard() { // Use the Clipboard API to copy the config into the users clipboard const chartConfigObject = { - ...this.props.editor.grapher.object, + ...this.props.editor.patchConfig, } delete chartConfigObject.id delete chartConfigObject.dimensions @@ -149,7 +149,7 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> { // Avoid modifying the original JSON object // Due to mobx memoizing computed values, the JSON can be mutated. const chartConfigObject = { - ...this.props.editor.grapher.object, + ...this.props.editor.patchConfig, } return (
@@ -157,7 +157,12 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> {
diff --git a/adminSiteClient/GrapherConfigGridEditor.tsx b/adminSiteClient/GrapherConfigGridEditor.tsx index d66fc72ddd1..ae6eb349504 100644 --- a/adminSiteClient/GrapherConfigGridEditor.tsx +++ b/adminSiteClient/GrapherConfigGridEditor.tsx @@ -347,6 +347,7 @@ export class GrapherConfigGridEditor extends React.Component => { + // if the schema version is missing, assume it's the latest + if (!config["$schema"]) { + config["$schema"] = defaultGrapherConfig["$schema"] + } + + // compute patch and full configs + const parentConfig = defaultGrapherConfig + const patchConfig = diffGrapherConfigs(config, parentConfig) + const fullConfig = mergeGrapherConfigs(parentConfig, patchConfig) + + // insert patch & full configs into the chart_configs table + const configId = uuidv7() + await db.knexRaw( + knex, + `-- sql + INSERT INTO chart_configs (id, patch, full) + VALUES (?, ?, ?) + `, + [configId, JSON.stringify(patchConfig), JSON.stringify(fullConfig)] + ) + + // add a new chart to the charts table + const result = await db.knexRawInsert( + knex, + `-- sql + INSERT INTO charts (configId, lastEditedAt, lastEditedByUserId) + VALUES (?, ?, ?) + `, + [configId, new Date(), user.id] + ) + + // The chart config itself has an id field that should store the id of the chart - update the chart now so this is true + const chartId = result.insertId + patchConfig.id = chartId + fullConfig.id = chartId + await db.knexRaw( + knex, + `-- sql + UPDATE chart_configs cc + JOIN charts c ON c.configId = cc.id + SET + cc.patch=JSON_SET(cc.patch, '$.id', ?), + cc.full=JSON_SET(cc.full, '$.id', ?) + WHERE c.id = ? + `, + [chartId, chartId, chartId] + ) + + return { patchConfig, fullConfig } +} + +const updateExistingChart = async ( + knex: db.KnexReadWriteTransaction, + { + config, + user, + chartId, + }: { config: GrapherInterface; user: DbPlainUser; chartId: number } +): Promise<{ patchConfig: GrapherInterface; fullConfig: GrapherInterface }> => { + // make sure that the id of the incoming config matches the chart id + config.id = chartId + + // if the schema version is missing, assume it's the latest + if (!config["$schema"]) { + config["$schema"] = defaultGrapherConfig["$schema"] + } + + // compute patch and full configs + const parentConfig = defaultGrapherConfig + const patchConfig = diffGrapherConfigs(config, parentConfig) + const fullConfig = mergeGrapherConfigs(parentConfig, patchConfig) + + // update configs + await db.knexRaw( + knex, + `-- sql + UPDATE chart_configs cc + JOIN charts c ON c.configId = cc.id + SET + cc.patch=?, + cc.full=? + WHERE c.id = ? + `, + [JSON.stringify(patchConfig), JSON.stringify(fullConfig), chartId] + ) + + // update charts row + await db.knexRaw( + knex, + `-- sql + UPDATE charts + SET lastEditedAt=?, lastEditedByUserId=? + WHERE id = ? + `, + [new Date(), user.id, chartId] + ) + + return { + patchConfig, + fullConfig, + } +} + const saveGrapher = async ( knex: db.KnexReadWriteTransaction, user: DbPlainUser, @@ -349,72 +459,32 @@ const saveGrapher = async ( else newConfig.version = 1 // Execute the actual database update or creation - const now = new Date() - let chartId = existingConfig && existingConfig.id - const newJsonConfig = JSON.stringify(newConfig) + let chartId: number + let newFullConfig: GrapherInterface if (existingConfig) { - await db.knexRaw( - knex, - `-- sql - UPDATE chart_configs cc - JOIN charts c ON c.configId = cc.id - SET - cc.patch=?, - cc.full=? - WHERE c.id = ? - `, - [newJsonConfig, newJsonConfig, chartId] - ) - await db.knexRaw( - knex, - `-- sql - UPDATE charts - SET lastEditedAt=?, lastEditedByUserId=? - WHERE id = ? - `, - [now, user.id, chartId] - ) + chartId = existingConfig.id! + const newConfigPair = await updateExistingChart(knex, { + config: newConfig, + user, + chartId, + }) + newConfig = newConfigPair.patchConfig + newFullConfig = newConfigPair.fullConfig } else { - const configId = uuidv7() - await db.knexRaw( - knex, - `-- sql - INSERT INTO chart_configs (id, patch, full) - VALUES (?, ?, ?) - `, - [configId, newJsonConfig, newJsonConfig] - ) - const result = await db.knexRawInsert( - knex, - `-- sql - INSERT INTO charts (configId, lastEditedAt, lastEditedByUserId) - VALUES (?, ?, ?) - `, - [configId, now, user.id] - ) - chartId = result.insertId - // The chart config itself has an id field that should store the id of the chart - update the chart now so this is true - newConfig.id = chartId - await db.knexRaw( - knex, - `-- sql - UPDATE chart_configs cc - JOIN charts c ON c.configId = cc.id - SET - cc.patch=JSON_SET(cc.patch, '$.id', ?), - cc.full=JSON_SET(cc.full, '$.id', ?) - WHERE c.id = ? - `, - [chartId, chartId, chartId] - ) + const newConfigPair = await saveNewChart(knex, { + config: newConfig, + user, + }) + newConfig = newConfigPair.patchConfig + newFullConfig = newConfigPair.fullConfig + chartId = newConfig.id! } // Record this change in version history - const chartRevisionLog = { chartId: chartId as number, userId: user.id, - config: serializeChartConfig(newConfig), + config: serializeChartConfig(newFullConfig), createdAt: new Date(), updatedAt: new Date(), } satisfies DbInsertChartRevision @@ -461,7 +531,7 @@ const saveGrapher = async ( await db.knexRaw( knex, `UPDATE charts SET publishedAt=?, publishedByUserId=? WHERE id = ? `, - [now, user.id, chartId] + [new Date(), user.id, chartId] ) await triggerStaticBuild(user, `Publishing chart ${newConfig.slug}`) } else if ( @@ -479,7 +549,10 @@ const saveGrapher = async ( } else if (newConfig.isPublished) await triggerStaticBuild(user, `Updating chart ${newConfig.slug}`) - return chartId + return { + chartId, + savedPatch: newConfig, + } } getRouteWithROTransaction(apiRouter, "/charts.json", async (req, res, trx) => { @@ -756,7 +829,7 @@ apiRouter.get( ) postRouteWithRWTransaction(apiRouter, "/charts", async (req, res, trx) => { - const chartId = await saveGrapher(trx, res.locals.user, req.body) + const { chartId } = await saveGrapher(trx, res.locals.user, req.body) return { success: true, chartId: chartId } }) @@ -779,10 +852,20 @@ putRouteWithRWTransaction( async (req, res, trx) => { const existingConfig = await expectChartById(trx, req.params.chartId) - await saveGrapher(trx, res.locals.user, req.body, existingConfig) + const { chartId, savedPatch } = await saveGrapher( + trx, + res.locals.user, + req.body, + existingConfig + ) const logs = await getLogsByChartId(trx, existingConfig.id as number) - return { success: true, chartId: existingConfig.id, newLog: logs[0] } + return { + success: true, + chartId, + savedPatch, + newLog: logs[0], + } } ) diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index 8118f0c050d..f810f822d3c 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -142,6 +142,7 @@ export async function renderDataPageV2( // this is true for preview pages for datapages on the indicator level but false // if we are on Grapher pages. Once we have a good way in the grapher admin for how // to use indicator level defaults, we should reconsider how this works here. + // TODO(inheritance): use mergeGrapherConfigs instead const grapher = useIndicatorGrapherConfigs ? mergePartialGrapherConfigs(grapherConfigForVariable, pageGrapher) : pageGrapher ?? {} diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index 12a763c3c07..3a4ec6635bf 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -801,6 +801,7 @@ export const renderExplorerPage = async ( config: row.grapherConfigETL as string, }) : {} + // TODO(inheritance): use mergeGrapherConfigs instead return mergePartialGrapherConfigs(etlConfig, adminConfig) }) diff --git a/db/migration/1720600092980-MakeChartsInheritDefaults.ts b/db/migration/1720600092980-MakeChartsInheritDefaults.ts new file mode 100644 index 00000000000..93edf367602 --- /dev/null +++ b/db/migration/1720600092980-MakeChartsInheritDefaults.ts @@ -0,0 +1,59 @@ +import { MigrationInterface, QueryRunner } from "typeorm" +import { diffGrapherConfigs, mergeGrapherConfigs } from "@ourworldindata/utils" +import { defaultGrapherConfig } from "@ourworldindata/grapher" + +export class MakeChartsInheritDefaults1720600092980 + implements MigrationInterface +{ + public async up(queryRunner: QueryRunner): Promise { + const charts = (await queryRunner.query( + `-- sql + SELECT id, patch as config FROM chart_configs + ` + )) as { id: string; config: string }[] + + for (const chart of charts) { + const originalConfig = JSON.parse(chart.config) + + // if the schema version is missing, assume it's the latest + if (!originalConfig["$schema"]) { + originalConfig["$schema"] = defaultGrapherConfig["$schema"] + } + + const patchConfig = diffGrapherConfigs( + originalConfig, + defaultGrapherConfig + ) + const fullConfig = mergeGrapherConfigs( + defaultGrapherConfig, + patchConfig + ) + + await queryRunner.query( + `-- sql + UPDATE chart_configs + SET + patch = ?, + full = ? + WHERE id = ? + `, + [ + JSON.stringify(patchConfig), + JSON.stringify(fullConfig), + chart.id, + ] + ) + } + } + + public async down(queryRunner: QueryRunner): Promise { + // we can't recover the original configs, + // but the patched one is the next best thing + await queryRunner.query( + `-- sql + UPDATE chart_configs + SET full = patch + ` + ) + } +} diff --git a/db/model/Variable.ts b/db/model/Variable.ts index af2588be4b7..8bcd74abe0e 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -45,6 +45,7 @@ export async function getMergedGrapherConfigForVariable( const grapherConfigETL = row.grapherConfigETL ? JSON.parse(row.grapherConfigETL) : undefined + // TODO(inheritance): use mergeGrapherConfigs instead return _.merge({}, grapherConfigAdmin, grapherConfigETL) } diff --git a/devTools/schema/generate-default-object-from-schema.ts b/devTools/schema/generate-default-object-from-schema.ts index b831c70b853..3ef1f66ca00 100644 --- a/devTools/schema/generate-default-object-from-schema.ts +++ b/devTools/schema/generate-default-object-from-schema.ts @@ -44,15 +44,31 @@ async function main(parsedArgs: parseArgs.ParsedArgs) { let schema = fs.readJSONSync(schemaFilename) const defs = schema.$defs || {} - const defaultObject = generateDefaultObjectFromSchema(schema, defs) - process.stdout.write(JSON.stringify(defaultObject, undefined, 2)) + const defaultConfig = generateDefaultObjectFromSchema(schema, defs) + const defaultConfigJSON = JSON.stringify(defaultConfig, undefined, 2) + + // save as ts file if requested + if (parsedArgs["save-ts"]) { + const out = parsedArgs["save-ts"] + const content = `// THIS IS A GENERATED FILE, DO NOT EDIT DIRECTLY + +// GENERATED BY devTools/schema/generate-default-object-from-schema.ts + +import { GrapherInterface } from "@ourworldindata/types" + +export const defaultGrapherConfig = ${defaultConfigJSON} as GrapherInterface` + fs.outputFileSync(out, content) + } + + // write json to stdout + process.stdout.write(defaultConfigJSON) } function help() { console.log(`generate-default-object-from-schema.ts - utility to generate an object with all default values that are given in a JSON schema Usage: - generate-default-object-from-schema.js `) + generate-default-object-from-schema.js --save-ts `) } const parsedArgs = parseArgs(process.argv.slice(2)) diff --git a/devTools/svgTester/dump-data.ts b/devTools/svgTester/dump-data.ts index 0e08ec6983e..f0d17b5c8d6 100644 --- a/devTools/svgTester/dump-data.ts +++ b/devTools/svgTester/dump-data.ts @@ -1,6 +1,8 @@ #! /usr/bin/env node import { getPublishedGraphersBySlug } from "../../baker/GrapherImageBaker.js" +import { defaultGrapherConfig } from "@ourworldindata/grapher" +import { diffGrapherConfigs } from "@ourworldindata/utils" import { TransactionCloseMode, knexReadonlyTransaction } from "../../db/db.js" @@ -23,7 +25,13 @@ async function main(parsedArgs: parseArgs.ParsedArgs) { ) const allGraphers = [...graphersBySlug.values()] const saveJobs: utils.SaveGrapherSchemaAndDataJob[] = allGraphers.map( - (grapher) => ({ config: grapher, outDir }) + (grapher) => { + // since we're not baking defaults, we also exlcude them here + return { + config: diffGrapherConfigs(grapher, defaultGrapherConfig), + outDir, + } + } ) await pMap(saveJobs, utils.saveGrapherSchemaAndData, { diff --git a/explorer/GrapherGrammar.ts b/explorer/GrapherGrammar.ts index 28bcac3521c..ed67b32c649 100644 --- a/explorer/GrapherGrammar.ts +++ b/explorer/GrapherGrammar.ts @@ -224,8 +224,7 @@ export const GrapherGrammar: Grammar = { hideRelativeToggle: { ...BooleanCellDef, keyword: "hideRelativeToggle", - description: - "Whether to hide the relative mode UI toggle. Default depends on the chart type.", + description: "Whether to hide the relative mode UI toggle", }, timelineMinTime: { ...IntegerCellDef, diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index ed0b5d920ee..404f7eaa754 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -1007,6 +1007,9 @@ export class Grapher @computed get whatAreWeWaitingFor(): string { const { newSlugs, inputTable, dimensions } = this + if (dimensions.length === 0 && newSlugs.length === 0) { + return `Waiting for dimensions to be set. ${inputTable.tableDescription}` + } if (newSlugs.length || dimensions.length === 0) { const missingColumns = newSlugs.filter( (slug) => !inputTable.has(slug) diff --git a/packages/@ourworldindata/grapher/src/index.ts b/packages/@ourworldindata/grapher/src/index.ts index d28ffab95d9..9e9d08b9902 100644 --- a/packages/@ourworldindata/grapher/src/index.ts +++ b/packages/@ourworldindata/grapher/src/index.ts @@ -79,3 +79,4 @@ export { type SlideShowManager, SlideShowController, } from "./slideshowController/SlideShowController" +export { defaultGrapherConfig } from "./schema/defaultGrapherConfig" diff --git a/packages/@ourworldindata/grapher/src/schema/.gitignore b/packages/@ourworldindata/grapher/src/schema/.gitignore new file mode 100644 index 00000000000..b1c3b6f48ad --- /dev/null +++ b/packages/@ourworldindata/grapher/src/schema/.gitignore @@ -0,0 +1 @@ +grapher-schema.*.json diff --git a/packages/@ourworldindata/grapher/src/schema/README.md b/packages/@ourworldindata/grapher/src/schema/README.md index 19a5afc067b..76baa75ef77 100644 --- a/packages/@ourworldindata/grapher/src/schema/README.md +++ b/packages/@ourworldindata/grapher/src/schema/README.md @@ -6,6 +6,8 @@ The schema is versioned. There is one yaml file with a version number. For nonbr edit the yaml file as is. A github action will then generate a .latest.yaml and two json files (one .latest.json and one with the version number.json) and upload them to S3 so they can be accessed publicly. +If you update the default value of an existing property or you add a new property with a default value, make sure to regenerate the default object from the schema and save it to `defaultGrapherConfig.ts` (see below). You should also write a migration to update the `chart_configs.full` column in the database for all stand-alone charts. + Breaking changes should be done by renaming the schema file to an increased version number. Make sure to also rename the authorative url inside the schema file (the "$id" field at the top level) to point to the new version number json. Then write the migrations from the last to the current version of the schema, including the migration of pointing to the URL of the new schema version. Also update `DEFAULT_GRAPHER_CONFIG_SCHEMA` in `GrapherConstants.ts` to point to the new schema version number url. @@ -16,3 +18,15 @@ Checklist for breaking changes: - Rename the authorative url inside the schema file to point to the new version number json - Write the migrations from the last to the current version of the schema, including the migration of pointing to the URL of the new schema version - Update `DEFAULT_GRAPHER_CONFIG_SCHEMA` in `GrapherConstants.ts` to point to the new schema version number url +- Regenerate the default object from the schema and save it to `defaultGrapherConfig.ts` (see below) +- Write a migration to update the `chart_configs.full` column in the database for all stand-alone charts + +To regenerate `defaultGrapherConfig.ts` from the schema, replace `XXX` with the current schema version number and run: + +```bash +# generate json from the yaml schema +nu -c 'open packages/@ourworldindata/grapher/src/schema/grapher-schema.XXX.yaml | to json' > packages/@ourworldindata/grapher/src/schema/grapher-schema.XXX.json + +# generate the default object from the schema +node itsJustJavascript/devTools/schema/generate-default-object-from-schema.js packages/@ourworldindata/grapher/src/schema/grapher-schema.XXX.json --save-ts packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts +``` diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts new file mode 100644 index 00000000000..dbbb82d9d5c --- /dev/null +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -0,0 +1,81 @@ +// THIS IS A GENERATED FILE, DO NOT EDIT DIRECTLY + +// GENERATED BY devTools/schema/generate-default-object-from-schema.ts + +import { GrapherInterface } from "@ourworldindata/types" + +export const defaultGrapherConfig = { + $schema: "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + map: { + projection: "World", + hideTimeline: false, + colorScale: { + equalSizeBins: true, + binningStrategy: "ckmeans", + customNumericColorsActive: false, + colorSchemeInvert: false, + binningStrategyBinCount: 5, + }, + timeTolerance: 0, + toleranceStrategy: "closest", + tooltipUseCustomLabels: false, + time: "latest", + }, + maxTime: "latest", + yAxis: { + removePointsOutsideDomain: false, + scaleType: "linear", + canChangeScaleType: false, + facetDomain: "shared", + }, + tab: "chart", + matchingEntitiesOnly: false, + hasChartTab: true, + hideLegend: false, + hideLogo: false, + hideTimeline: false, + colorScale: { + equalSizeBins: true, + binningStrategy: "ckmeans", + customNumericColorsActive: false, + colorSchemeInvert: false, + binningStrategyBinCount: 5, + }, + scatterPointLabelStrategy: "year", + selectedFacetStrategy: "none", + isPublished: false, + invertColorScheme: false, + hideRelativeToggle: true, + logo: "owid", + entityType: "country or region", + facettingLabelByYVariables: "metric", + addCountryMode: "add-country", + compareEndPointsOnly: false, + type: "LineChart", + hasMapTab: false, + stackMode: "absolute", + minTime: "earliest", + hideAnnotationFieldsInTitle: { + entity: false, + time: false, + changeInPrefix: false, + }, + xAxis: { + removePointsOutsideDomain: false, + scaleType: "linear", + canChangeScaleType: false, + facetDomain: "shared", + }, + hideConnectedScatterLines: false, + showNoDataArea: true, + zoomToSelection: false, + showYearLabels: false, + hideLinesOutsideTolerance: false, + hideTotalValueLabel: false, + hideScatterLabels: false, + sortBy: "total", + sortOrder: "desc", + hideFacetControl: true, + entityTypePlural: "countries", + missingDataStrategy: "auto", +} as GrapherInterface diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml index 18deb0f59b6..055ce130ead 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml @@ -57,7 +57,6 @@ $defs: baseColorScheme: type: string description: One of the predefined base color schemes - default: default enum: - YlGn - YlGnBu @@ -258,7 +257,9 @@ properties: - earliest columnSlug: # TODO: remove this once we have a convention of using the first y dimension instead - description: "Column to show in the map tab. Can be a column slug (e.g. in explorers) or a variable ID (as string)." + description: | + Column to show in the map tab. Can be a column slug (e.g. in explorers) or a variable ID (as string). + If not provided, the first y dimension is used. type: string additionalProperties: false maxTime: @@ -281,7 +282,6 @@ properties: - string baseColorScheme: type: string - default: default description: The default color scheme if no color overrides are specified yAxis: $ref: "#/$defs/axis" @@ -356,7 +356,8 @@ properties: description: Reverse the order of colors in the color scheme hideRelativeToggle: type: boolean - description: Whether to hide the relative mode UI toggle. Default depends on the chart type + default: true + description: Whether to hide the relative mode UI toggle comparisonLines: description: List of vertical comparison lines to draw type: array @@ -375,7 +376,6 @@ properties: type: string version: type: integer - default: 1 minimum: 0 logo: type: string @@ -490,13 +490,16 @@ properties: type: integer description: Number of decimal places to show minimum: 0 + default: 2 numSignificantFigures: type: integer description: Number of significant figures to show minimum: 1 + default: 3 zeroDay: type: string description: Iso date day string for the starting date if yearIsDay is used + default: "2020-01-21" additionalProperties: false variableId: type: integer diff --git a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts index 0d067710c0d..a24459de6b4 100644 --- a/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts +++ b/packages/@ourworldindata/types/src/grapherTypes/GrapherTypes.ts @@ -106,6 +106,11 @@ export enum TimeBoundValue { positiveInfinity = Infinity, } +export enum TimeBoundValueStr { + unboundedLeft = "earliest", + unboundedRight = "latest", +} + /** * Time tolerance strategy used for maps */ @@ -509,12 +514,12 @@ export enum MapProjectionName { export interface MapConfigInterface { columnSlug?: ColumnSlug - time?: Time + time?: Time | TimeBoundValueStr timeTolerance?: number toleranceStrategy?: ToleranceStrategy hideTimeline?: boolean projection?: MapProjectionName - colorScale?: ColorScaleConfigInterface + colorScale?: Partial tooltipUseCustomLabels?: boolean } @@ -532,8 +537,8 @@ export interface GrapherInterface extends SortConfig { sourceDesc?: string note?: string hideAnnotationFieldsInTitle?: AnnotationFieldsInTitle - minTime?: TimeBound - maxTime?: TimeBound + minTime?: TimeBound | TimeBoundValueStr + maxTime?: TimeBound | TimeBoundValueStr timelineMinTime?: Time timelineMaxTime?: Time dimensions?: OwidChartDimensionInterface[] diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index 895ae54e69b..87b227f14bc 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -53,6 +53,7 @@ export { type ValueRange, type Year, TimeBoundValue, + TimeBoundValueStr, type TimeRange, type Color, type ColumnSlug, diff --git a/packages/@ourworldindata/utils/src/TimeBounds.ts b/packages/@ourworldindata/utils/src/TimeBounds.ts index 736fc5c4009..08a46390a5b 100644 --- a/packages/@ourworldindata/utils/src/TimeBounds.ts +++ b/packages/@ourworldindata/utils/src/TimeBounds.ts @@ -3,6 +3,7 @@ import { Time, TimeBound, TimeBoundValue, + TimeBoundValueStr, } from "@ourworldindata/types" import { parseIntOrUndefined, @@ -13,11 +14,6 @@ import { isPositiveInfinity, } from "./Util.js" -enum TimeBoundValueStr { - unboundedLeft = "earliest", - unboundedRight = "latest", -} - export const timeFromTimebounds = ( timeBound: TimeBound, minTime: Time, diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts index 9dfcc41c96c..18d0d7ffaf5 100644 --- a/packages/@ourworldindata/utils/src/Util.ts +++ b/packages/@ourworldindata/utils/src/Util.ts @@ -35,6 +35,7 @@ import { maxBy, memoize, merge, + mergeWith, min, minBy, noop, @@ -95,6 +96,7 @@ export { isNil, isNull, isNumber, + isPlainObject, isString, isUndefined, keyBy, @@ -103,6 +105,7 @@ export { maxBy, memoize, merge, + mergeWith, min, minBy, noop, @@ -1113,6 +1116,32 @@ export const omitNullableValues = (object: T): NoUndefinedValues => { return result } +/** + * Omits undefined values and empty objects recursively. + */ +export function omitUndefinedValuesRecursive>( + obj: T +): NoUndefinedValues { + const result: any = {} + for (const key in obj) { + const isEmptyObject = + isPlainObject(obj[key]) && isEmpty(omitUndefinedValues(obj[key])) + const isNonEmptyObject = + isPlainObject(obj[key]) && !isEmpty(omitUndefinedValues(obj[key])) + + if (isNonEmptyObject) { + // re-apply the function if we encounter a non-empty object + result[key] = omitUndefinedValuesRecursive(obj[key]) + } else if (obj[key] === undefined || isEmptyObject) { + // omit undefined values and empty objects + } else { + // otherwise, keep the value + result[key] = obj[key] + } + } + return result +} + export const isInIFrame = (): boolean => { try { return window.self !== window.top @@ -1739,7 +1768,7 @@ export function filterValidStringValues( return filteredValues } -// TODO: type this correctly once we have moved types into their own top level package +// TODO(inheritance): remove in favour of mergeGrapherConfigs export function mergePartialGrapherConfigs>( ...grapherConfigs: (T | undefined)[] ): T { @@ -1917,3 +1946,19 @@ export function lazy(fn: () => T): () => T { return _value } } + +export function traverseObjects>( + obj: T, + ref: Record, + cb: (objValue: unknown, refValue: unknown, key: string) => unknown +): Partial { + const result: any = {} + for (const key in obj) { + if (isPlainObject(obj[key]) && isPlainObject(ref[key])) { + result[key] = traverseObjects(obj[key], ref[key], cb) + } else { + result[key] = cb(obj[key], ref[key], key) + } + } + return result +} diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts new file mode 100644 index 00000000000..2bff9918515 --- /dev/null +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts @@ -0,0 +1,348 @@ +#! /usr/bin/env jest + +import { + GrapherInterface, + GrapherTabOption, + MapProjectionName, +} from "@ourworldindata/types" +import { + mergeGrapherConfigs, + diffGrapherConfigs, +} from "./grapherConfigUtils.js" + +describe(mergeGrapherConfigs, () => { + it("merges empty configs", () => { + expect(mergeGrapherConfigs({}, {})).toEqual({}) + expect(mergeGrapherConfigs({ title: "Parent title" }, {})).toEqual({ + title: "Parent title", + }) + expect(mergeGrapherConfigs({}, { title: "Child title" })).toEqual({ + title: "Child title", + }) + }) + + it("doesn't mutate input objects", () => { + const parentConfig = { title: "Title" } + const childConfig = { subtitle: "Subtitle" } + mergeGrapherConfigs(parentConfig, childConfig) + expect(parentConfig).toEqual({ title: "Title" }) + expect(childConfig).toEqual({ subtitle: "Subtitle" }) + }) + + it("merges two objects", () => { + expect( + mergeGrapherConfigs( + { title: "Parent title" }, + { subtitle: "Child subtitle" } + ) + ).toEqual({ + title: "Parent title", + subtitle: "Child subtitle", + }) + expect( + mergeGrapherConfigs( + { title: "Parent title" }, + { title: "Child title" } + ) + ).toEqual({ title: "Child title" }) + expect( + mergeGrapherConfigs( + { title: "Parent title", subtitle: "Parent subtitle" }, + { title: "Child title", hideRelativeToggle: true } + ) + ).toEqual({ + title: "Child title", + subtitle: "Parent subtitle", + hideRelativeToggle: true, + }) + }) + + it("merges three objects", () => { + expect( + mergeGrapherConfigs( + { title: "Parent title" }, + { subtitle: "Child subtitle" }, + { note: "Grandchild note" } + ) + ).toEqual({ + title: "Parent title", + subtitle: "Child subtitle", + note: "Grandchild note", + }) + expect( + mergeGrapherConfigs( + { + title: "Parent title", + subtitle: "Parent subtitle", + sourceDesc: "Parent sources", + }, + { title: "Child title", subtitle: "Child subtitle" }, + { title: "Grandchild title", note: "Grandchild note" } + ) + ).toEqual({ + title: "Grandchild title", + subtitle: "Child subtitle", + note: "Grandchild note", + sourceDesc: "Parent sources", + }) + }) + + it("merges nested objects", () => { + expect( + mergeGrapherConfigs( + { + map: { + projection: MapProjectionName.World, + time: 2000, + }, + }, + { + map: { + projection: MapProjectionName.Africa, + hideTimeline: true, + }, + } + ) + ).toEqual({ + map: { + projection: MapProjectionName.Africa, + time: 2000, + hideTimeline: true, + }, + }) + }) + + it("overwrites arrays", () => { + expect( + mergeGrapherConfigs( + { selectedEntityNames: ["France", "Italy"] }, + { selectedEntityNames: ["Italy", "Spain"] } + ) + ).toEqual({ + selectedEntityNames: ["Italy", "Spain"], + }) + expect( + mergeGrapherConfigs( + { colorScale: { customNumericValues: [1, 2] } }, + { colorScale: { customNumericValues: [3, 4] } } + ) + ).toEqual({ + colorScale: { customNumericValues: [3, 4] }, + }) + }) + + it("doesn't merge configs of different schema versions", () => { + expect(() => + mergeGrapherConfigs( + { $schema: "1", title: "Title A" }, + { $schema: "2", title: "Title B" } + ) + ).toThrowError() + }) + + it("excludes $schema, id, slug, version and isPublished from inheritance", () => { + expect( + mergeGrapherConfigs( + { + $schema: "004", + id: 1, + slug: "parent-slug", + version: 1, + title: "Title A", + }, + { title: "Title B" } + ) + ).toEqual({ title: "Title B" }) + expect( + mergeGrapherConfigs( + { + $schema: "004", + id: 1, + slug: "parent-slug", + version: 1, + title: "Title A", + }, + { slug: "child-slug", version: 1, title: "Title B" } + ) + ).toEqual({ slug: "child-slug", version: 1, title: "Title B" }) + }) + + it("overwrites values with an empty string if requested", () => { + expect( + mergeGrapherConfigs( + { title: "Parent title", subtitle: "Parent subtitle" }, + { subtitle: "" } + ) + ).toEqual({ title: "Parent title", subtitle: "" }) + }) + + it("is associative", () => { + const configA: GrapherInterface = { + title: "Title A", + subtitle: "Subtitle A", + } + const configB: GrapherInterface = { title: "Title B", note: "Note B" } + const configC: GrapherInterface = { + title: "Title C", + subtitle: "Subtitle C", + sourceDesc: "Source C", + } + expect( + mergeGrapherConfigs(configA, mergeGrapherConfigs(configB, configC)) + ).toEqual( + mergeGrapherConfigs(mergeGrapherConfigs(configA, configB), configC) + ) + expect( + mergeGrapherConfigs(mergeGrapherConfigs(configA, configB), configC) + ).toEqual(mergeGrapherConfigs(configA, configB, configC)) + }) +}) + +describe(diffGrapherConfigs, () => { + it("returns the given config if the reference is empty", () => { + expect(diffGrapherConfigs({ title: "Chart" }, {})).toEqual({ + title: "Chart", + }) + }) + + it("returns the given config if it's empty", () => { + expect(diffGrapherConfigs({}, { title: "Reference chart" })).toEqual({}) + }) + + it("drops redundant entries", () => { + expect( + diffGrapherConfigs( + { tab: GrapherTabOption.map }, + { tab: GrapherTabOption.map } + ) + ).toEqual({}) + expect( + diffGrapherConfigs( + { tab: GrapherTabOption.chart, title: "Chart" }, + { tab: GrapherTabOption.chart, title: "Reference chart" } + ) + ).toEqual({ title: "Chart" }) + }) + + it("diffs nested configs correctly", () => { + expect( + diffGrapherConfigs( + { + title: "Chart", + tab: GrapherTabOption.chart, + map: { + projection: MapProjectionName.World, + hideTimeline: true, + }, + }, + { + title: "Reference chart", + tab: GrapherTabOption.chart, + map: { + projection: MapProjectionName.World, + hideTimeline: false, + }, + } + ) + ).toEqual({ title: "Chart", map: { hideTimeline: true } }) + expect( + diffGrapherConfigs( + { + tab: GrapherTabOption.chart, + map: { + projection: MapProjectionName.World, + hideTimeline: true, + }, + }, + { + tab: GrapherTabOption.chart, + map: { + projection: MapProjectionName.World, + hideTimeline: true, + }, + } + ) + ).toEqual({}) + }) + + it("strips undefined values from the config", () => { + expect( + diffGrapherConfigs( + { + tab: GrapherTabOption.chart, + title: "Chart", + subtitle: undefined, + }, + { tab: GrapherTabOption.chart, title: "Reference chart" } + ) + ).toEqual({ title: "Chart" }) + }) + + it("doesn't diff $schema, id, version, slug or isPublished", () => { + expect( + diffGrapherConfigs( + { + title: "Chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + }, + { + title: "Reference chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + } + ) + ).toEqual({ + title: "Chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + }) + }) + + it("is idempotent", () => { + const config: GrapherInterface = { + tab: GrapherTabOption.chart, + title: "Chart", + subtitle: undefined, + } + const reference: GrapherInterface = { + tab: GrapherTabOption.chart, + title: "Reference chart", + } + const diffedOnce = diffGrapherConfigs(config, reference) + const diffedTwice = diffGrapherConfigs(diffedOnce, reference) + expect(diffedTwice).toEqual(diffedOnce) + }) +}) + +describe("diff+merge", () => { + it("are consistent", () => { + const config: GrapherInterface = { + tab: GrapherTabOption.chart, + title: "Chart", + subtitle: "Chart subtitle", + } + const reference: GrapherInterface = { + tab: GrapherTabOption.chart, + title: "Reference chart", + } + const diffedAndMerged = mergeGrapherConfigs( + reference, + diffGrapherConfigs(config, reference) + ) + const onlyMerged = mergeGrapherConfigs(reference, config) + expect(diffedAndMerged).toEqual(onlyMerged) + }) +}) diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts new file mode 100644 index 00000000000..e18511ec46d --- /dev/null +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts @@ -0,0 +1,76 @@ +import { GrapherInterface } from "@ourworldindata/types" +import { + isEqual, + mergeWith, + uniq, + omit, + excludeUndefined, + omitUndefinedValuesRecursive, + traverseObjects, +} from "./Util" + +const KEYS_EXCLUDED_FROM_INHERITANCE = [ + "$schema", + "id", + "slug", + "version", + "isPublished", +] + +export function mergeGrapherConfigs( + ...grapherConfigs: GrapherInterface[] +): GrapherInterface { + // warn if one of the configs is missing a schema version + const configsWithoutSchema = grapherConfigs.filter( + (c) => c["$schema"] === undefined + ) + if (configsWithoutSchema.length > 0) { + const ids = configsWithoutSchema.map((c) => c.id) + console.warn( + `About to merge Grapher configs with missing schema information. Charts with missing schema information: ${ids}` + ) + } + + // abort if the grapher configs have different schema versions + const uniqueSchemas = uniq( + excludeUndefined(grapherConfigs.map((c) => c["$schema"])) + ) + if (uniqueSchemas.length > 1) { + throw new Error( + `Can't merge Grapher configs with different schema versions: ${uniqueSchemas.join( + ", " + )}` + ) + } + + // keys that should not be inherited are removed from all but the last config + const cleanedConfigs = grapherConfigs.map((config, index) => { + if (index === grapherConfigs.length - 1) return config + return omit(config, KEYS_EXCLUDED_FROM_INHERITANCE) + }) + + return mergeWith( + {}, // mergeWith mutates the first argument + ...cleanedConfigs, + (_: unknown, childValue: unknown): any => { + // don't concat arrays, just use the last one + if (Array.isArray(childValue)) { + return childValue + } + } + ) +} + +export function diffGrapherConfigs( + config: GrapherInterface, + reference: GrapherInterface +): GrapherInterface { + return omitUndefinedValuesRecursive( + traverseObjects(config, reference, (value, refValue, key) => { + if (KEYS_EXCLUDED_FROM_INHERITANCE.includes(key)) return value + if (refValue === undefined) return value + if (!isEqual(value, refValue)) return value + else return undefined + }) + ) +} diff --git a/packages/@ourworldindata/utils/src/index.ts b/packages/@ourworldindata/utils/src/index.ts index 0a0f2413777..b68c5c87c28 100644 --- a/packages/@ourworldindata/utils/src/index.ts +++ b/packages/@ourworldindata/utils/src/index.ts @@ -331,6 +331,11 @@ export { export { isAndroid, isIOS } from "./BrowserUtils.js" +export { + diffGrapherConfigs, + mergeGrapherConfigs, +} from "./grapherConfigUtils.js" + export { MultiDimDataPageConfig, extractMultiDimChoicesFromQueryStr, diff --git a/site/DataPageV2.tsx b/site/DataPageV2.tsx index 2e032155f5d..34e4b05f5f6 100644 --- a/site/DataPageV2.tsx +++ b/site/DataPageV2.tsx @@ -1,4 +1,5 @@ import { + defaultGrapherConfig, getVariableDataRoute, getVariableMetadataRoute, GrapherProgrammaticInterface, @@ -15,6 +16,7 @@ import { GrapherInterface, ImageMetadata, Url, + diffGrapherConfigs, } from "@ourworldindata/utils" import { MarkdownTextWrap } from "@ourworldindata/components" import React from "react" @@ -86,6 +88,7 @@ export const DataPageV2 = (props: { compact(grapher?.dimensions?.map((d) => d.variableId)) ) + // TODO(inheritance): use mergeGrapherConfigs instead const mergedGrapherConfig = mergePartialGrapherConfigs( datapageData.chartConfig as GrapherInterface, grapher @@ -103,6 +106,12 @@ export const DataPageV2 = (props: { dataApiUrl: DATA_API_URL, } + // We bake the Grapher config without defaults + const grapherConfigToBake = diffGrapherConfigs( + grapherConfig, + defaultGrapherConfig + ) + // Only embed the tags that are actually used by the datapage, instead of the complete JSON object with ~240 properties const minimalTagToSlugMap = pick( tagToSlugMap, @@ -182,7 +191,7 @@ export const DataPageV2 = (props: {