diff --git a/adminSiteClient/GrapherConfigGridEditor.tsx b/adminSiteClient/GrapherConfigGridEditor.tsx index b26aa5edfde..979101f95cc 100644 --- a/adminSiteClient/GrapherConfigGridEditor.tsx +++ b/adminSiteClient/GrapherConfigGridEditor.tsx @@ -7,6 +7,7 @@ import { getWindowUrl, setWindowUrl, excludeNull, + mergeGrapherConfigs, } from "@ourworldindata/utils" import { GrapherConfigPatch } from "../adminShared/AdminSessionTypes.js" import { @@ -348,7 +349,10 @@ export class GrapherConfigGridEditor extends React.Component => { + // all charts inherit from the default config + const fullConfig = mergeGrapherConfigs(defaultGrapherConfig, config) + + // insert patch & full config into the chart_configs table + const configId = await getBinaryUUID(knex) + await db.knexRaw( + knex, + `-- sql + INSERT INTO chart_configs (id, patch, full) + VALUES (?, ?, ?) + `, + [configId, JSON.stringify(config), 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 + config.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=?, cc.full=? + WHERE c.id = ? + `, + [JSON.stringify(config), JSON.stringify(fullConfig), chartId] + ) + + return config +} + +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 + + // all charts inherit from the default config + const fullConfig = mergeGrapherConfigs(defaultGrapherConfig, config) + + // 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(config), 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 config +} + const saveGrapher = async ( knex: db.KnexReadWriteTransaction, user: DbPlainUser, @@ -364,62 +454,17 @@ 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 = await getBinaryUUID(knex) - 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=?, cc.full=? - WHERE c.id = ? - `, - [JSON.stringify(newConfig), JSON.stringify(newConfig), chartId] - ) + newConfig = await saveNewChart(knex, { config: newConfig, user }) + chartId = newConfig.id! } // Record this change in version history diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index 50d16876d0e..49b487aede8 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -9,7 +9,7 @@ import { deserializeJSONFromHTML, uniq, keyBy, - mergePartialGrapherConfigs, + mergeGrapherConfigs, compact, partition, } from "@ourworldindata/utils" @@ -158,7 +158,7 @@ export async function renderDataPageV2( // 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. const grapher = useIndicatorGrapherConfigs - ? mergePartialGrapherConfigs(grapherConfigForVariable, pageGrapher) + ? mergeGrapherConfigs(grapherConfigForVariable, pageGrapher) : pageGrapher ?? {} const faqDocs = compact( diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index 1a9d9145562..a12edb0e12c 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -43,7 +43,7 @@ import { JsonError, Url, IndexPost, - mergePartialGrapherConfigs, + mergeGrapherConfigs, OwidGdocType, OwidGdoc, OwidGdocDataInsightInterface, @@ -791,7 +791,7 @@ export const renderExplorerPage = async ( config: row.grapherConfigETL as string, }) : {} - return mergePartialGrapherConfigs(etlConfig, adminConfig) + return mergeGrapherConfigs(etlConfig, adminConfig) }) const wpContent = transformedProgram.wpBlockId diff --git a/db/migration/1720600092980-MakeChartsInheritDefaults.ts b/db/migration/1720600092980-MakeChartsInheritDefaults.ts new file mode 100644 index 00000000000..0f1966459b3 --- /dev/null +++ b/db/migration/1720600092980-MakeChartsInheritDefaults.ts @@ -0,0 +1,42 @@ +import { MigrationInterface, QueryRunner } from "typeorm" +import { 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, uuid, patch as config FROM chart_configs + ` + )) as { id: string; uuid: string; config: string }[] + + for (const chart of charts) { + const originalConfig = JSON.parse(chart.config) + + const fullConfig = mergeGrapherConfigs( + defaultGrapherConfig, + originalConfig + ) + + await queryRunner.query( + `-- sql + UPDATE chart_configs + SET full = ? + WHERE id = ? + `, + [JSON.stringify(fullConfig), chart.id] + ) + } + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `-- sql + UPDATE chart_configs + SET full = patch + ` + ) + } +} diff --git a/db/model/Variable.ts b/db/model/Variable.ts index af2588be4b7..3a98d5c7622 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -1,7 +1,11 @@ import _ from "lodash" import { Writable } from "stream" import * as db from "../db.js" -import { retryPromise, isEmpty } from "@ourworldindata/utils" +import { + retryPromise, + isEmpty, + mergeGrapherConfigs, +} from "@ourworldindata/utils" import { getVariableDataRoute, getVariableMetadataRoute, @@ -45,7 +49,7 @@ export async function getMergedGrapherConfigForVariable( const grapherConfigETL = row.grapherConfigETL ? JSON.parse(row.grapherConfigETL) : undefined - return _.merge({}, grapherConfigAdmin, grapherConfigETL) + return mergeGrapherConfigs(grapherConfigETL, grapherConfigAdmin) } // TODO: these are domain functions and should live somewhere else diff --git a/devTools/schema/generate-default-object-from-schema.ts b/devTools/schema/generate-default-object-from-schema.ts index b831c70b853..8c00e2a58d4 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 js 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/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/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..34415fc445d 100644 --- a/packages/@ourworldindata/grapher/src/schema/README.md +++ b/packages/@ourworldindata/grapher/src/schema/README.md @@ -6,9 +6,11 @@ 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 a default value in the schema, make sure to regenerate the default object from the schema and save it to `defaultGrapherConfig.ts` (see below). + 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. Also update `DEFAULT_GRAPHER_CONFIG_SCHEMA` to point to the new schema version number url. Checklist for breaking changes: @@ -16,3 +18,14 @@ 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) + +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..ace1284186d --- /dev/null +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -0,0 +1,82 @@ +// 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, + version: 1, + 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 3d4042a5586..1d51b63eff6 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 @@ -259,7 +258,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: @@ -282,7 +283,6 @@ properties: - string baseColorScheme: type: string - default: default description: The default color scheme if no color overrides are specified yAxis: $ref: "#/$defs/axis" @@ -357,7 +357,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 @@ -491,13 +492,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 530b86fe082..e12b0267684 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -54,6 +54,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 567ba543efe..b34d490997a 100644 --- a/packages/@ourworldindata/utils/src/Util.ts +++ b/packages/@ourworldindata/utils/src/Util.ts @@ -38,6 +38,7 @@ import { maxBy, memoize, merge, + mergeWith, min, minBy, noop, @@ -102,6 +103,7 @@ export { isNil, isNull, isNumber, + isPlainObject, isString, isUndefined, keyBy, @@ -110,6 +112,7 @@ export { maxBy, memoize, merge, + mergeWith, min, minBy, noop, @@ -1747,13 +1750,6 @@ export function filterValidStringValues( return filteredValues } -// TODO: type this correctly once we have moved types into their own top level package -export function mergePartialGrapherConfigs>( - ...grapherConfigs: (T | undefined)[] -): T { - return merge({}, ...grapherConfigs) -} - /** Works for: * #dod:text * #dod:text-hyphenated diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts new file mode 100644 index 00000000000..b05f340619e --- /dev/null +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts @@ -0,0 +1,270 @@ +#! /usr/bin/env jest + +import { 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 mutate input objects", () => { + const parentConfig = { title: "Title" } + const childConfig = { + title: "Title overwrite", + subtitle: "Subtitle", + } + mergeGrapherConfigs(parentConfig, childConfig) + expect(parentConfig).toEqual({ title: "Title" }) + expect(childConfig).toEqual({ + title: "Title overwrite", + subtitle: "Subtitle", + }) + }) + + it("doesn't merge configs of different schema versions", () => { + expect(() => + mergeGrapherConfigs( + { $schema: "1", title: "Title A" }, + { $schema: "2", title: "Title B" } + ) + ).toThrowError() + }) + + it("doesn't overwrite id, slug, version and isPublished", () => { + expect( + mergeGrapherConfigs( + { id: 1, slug: "parent-slug", version: 1, title: "Title A" }, + { title: "Title B" } + ) + ).toEqual({ title: "Title B" }) + expect( + mergeGrapherConfigs( + { id: 1, slug: "parent-slug", version: 1, title: "Title A" }, + { slug: "child-slug", title: "Title B" } + ) + ).toEqual({ slug: "child-slug", title: "Title B" }) + }) +}) + +describe(diffGrapherConfigs, () => { + it("drops redundant entries", () => { + expect( + diffGrapherConfigs( + { tab: GrapherTabOption.chart, title: "Chart" }, + { tab: GrapherTabOption.chart, title: "Reference chart" } + ) + ).toEqual({ title: "Chart" }) + expect( + diffGrapherConfigs( + { tab: GrapherTabOption.map }, + { tab: GrapherTabOption.map } + ) + ).toEqual({}) + }) + + 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("excludes $schema, id, version, slug and isPublished", () => { + expect( + diffGrapherConfigs( + { + title: "Chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + }, + { + title: "Chart", + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + } + ) + ).toEqual({ + $schema: + "https://files.ourworldindata.org/schemas/grapher-schema.004.json", + id: 20, + version: 1, + slug: "slug", + isPublished: false, + }) + }) +}) diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts new file mode 100644 index 00000000000..c0441cbbf8f --- /dev/null +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts @@ -0,0 +1,113 @@ +import { GrapherInterface } from "@ourworldindata/types" +import { + isEqual, + mergeWith, + uniq, + omit, + isPlainObject, + isEmpty, + NoUndefinedValues, + omitUndefinedValues, + excludeUndefined, +} 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}` + ) + } + + // warn if the grapher configs have different schema versions + const uniqueSchemas = uniq( + excludeUndefined(grapherConfigs.map((c) => c["$schema"])) + ) + if (uniqueSchemas.length > 1) { + console.warn( + `About to 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 + } + } + ) +} + +function omitUndefinedValuesRecursive>( + obj: TObj +): NoUndefinedValues { + const result: any = {} + for (const key in obj) { + const isNonEmptyObject = + isPlainObject(obj[key]) && !isEmpty(omitUndefinedValues(obj[key])) + if (isNonEmptyObject) { + result[key] = omitUndefinedValuesRecursive(obj[key]) + } else if ( + obj[key] !== undefined && + (!isPlainObject(obj[key]) || isNonEmptyObject) + ) { + result[key] = obj[key] + } + } + return result +} + +function traverseObjects>( + obj: TObj, + 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 +} + +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 1d0d4559a85..2d6e9ffe90a 100644 --- a/packages/@ourworldindata/utils/src/index.ts +++ b/packages/@ourworldindata/utils/src/index.ts @@ -109,7 +109,6 @@ export { type NodeWithUrl, filterValidStringValues, traverseEnrichedSpan, - mergePartialGrapherConfigs, copyToClipboard, checkIsGdocPost, checkIsGdocPostExcludingFragments, @@ -331,3 +330,8 @@ export { } from "./DonateUtils.js" export { isAndroid, isIOS } from "./BrowserUtils.js" + +export { + diffGrapherConfigs, + mergeGrapherConfigs, +} from "./grapherConfigUtils.js" diff --git a/site/DataPageV2.tsx b/site/DataPageV2.tsx index 83877927fa5..f10176a6968 100644 --- a/site/DataPageV2.tsx +++ b/site/DataPageV2.tsx @@ -8,7 +8,7 @@ import { SiteFooterContext, DataPageDataV2, serializeJSONForHTML, - mergePartialGrapherConfigs, + mergeGrapherConfigs, compact, FaqEntryData, pick, @@ -85,7 +85,7 @@ export const DataPageV2 = (props: { compact(grapher?.dimensions?.map((d) => d.variableId)) ) - const mergedGrapherConfig = mergePartialGrapherConfigs( + const mergedGrapherConfig = mergeGrapherConfigs( datapageData.chartConfig as GrapherInterface, grapher ) diff --git a/site/gdocs/components/Chart.tsx b/site/gdocs/components/Chart.tsx index 0dad75dd892..b73a58e31c0 100644 --- a/site/gdocs/components/Chart.tsx +++ b/site/gdocs/components/Chart.tsx @@ -11,7 +11,7 @@ import { EnrichedBlockChart, identity, Url, - merge, + mergeGrapherConfigs, } from "@ourworldindata/utils" import { renderSpans, useLinkedChart } from "../utils.js" import cx from "classnames" @@ -58,7 +58,7 @@ export default function Chart({ .map(mapKeywordToGrapherConfig) .filter(identity) as GrapherProgrammaticInterface[] - customizedChartConfig = merge( + customizedChartConfig = mergeGrapherConfigs( {}, !showAllControls ? grapherInterfaceWithHiddenControlsOnly : {}, !showAllTabs ? grapherInterfaceWithHiddenTabsOnly : {}, diff --git a/site/multiembedder/MultiEmbedder.tsx b/site/multiembedder/MultiEmbedder.tsx index b3416dc7ea3..5dc2363dc95 100644 --- a/site/multiembedder/MultiEmbedder.tsx +++ b/site/multiembedder/MultiEmbedder.tsx @@ -225,14 +225,20 @@ class MultiEmbedder { localConfig.hasTableTab = true } - const config = merge(grapherPageConfig, common, localConfig, { - manager: { - selection: new SelectionArray( - this.selection.selectedEntityNames - ), - }, - annotation, - }) + const config = merge( + {}, // merge mutates the first argument + grapherPageConfig, + common, + localConfig, + { + manager: { + selection: new SelectionArray( + this.selection.selectedEntityNames + ), + }, + annotation, + } + ) if (config.manager?.selection) this.graphersAndExplorersToUpdate.add(config.manager.selection)