diff --git a/adminSiteClient/AbstractChartEditor.ts b/adminSiteClient/AbstractChartEditor.ts index 0fda11075f3..86ba3884def 100644 --- a/adminSiteClient/AbstractChartEditor.ts +++ b/adminSiteClient/AbstractChartEditor.ts @@ -80,9 +80,19 @@ export abstract class AbstractChartEditor< return mergeGrapherConfigs(parentConfig ?? {}, patchConfig) } - /** live-updating full config */ + /** live-updating config */ + @computed get liveConfig(): GrapherInterface { + return this.grapher.object + } + + @computed get liveConfigWithDefaults(): GrapherInterface { + return mergeGrapherConfigs(defaultGrapherConfig, this.liveConfig) + } + + /** full config: patch config merged with parent config */ @computed get fullConfig(): GrapherInterface { - return mergeGrapherConfigs(defaultGrapherConfig, this.grapher.object) + if (!this.activeParentConfig) return this.liveConfig + return mergeGrapherConfigs(this.activeParentConfig, this.liveConfig) } /** parent config currently applied to grapher */ @@ -103,7 +113,7 @@ export abstract class AbstractChartEditor< /** patch config of the chart that is written to the db on save */ @computed get patchConfig(): GrapherInterface { return diffGrapherConfigs( - this.fullConfig, + this.liveConfigWithDefaults, this.activeParentConfigWithDefaults ?? defaultGrapherConfig ) } diff --git a/adminSiteClient/ChartEditor.ts b/adminSiteClient/ChartEditor.ts index 6c9e7809eaf..c9a7bbca7b5 100644 --- a/adminSiteClient/ChartEditor.ts +++ b/adminSiteClient/ChartEditor.ts @@ -75,7 +75,7 @@ export class ChartEditor extends AbstractChartEditor { /** parent variable id, derived from the config */ @computed get parentVariableId(): number | undefined { - return getParentVariableIdFromChartConfig(this.fullConfig) + return getParentVariableIdFromChartConfig(this.liveConfig) } @computed get availableTabs(): EditorTab[] { diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 3332fe34c90..85f6e8ae690 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -107,7 +107,6 @@ import { } from "@ourworldindata/types" import { uuidv7 } from "uuidv7" import { - defaultGrapherConfig, migrateGrapherConfigToLatestVersion, getVariableDataRoute, getVariableMetadataRoute, @@ -324,14 +323,10 @@ const saveNewChart = async ( const parent = shouldInherit ? await getParentByChartConfig(knex, config) : undefined - const fullParentConfig = mergeGrapherConfigs( - defaultGrapherConfig, - parent?.config ?? {} - ) // compute patch and full configs - const patchConfig = diffGrapherConfigs(config, fullParentConfig) - const fullConfig = mergeGrapherConfigs(fullParentConfig, patchConfig) + const patchConfig = diffGrapherConfigs(config, parent?.config ?? {}) + const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig) const fullConfigStringified = serializeChartConfig(fullConfig) // insert patch & full configs into the chart_configs table @@ -420,14 +415,10 @@ const updateExistingChart = async ( const parent = shouldInherit ? await getParentByChartConfig(knex, config) : undefined - const fullParentConfig = mergeGrapherConfigs( - defaultGrapherConfig, - parent?.config ?? {} - ) // compute patch and full configs - const patchConfig = diffGrapherConfigs(config, fullParentConfig) - const fullConfig = mergeGrapherConfigs(fullParentConfig, patchConfig) + const patchConfig = diffGrapherConfigs(config, parent?.config ?? {}) + const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig) const fullConfigStringified = serializeChartConfig(fullConfig) const chartConfigId = await db.knexRawFirst>( diff --git a/db/migration/1730455806132-RemoveDefaultConfigLayer.ts b/db/migration/1730455806132-RemoveDefaultConfigLayer.ts new file mode 100644 index 00000000000..aa0ab450607 --- /dev/null +++ b/db/migration/1730455806132-RemoveDefaultConfigLayer.ts @@ -0,0 +1,116 @@ +import { defaultGrapherConfig } from "@ourworldindata/grapher" +import { mergeGrapherConfigs } from "@ourworldindata/utils" +import { MigrationInterface, QueryRunner } from "typeorm" + +export class RemoveDefaultConfigLayer1730455806132 + implements MigrationInterface +{ + private async recomputeFullChartConfigs( + queryRunner: QueryRunner, + { useDefaultLayer }: { useDefaultLayer: boolean } + ): Promise { + // recompute all full configs, without the default layer + const charts = await queryRunner.query(` + -- sql + SELECT + cc.id AS configId, + cc.patch AS patchConfig, + cc_etl.patch AS etlConfig, + cc_admin.patch AS adminConfig, + c.isInheritanceEnabled + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + LEFT JOIN charts_x_parents cxp ON cxp.chartId = c.id + LEFT JOIN variables v ON v.id = cxp.variableId + LEFT JOIN chart_configs cc_etl ON cc_etl.id = v.grapherConfigIdETL + LEFT JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin + `) + for (const chart of charts) { + const patchConfig = JSON.parse(chart.patchConfig) + + const etlConfig = chart.etlConfig + ? JSON.parse(chart.etlConfig) + : undefined + const adminConfig = chart.adminConfig + ? JSON.parse(chart.adminConfig) + : undefined + + const fullConfig = mergeGrapherConfigs( + useDefaultLayer ? defaultGrapherConfig : {}, + chart.isInheritanceEnabled ? etlConfig ?? {} : {}, + chart.isInheritanceEnabled ? adminConfig ?? {} : {}, + patchConfig + ) + + await queryRunner.query( + ` + -- sql + UPDATE chart_configs cc + SET cc.full = ? + WHERE cc.id = ? + `, + [JSON.stringify(fullConfig), chart.configId] + ) + } + } + + private async updateChartsXParentsView( + queryRunner: QueryRunner + ): Promise { + // identical to the current view definition, + // but uses `COALESCE(cc.full ->> '$.type', 'LineChart')` + // instead of `cc.full ->> '$.type'` since the full config + // might not have a type + await queryRunner.query(`-- sql + ALTER VIEW charts_x_parents AS ( + WITH y_dimensions AS ( + SELECT + * + FROM + chart_dimensions + WHERE + property = 'y' + ), + single_y_indicator_charts AS ( + SELECT + c.id as chartId, + cc.patch as patchConfig, + -- should only be one + max(yd.variableId) as variableId + FROM + charts c + JOIN chart_configs cc ON cc.id = c.configId + JOIN y_dimensions yd ON c.id = yd.chartId + WHERE + -- scatter plots can't inherit settings + COALESCE(cc.full ->> '$.type', 'LineChart') != 'ScatterPlot' + GROUP BY + c.id + HAVING + -- restrict to single y-variable charts + COUNT(distinct yd.variableId) = 1 + ) + SELECT + variableId, + chartId + FROM + single_y_indicator_charts + ORDER BY + variableId + ) + `) + } + + public async up(queryRunner: QueryRunner): Promise { + await this.recomputeFullChartConfigs(queryRunner, { + useDefaultLayer: false, + }) + await this.updateChartsXParentsView(queryRunner) + } + + public async down(queryRunner: QueryRunner): Promise { + await this.recomputeFullChartConfigs(queryRunner, { + useDefaultLayer: true, + }) + } +} diff --git a/db/model/Variable.ts b/db/model/Variable.ts index ebc232fbc3e..753dcf86d6b 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -11,7 +11,6 @@ import { import { getVariableDataRoute, getVariableMetadataRoute, - defaultGrapherConfig, } from "@ourworldindata/grapher" import pl from "nodejs-polars" import { uuidv7 } from "uuidv7" @@ -262,7 +261,6 @@ export async function updateAllChartsThatInheritFromIndicator( for (const chart of inheritingCharts) { const fullConfig = mergeGrapherConfigs( - defaultGrapherConfig, patchConfigETL ?? {}, patchConfigAdmin ?? {}, chart.patchConfig diff --git a/devTools/svgTester/dump-data.ts b/devTools/svgTester/dump-data.ts index 86bb43174a0..49a26ae98c5 100644 --- a/devTools/svgTester/dump-data.ts +++ b/devTools/svgTester/dump-data.ts @@ -1,8 +1,6 @@ #! /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" @@ -26,11 +24,7 @@ async function main(parsedArgs: parseArgs.ParsedArgs) { const allGraphers = [...graphersBySlug.values()] const saveJobs: utils.SaveGrapherSchemaAndDataJob[] = allGraphers.map( (grapher) => { - // since we're not baking defaults, we also exclude them here - return { - config: diffGrapherConfigs(grapher, defaultGrapherConfig), - outDir, - } + return { config: grapher, outDir } } ) diff --git a/packages/@ourworldindata/grapher/src/schema/README.md b/packages/@ourworldindata/grapher/src/schema/README.md index 2d6ae518365..dfee8e758e2 100644 --- a/packages/@ourworldindata/grapher/src/schema/README.md +++ b/packages/@ourworldindata/grapher/src/schema/README.md @@ -6,7 +6,7 @@ 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. +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). 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 diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts index 45bcdfd4263..60209959c34 100644 --- a/packages/@ourworldindata/utils/src/Util.ts +++ b/packages/@ourworldindata/utils/src/Util.ts @@ -1968,7 +1968,7 @@ export function traverseObjects>( } export function getParentVariableIdFromChartConfig( - config: GrapherInterface // could be a patch config + config: GrapherInterface ): number | undefined { const { type = ChartTypeName.LineChart, dimensions } = config diff --git a/site/DataPageV2.tsx b/site/DataPageV2.tsx index 7133cd00653..4d076fdefcf 100644 --- a/site/DataPageV2.tsx +++ b/site/DataPageV2.tsx @@ -1,5 +1,4 @@ import { - defaultGrapherConfig, getVariableDataRoute, getVariableMetadataRoute, GrapherProgrammaticInterface, @@ -105,12 +104,6 @@ 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, @@ -190,7 +183,7 @@ export const DataPageV2 = (props: {