From 3cc05db3694882fe8e84d4ed03aed836ec74ea5b Mon Sep 17 00:00:00 2001 From: Sophia Mersmann Date: Tue, 3 Sep 2024 15:01:25 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20(grapher)=20let=20chart=20config?= =?UTF-8?q?s=20inherit=20default=20values=20/=20TAS-565=20(#3782)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Implements chart config inheritance and makes all chart configs inherit values from a parent config with default values. ## Details - For all standalone charts, the `full` config is the `patch` config merged with the default layer - Whenever we save a new chart or update the existing one, we first compute the patch config by diffing against the parent/default config and then compute the full config by merging the patch with the parent - We usually operate on the full config, but we're baking the full config without defaults so that we're not baking the defaults again and again and again (makes a difference on explorer pages where we potentially bake tens/hundreds of configs) - We use the ~full~ patch config for version history. The way we currently do revisions doesn't always work[^1]. ~Using the full config for version history fixes this, but has the disadvantage that "old" default values (that have since been updated) are copied over when a config is restored.~ - The default grapher config is derived from the schema and lives in the codebase. It can be generated by running the `generate-default-object-from-schema.ts` with the `--save-ts` flag - Whenever Grapher defaults are updated, the default config should be regenerated, and all `full` configs of stand-alone charts should be recomputed - We could have also stored the default layer in the database, but it's convenient to have it at hand in the codebase (where it's also type-checked). If some external entity needs the default config, they can access https://files.ourworldindata.org/schemas/grapher-schema.default.latest.json - Two new functions operate on chart configs and implement inheritance: - `mergeGrapherConfigs`: merges two or more patch configs - Some keys are excluded from inheritance ($schema, id, slug, version, isPublished) - Configs with different schema versions can't be merged - `diffGrapherConfigs`: diffs a given config against some reference config, where only values that are different in both configs are kept ## Caveats - ~I left the chart revisions as is but now store the full config in the chart revisions table whenever a chart is saved – this could be a bit confusing~ - ~As I said above, storing the full config in version history comes at the cost of copying over old default values when a chart is restored – how big of a problem is that?~ ## SVG tester I dumped two new set of configs and reference charts for the SVG tester: - [Patch configs](https://github.com/owid/owid-grapher-svgs/compare/configs-2024-july...configs-july-2024-defaults?short_path=4814afd#diff-4814afdf72df1537e0f28ca29966dde09611474e3ed214d1609754491faa83e0) - The config changes are due to diffing against the default layer (meaning, default values are removed) - There is one chart that is rendered differently, but it seems to be unrelated to the changes in this PR. - [Full configs](https://github.com/owid/owid-grapher-svgs/compare/configs-2024-08-20...configs-2024-08-20-defaults-2) - Few charts are added and removed, probably because I refreshed the database in between - Other than that, no changes ## To do - ~Add some real-world tests~ it's ok as is - [x] Add a GitHub workflow that re-generates the default config and checks if the diff is clean - [x] Delete `DEFAULT_GRAPHER_CONFIG_SCHEMA` [^1]: When restoring a config from version history, we re-apply an old config version. But that doesn't take default values into account. For example, let's say the v1 version of a chart relies on the default value for `hasMapTab` and has thus no map tab. In v2, a map tab is added (the config now contains `{..., hasMapTab: true}`. Restoring v1 doesn't switch `hasMapTab` back to `false` since `hasMapTab` isn't contained in the v1 version of the config (since we were relying on the default). --- .../check-default-grapher-config.yml | 54 +++ adminSiteClient/GrapherConfigGridEditor.tsx | 1 + adminSiteServer/apiRouter.ts | 209 ++++++--- baker/GrapherBaker.tsx | 1 + baker/siteRenderers.tsx | 1 + ...1720600092980-MakeChartsInheritDefaults.ts | 64 +++ db/model/Variable.ts | 1 + .../generate-default-object-from-schema.ts | 22 +- devTools/schemaProcessor/columns.json | 17 +- devTools/svgTester/dump-data.ts | 10 +- explorer/GrapherGrammar.ts | 3 +- .../grapher/src/core/Grapher.jsdom.test.ts | 8 +- .../grapher/src/core/Grapher.tsx | 10 +- .../grapher/src/core/GrapherConstants.ts | 3 - packages/@ourworldindata/grapher/src/index.ts | 1 + .../grapher/src/schema/.gitignore | 1 + .../grapher/src/schema/README.md | 17 +- .../src/schema/defaultGrapherConfig.ts | 79 ++++ .../src/schema/grapher-schema.004.yaml | 40 +- .../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 | 54 ++- .../utils/src/grapherConfigUtils.test.ts | 396 ++++++++++++++++++ .../utils/src/grapherConfigUtils.ts | 93 ++++ packages/@ourworldindata/utils/src/index.ts | 5 + site/DataPageV2.tsx | 11 +- site/ExplorerPage.tsx | 14 +- site/GrapherPage.tsx | 7 +- site/multiembedder/MultiEmbedder.tsx | 22 +- 30 files changed, 1035 insertions(+), 129 deletions(-) create mode 100644 .github/workflows/check-default-grapher-config.yml 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/.github/workflows/check-default-grapher-config.yml b/.github/workflows/check-default-grapher-config.yml new file mode 100644 index 00000000000..7750f9ea60f --- /dev/null +++ b/.github/workflows/check-default-grapher-config.yml @@ -0,0 +1,54 @@ +name: Check default grapher config +on: + push: + branches: + - "**" + - "!master" + paths: + - "packages/@ourworldindata/grapher/src/schema/**" + +jobs: + commit-default-config: + runs-on: ubuntu-latest + + steps: + - name: Clone repository + uses: actions/checkout@v4 + with: + ref: ${{ github.head_ref }} + + - uses: ./.github/actions/setup-node-yarn-deps + - uses: ./.github/actions/build-tsc + + - uses: hustcer/setup-nu@v3 + with: + version: "0.80" # Don't use 0.80 here, as it was a float number and will be convert to 0.8, you can use v0.80/0.80.0 or '0.80' + + # Turn all yaml files in the schema directory into json (should only be one) + - name: Convert yaml schema to json + run: | + (ls packages/@ourworldindata/grapher/src/schema/*.yaml + | each {|yaml| + open $yaml.name + | to json + | save -f ($yaml.name + | path parse + | upsert extension "json" + | path join) }) + shell: nu {0} + + # Construct default config objects for all grapher schemas in the schema directory (should only be one) + - name: Generate default grapher config + run: | + (ls packages/@ourworldindata/grapher/src/schema/grapher-schema.*.json + | each {|json| + node itsJustJavascript/devTools/schema/generate-default-object-from-schema.js $json.name --save-ts packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts }) + shell: nu {0} + + - name: Run prettier + run: yarn fixPrettierAll + + - uses: stefanzweifel/git-auto-commit-action@v5 + with: + commit_message: "🤖 update default grapher config" + file_pattern: "packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts" 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 + } + + // if isPublished is missing, add it + if (!config.isPublished) { + config.isPublished = false + } + + // 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 +} + +const updateExistingChart = async ( + knex: db.KnexReadWriteTransaction, + { + config, + user, + chartId, + }: { config: GrapherInterface; user: DbPlainUser; chartId: number } +): Promise => { + // 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 + } + + // if isPublished is missing, add it + if (!config.isPublished) { + config.isPublished = false + } + + // 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 +} + const saveGrapher = async ( knex: db.KnexReadWriteTransaction, user: DbPlainUser, @@ -349,68 +466,23 @@ 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 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! + newConfig = await updateExistingChart(knex, { + config: newConfig, + user, + chartId, + }) } 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] - ) + newConfig = await saveNewChart(knex, { + config: newConfig, + user, + }) + chartId = newConfig.id! } // Record this change in version history - const chartRevisionLog = { chartId: chartId as number, userId: user.id, @@ -461,7 +533,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 +551,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 +831,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 +854,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..03a1633a4db --- /dev/null +++ b/db/migration/1720600092980-MakeChartsInheritDefaults.ts @@ -0,0 +1,64 @@ +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 + } + + // if isPublished is missing, add it + if (!originalConfig.isPublished) { + originalConfig.isPublished = false + } + + 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/schemaProcessor/columns.json b/devTools/schemaProcessor/columns.json index 801878d4584..ef55467b8bb 100644 --- a/devTools/schemaProcessor/columns.json +++ b/devTools/schemaProcessor/columns.json @@ -44,7 +44,6 @@ { "type": "string", "pointer": "/map/colorScale/baseColorScheme", - "default": "default", "editor": "dropdown", "enumOptions": [ "YlGn", @@ -156,7 +155,6 @@ { "type": "integer", "pointer": "/map/timeTolerance", - "default": 0, "editor": "numeric" }, { @@ -195,7 +193,6 @@ { "type": "string", "pointer": "/baseColorScheme", - "default": "default", "editor": "textfield" }, { @@ -303,7 +300,6 @@ { "type": "string", "pointer": "/colorScale/baseColorScheme", - "default": "default", "editor": "dropdown", "enumOptions": [ "YlGn", @@ -434,7 +430,6 @@ { "type": "boolean", "pointer": "/isPublished", - "default": false, "editor": "checkbox" }, { @@ -471,7 +466,6 @@ { "type": "integer", "pointer": "/version", - "default": 1, "editor": "numeric" }, { @@ -583,17 +577,20 @@ { "type": "integer", "pointer": "/dimensions/0/display/numDecimalPlaces", - "editor": "numeric" + "editor": "numeric", + "default": 2 }, { "type": "integer", "pointer": "/dimensions/0/display/numSignificantFigures", - "editor": "numeric" + "editor": "numeric", + "default": 3 }, { "type": "string", "pointer": "/dimensions/0/display/zeroDay", - "editor": "textfield" + "editor": "textfield", + "default": "2020-01-21" }, { "type": "integer", @@ -801,7 +798,7 @@ { "type": "string", "pointer": "/entityTypePlural", - "default": "countries or regions", + "default": "countries and regions", "editor": "textfield" } ] diff --git a/devTools/svgTester/dump-data.ts b/devTools/svgTester/dump-data.ts index 0e08ec6983e..86bb43174a0 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 exclude 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.jsdom.test.ts b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts index 1b7d4400b71..96ff4d3a4df 100755 --- a/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts +++ b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts @@ -1,6 +1,6 @@ #! /usr/bin/env jest import { Grapher, GrapherProgrammaticInterface } from "../core/Grapher" -import { DEFAULT_GRAPHER_CONFIG_SCHEMA } from "./GrapherConstants" +import { defaultGrapherConfig } from "../schema/defaultGrapherConfig" import { ChartTypeName, EntitySelectionMode, @@ -76,13 +76,13 @@ it("can get dimension slots", () => { it("an empty Grapher serializes to an object that includes only the schema", () => { expect(new Grapher().toObject()).toEqual({ - $schema: DEFAULT_GRAPHER_CONFIG_SCHEMA, + $schema: defaultGrapherConfig.$schema, }) }) it("a bad chart type does not crash grapher", () => { const input = { - $schema: DEFAULT_GRAPHER_CONFIG_SCHEMA, + $schema: defaultGrapherConfig.$schema, type: "fff" as any, } expect(new Grapher(input).toObject()).toEqual(input) @@ -90,7 +90,7 @@ it("a bad chart type does not crash grapher", () => { it("does not preserve defaults in the object (except for the schema)", () => { expect(new Grapher({ tab: GrapherTabOption.chart }).toObject()).toEqual({ - $schema: DEFAULT_GRAPHER_CONFIG_SCHEMA, + $schema: defaultGrapherConfig.$schema, }) }) diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx index ed0b5d920ee..8a46e74a5ae 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -114,7 +114,6 @@ import { BASE_FONT_SIZE, CookieKey, ThereWasAProblemLoadingThisChart, - DEFAULT_GRAPHER_CONFIG_SCHEMA, DEFAULT_GRAPHER_WIDTH, DEFAULT_GRAPHER_HEIGHT, DEFAULT_GRAPHER_FRAME_PADDING, @@ -127,6 +126,7 @@ import { isContinentsVariableId, isPopulationVariableETLPath, } from "../core/GrapherConstants" +import { defaultGrapherConfig } from "../schema/defaultGrapherConfig" import { loadVariableDataAndMetadata } from "./loadVariable" import Cookies from "js-cookie" import { @@ -344,7 +344,7 @@ export class Grapher MapChartManager, SlopeChartManager { - @observable.ref $schema = DEFAULT_GRAPHER_CONFIG_SCHEMA + @observable.ref $schema = defaultGrapherConfig.$schema @observable.ref type = ChartTypeName.LineChart @observable.ref id?: number = undefined @observable.ref version = 1 @@ -527,7 +527,7 @@ export class Grapher deleteRuntimeAndUnchangedProps(obj, defaultObject) // always include the schema, even if it's the default - obj.$schema = this.$schema || DEFAULT_GRAPHER_CONFIG_SCHEMA + obj.$schema = this.$schema || defaultGrapherConfig.$schema // todo: nulls got into the DB for this one. we can remove after moving Graphers from DB. if (obj.stackMode === null) delete obj.stackMode @@ -1000,6 +1000,10 @@ export class Grapher this.selection.setSelectedEntities(this.selectedEntityNames) } + @computed get hasData(): boolean { + return this.dimensions.length > 0 || this.newSlugs.length > 0 + } + // Ready to go iff we have retrieved data for every variable associated with the chart @computed get isReady(): boolean { return this.whatAreWeWaitingFor === "" diff --git a/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts b/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts index 35f47981580..820811a58b5 100644 --- a/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts +++ b/packages/@ourworldindata/grapher/src/core/GrapherConstants.ts @@ -9,9 +9,6 @@ export const GRAPHER_TIMELINE_CLASS = "timeline-component" export const GRAPHER_SIDE_PANEL_CLASS = "side-panel" export const GRAPHER_SETTINGS_CLASS = "settings-menu-contents" -export const DEFAULT_GRAPHER_CONFIG_SCHEMA = - "https://files.ourworldindata.org/schemas/grapher-schema.004.json" - export const DEFAULT_GRAPHER_ENTITY_TYPE = "country or region" export const DEFAULT_GRAPHER_ENTITY_TYPE_PLURAL = "countries and regions" 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..cad2a6eb595 100644 --- a/packages/@ourworldindata/grapher/src/schema/README.md +++ b/packages/@ourworldindata/grapher/src/schema/README.md @@ -6,13 +6,26 @@ 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. +the current version of the schema, including the migration of pointing to the URL of the new schema version.s Checklist for breaking changes: - Rename the schema file to an increased version number - 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..63b8e4a2670 --- /dev/null +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -0,0 +1,79 @@ +// 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, + }, + 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", + 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 and regions", + 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..327bc684c85 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml @@ -12,7 +12,7 @@ $defs: description: Axis label min: type: number - description: Minimum domain value of the axis + description: Minimum domain value of the axis. Inferred from data if not provided. scaleType: type: string description: Toggle linear/logarithmic @@ -22,7 +22,7 @@ $defs: - log max: type: number - description: Maximum domain value of the axis + description: Maximum domain value of the axis. Inferred from data if not provided. canChangeScaleType: type: boolean description: Allow user to change lin/log @@ -56,8 +56,9 @@ $defs: type: string baseColorScheme: type: string - description: One of the predefined base color schemes - default: default + description: | + One of the predefined base color schemes. + If not provided, a default is automatically chosen based on the chart type. enum: - YlGn - YlGnBu @@ -182,7 +183,7 @@ $defs: minimum: 0 customNumericMinValue: type: number - description: The minimum bracket of the first bin + description: The minimum bracket of the first bin. Inferred from data if not provided. additionalProperties: false required: - title @@ -226,11 +227,11 @@ properties: $ref: "#/$defs/colorScale" timeTolerance: type: integer - default: 0 description: | Tolerance to use. If data points are missing for a time point, a match is accepted if it lies within the specified time period. The unit is the dominating time unit, usually years but can be days for - daily time series. + daily time series. If not provided, the tolerance specified in the metadata of the indicator is used. + If that's not specified, 0 is used. minimum: 0 toleranceStrategy: type: string @@ -258,7 +259,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,8 +284,9 @@ properties: - string baseColorScheme: type: string - default: default - description: The default color scheme if no color overrides are specified + description: | + The default color scheme if no color overrides are specified. + If not provided, a default is chosen based on the chart type. yAxis: $ref: "#/$defs/axis" tab: @@ -311,10 +315,10 @@ properties: type: integer description: | The lowest year to show in the timeline. If this is set then the user is not able to see - any data before this year + any data before this year. Inferred from data if not provided. variantName: type: string - description: "Optional internal variant name for distinguishing charts with the same title" + description: Optional internal variant name for distinguishing charts with the same title hideTimeline: type: boolean default: false @@ -348,7 +352,6 @@ properties: description: Short comma-separated list of source names isPublished: type: boolean - default: false description: Indicates if the chart is published on Our World In Data or still in draft invertColorScheme: type: boolean @@ -356,7 +359,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 +379,6 @@ properties: type: string version: type: integer - default: 1 minimum: 0 logo: type: string @@ -490,13 +493,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 @@ -601,7 +607,7 @@ properties: type: integer description: | The highest year to show in the timeline. If this is set then the user is not able to see - any data after this year + any data after this year. Inferred from data if not provided. hideConnectedScatterLines: type: boolean default: false @@ -660,7 +666,7 @@ properties: type: number entityTypePlural: description: Plural of the entity type (i.e. when entityType is 'country' this would be 'countries') - default: countries + default: countries and regions type: string missingDataStrategy: type: string 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..93f0aa0289e 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,39 @@ export const omitNullableValues = (object: T): NoUndefinedValues => { return result } +export function omitUndefinedValuesRecursive>( + obj: T +): NoUndefinedValues { + const result: any = {} + for (const key in obj) { + if (isPlainObject(obj[key])) { + // re-apply the function if we encounter a non-empty object + result[key] = omitUndefinedValuesRecursive(obj[key]) + } else if (obj[key] === undefined) { + // omit undefined values + } else { + // otherwise, keep the value + result[key] = obj[key] + } + } + return result +} + +export function omitEmptyObjectsRecursive>( + obj: T +): Partial { + const result: any = {} + for (const key in obj) { + if (isPlainObject(obj[key])) { + const isObjectEmpty = isEmpty(omitEmptyObjectsRecursive(obj[key])) + if (!isObjectEmpty) result[key] = obj[key] + } else { + result[key] = obj[key] + } + } + return result +} + export const isInIFrame = (): boolean => { try { return window.self !== window.top @@ -1739,7 +1775,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 +1953,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..882b0d74326 --- /dev/null +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts @@ -0,0 +1,396 @@ +#! /usr/bin/env jest + +import { + DimensionProperty, + 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 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("ignores empty objects", () => { + expect( + mergeGrapherConfigs( + { + title: "Parent title", + subtitle: "Parent subtitle", + }, + { + $schema: "004", + id: 1, + slug: "parent-slug", + version: 1, + title: "Title A", + }, + {} + ) + ).toEqual({ + $schema: "004", + id: 1, + slug: "parent-slug", + version: 1, + title: "Title A", + subtitle: "Parent subtitle", + }) + }) + + 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("strips empty objects from the config", () => { + expect(diffGrapherConfigs({ map: {} }, {})).toEqual({}) + expect( + diffGrapherConfigs( + { map: { colorScale: { customCategoryColors: {} } } }, + { map: { colorScale: { colorSchemeInvert: false } } } + ) + ).toEqual({}) + }) + + it("doesn't diff $schema, id, version, slug, isPublished or dimensions", () => { + expect( + diffGrapherConfigs( + { + title: "Chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + dimensions: [ + { property: DimensionProperty.y, variableId: 123456 }, + ], + }, + { + title: "Reference chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + dimensions: [ + { property: DimensionProperty.y, variableId: 123456 }, + ], + } + ) + ).toEqual({ + title: "Chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + dimensions: [{ property: DimensionProperty.y, variableId: 123456 }], + }) + }) + + 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..2a8db8e0120 --- /dev/null +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts @@ -0,0 +1,93 @@ +import { GrapherInterface } from "@ourworldindata/types" +import { + isEqual, + mergeWith, + uniq, + omit, + pick, + excludeUndefined, + omitUndefinedValuesRecursive, + omitEmptyObjectsRecursive, + traverseObjects, + isEmpty, +} from "./Util" + +const REQUIRED_KEYS = ["$schema", "dimensions"] + +const KEYS_EXCLUDED_FROM_INHERITANCE = [ + "$schema", + "id", + "slug", + "version", + "isPublished", +] + +export function mergeGrapherConfigs( + ...grapherConfigs: GrapherInterface[] +): GrapherInterface { + const configsToMerge = grapherConfigs.filter((c) => !isEmpty(c)) + + // return early if there are no configs to merge + if (configsToMerge.length === 0) return {} + if (configsToMerge.length === 1) return configsToMerge[0] + + // warn if one of the configs is missing a schema version + const configsWithoutSchema = configsToMerge.filter( + (c) => c["$schema"] === undefined + ) + if (configsWithoutSchema.length > 0) { + const configsJson = JSON.stringify(configsWithoutSchema, null, 2) + console.warn( + `About to merge Grapher configs with missing schema information: ${configsJson}` + ) + } + + // abort if the grapher configs have different schema versions + const uniqueSchemas = uniq( + excludeUndefined(configsToMerge.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 = configsToMerge.map((config, index) => { + if (index === configsToMerge.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 { + const keepKeys = [...REQUIRED_KEYS, ...KEYS_EXCLUDED_FROM_INHERITANCE] + const keep = pick(config, keepKeys) + + const diffed = omitEmptyObjectsRecursive( + omitUndefinedValuesRecursive( + traverseObjects(config, reference, (value, refValue) => { + if (refValue === undefined) return value + if (!isEqual(value, refValue)) return value + return undefined + }) + ) + ) + + return { ...diffed, ...keep } +} 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: {