diff --git a/adminSiteClient/ChartEditor.ts b/adminSiteClient/ChartEditor.ts index d13047ff5e0..5b86caf0d4c 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 + baseGrapherConfig: 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 { baseGrapherConfig } = this.manager + if (!baseGrapherConfig) return this.fullConfig + return diffGrapherConfigs(this.fullConfig, baseGrapherConfig) + } + @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..2b590bfa592 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 previous config layer is the default layer + baseGrapherConfig = defaultGrapherConfig async fetchGrapher(): Promise { const { grapherId } = this.props @@ -144,6 +146,7 @@ export class ChartEditorPage @action.bound private updateGrapher(): void { const config = this.fetchedGrapherConfig ?? this.props.grapherConfig + const grapherConfig = { ...config, // binds the grapher instance to this.grapher diff --git a/adminSiteClient/EditorHistoryTab.tsx b/adminSiteClient/EditorHistoryTab.tsx index 0fa832b4acf..c59788e736f 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 (
diff --git a/adminSiteClient/GrapherConfigGridEditor.tsx b/adminSiteClient/GrapherConfigGridEditor.tsx index b26aa5edfde..06ebd7ab5c0 100644 --- a/adminSiteClient/GrapherConfigGridEditor.tsx +++ b/adminSiteClient/GrapherConfigGridEditor.tsx @@ -348,6 +348,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 baseConfig = defaultGrapherConfig + const patchConfig = diffGrapherConfigs(config, baseConfig) + const fullConfig = mergeGrapherConfigs(baseConfig, patchConfig) + + // insert patch & full configs into the chart_configs table + const configId = await db.getBinaryUUID(knex) + 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"] + } + + // compute patch and full configs + const baseConfig = getBaseLayerConfig() + const patchConfig = diffGrapherConfigs(config, baseConfig) + const fullConfig = mergeGrapherConfigs(baseConfig, 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, @@ -348,64 +455,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 db.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=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 @@ -460,7 +520,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 ( @@ -478,7 +538,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) => { @@ -755,7 +818,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 } }) @@ -778,10 +841,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 c01d62bd91c..9046c9cc82c 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -157,6 +157,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 e8c0a3568d6..b69285f95d6 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -840,6 +840,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/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 164b0e5eb54..91b3be0ef21 100644 --- a/packages/@ourworldindata/grapher/src/core/Grapher.tsx +++ b/packages/@ourworldindata/grapher/src/core/Grapher.tsx @@ -1003,7 +1003,10 @@ export class Grapher @computed get whatAreWeWaitingFor(): string { const { newSlugs, inputTable, dimensions } = this - if (newSlugs.length || dimensions.length === 0) { + if (dimensions.length === 0) { + return `Waiting for dimensions to be set. ${inputTable.tableDescription}` + } + if (newSlugs.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..8adef8fa07e 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 add a new property with a default value or if you update the default value of an existing property, 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. + 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,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 8f1efb09ce4..f888ef464bf 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 567ba543efe..a95acc7721b 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, @@ -1121,6 +1124,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 @@ -1747,7 +1776,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 { @@ -1925,3 +1954,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..1db0e31edaa --- /dev/null +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts @@ -0,0 +1,339 @@ +#! /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("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("excludes $schema, id, version, slug and isPublished from inheritance", () => { + 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: undefined, + } + 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 1d0d4559a85..7dd328dd516 100644 --- a/packages/@ourworldindata/utils/src/index.ts +++ b/packages/@ourworldindata/utils/src/index.ts @@ -331,3 +331,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..4d4c57b2a58 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" @@ -85,6 +87,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 @@ -102,6 +105,12 @@ export const DataPageV2 = (props: { dataApiUrl: DATA_API_URL, } + // We bake a minimal version of the Grapher config + 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, @@ -181,7 +190,7 @@ export const DataPageV2 = (props: {