From de5445693ff828f261f35db86420476025f62236 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Thu, 18 Jul 2024 11:54:38 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=89=20(grapher)=20make=20charts=20inhe?= =?UTF-8?q?rit=20indicator-level=20settings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/AdminApp.tsx | 11 + adminSiteClient/ChartEditorPage.tsx | 13 + adminSiteServer/apiRouter.ts | 384 ++++++++++++------ baker/GrapherBaker.tsx | 4 +- baker/siteRenderers.tsx | 15 +- ...veIndicatorChartsToTheChartConfigsTable.ts | 184 +++++++++ db/model/ChartConfigs.ts | 21 + db/model/Variable.ts | 181 ++++++++- packages/@ourworldindata/grapher/src/index.ts | 1 + .../types/src/dbTypes/Variables.ts | 42 +- packages/@ourworldindata/types/src/index.ts | 4 - 11 files changed, 671 insertions(+), 189 deletions(-) create mode 100644 db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts create mode 100644 db/model/ChartConfigs.ts diff --git a/adminSiteClient/AdminApp.tsx b/adminSiteClient/AdminApp.tsx index 56804458a34..b50588857b8 100644 --- a/adminSiteClient/AdminApp.tsx +++ b/adminSiteClient/AdminApp.tsx @@ -154,6 +154,17 @@ export class AdminApp extends React.Component<{ path="/charts" component={ChartIndexPage} /> + {/* TODO(inheritance) */} + {/* + */} [ + variableId, + +usageCount, + ]) + ) } async fetchLogs(): Promise { diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index c4d2a74a935..18680597263 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -26,7 +26,11 @@ import { fetchS3MetadataByPath, fetchS3DataValuesByPath, searchVariables, + getGrapherConfigsForVariable, + insertNewGrapherConfigForVariable, + getGrapherConfigETLForVariable, } from "../db/model/Variable.js" +import { updateExistingPatchConfig } from "../db/model/ChartConfigs.js" import { getCanonicalUrl } from "@ourworldindata/components" import { camelCaseProperties, @@ -91,6 +95,7 @@ import { defaultGrapherConfig, getVariableDataRoute, getVariableMetadataRoute, + DEFAULT_GRAPHER_CONFIG_SCHEMA, } from "@ourworldindata/grapher" import { getDatasetById, setTagsForDataset } from "../db/model/Dataset.js" import { getUserById, insertUser, updateUser } from "../db/model/User.js" @@ -282,7 +287,7 @@ const saveNewChart = async ( } // compute patch and full configs - const baseConfig = defaultGrapherConfig + const baseConfig = await getBaseLayerConfigForChart(knex) const patchConfig = diffGrapherConfigs(config, baseConfig) const fullConfig = mergeGrapherConfigs(baseConfig, patchConfig) @@ -344,7 +349,7 @@ const updateExistingChart = async ( } // compute patch and full configs - const baseConfig = getBaseLayerConfig() + const baseConfig = await getBaseLayerConfigForChart(knex, chartId) const patchConfig = diffGrapherConfigs(config, baseConfig) const fullConfig = mergeGrapherConfigs(baseConfig, patchConfig) @@ -626,6 +631,67 @@ getRouteWithROTransaction( async (req, res, trx) => expectChartById(trx, req.params.chartId) ) +// TODO(inheritance) +// async function getBaseLayerConfigForIndicatorChartAdmin( +// trx: db.KnexReadonlyTransaction, +// variableId: number +// ): Promise { +// // check if there is an ETL-authored indicator chart +// const variableETL = await getGrapherConfigETLForVariable(trx, variableId) +// return variableETL?.config ?? {} +// } + +async function getBaseLayerConfigForChart( + trx: db.KnexReadonlyTransaction, + chartId?: number +): Promise { + if (chartId === undefined) return defaultGrapherConfig + + // check if the chart inherits settings from an indicator + const baseIndicator = await db.knexRawFirst<{ id: number }>( + trx, + `-- sql + SELECT indicatorId AS id + FROM inheriting_charts + WHERE chartId = ? + `, + [chartId] + ) + + // all charts inherit from the default config + if (!baseIndicator) return defaultGrapherConfig + + const variable = await getGrapherConfigsForVariable(trx, baseIndicator.id) + return mergeGrapherConfigs( + defaultGrapherConfig, + variable?.grapherConfigETL ?? {}, + variable?.grapherConfigAdmin ?? {} + ) +} + +getRouteWithROTransaction( + apiRouter, + "/charts/:chartId.base.json", + async (req, res, trx) => { + const chartId = expectInt(req.params.chartId) + return getBaseLayerConfigForChart(trx, chartId) + } +) + +// TODO(inheritance) +// getRouteWithROTransaction( +// apiRouter, +// "/variables/:variableId/grapherConfigs", +// async (req, res, trx) => { +// const variableId = expectInt(req.params.variableId) +// const variable = await getGrapherConfigsForVariable(trx, variableId) +// if (!variable) { +// throw new JsonError(`No variable with id ${variableId} found`) +// } +// return variable +// } +// ) + getRouteWithROTransaction( apiRouter, "/editorData/namespaces.json", @@ -1134,21 +1200,25 @@ getRouteWithROTransaction( const whereClause = filterSExpr?.toSql() ?? "true" const resultsWithStringGrapherConfigs = await db.knexRaw( trx, - `SELECT variables.id as id, - variables.name as name, - variables.grapherConfigAdmin as config, - d.name as datasetname, - namespaces.name as namespacename, - variables.createdAt as createdAt, - variables.updatedAt as updatedAt, - variables.description as description -FROM variables -LEFT JOIN active_datasets as d on variables.datasetId = d.id -LEFT JOIN namespaces on d.namespace = namespaces.name -WHERE ${whereClause} -ORDER BY variables.id DESC -LIMIT 50 -OFFSET ${offset.toString()}` + `-- sql + SELECT + variables.id as id, + variables.name as name, + chart_configs.patch as config, + d.name as datasetname, + namespaces.name as namespacename, + variables.createdAt as createdAt, + variables.updatedAt as updatedAt, + variables.description as description + FROM variables + LEFT JOIN active_datasets as d on variables.datasetId = d.id + LEFT JOIN namespaces on d.namespace = namespaces.name + LEFT JOIN chart_configs on variables.grapherConfigIdAdmin = chart_configs.id + WHERE ${whereClause} + ORDER BY variables.id DESC + LIMIT 50 + OFFSET ${offset.toString()} + ` ) const results = resultsWithStringGrapherConfigs.map((row: any) => ({ @@ -1157,11 +1227,14 @@ OFFSET ${offset.toString()}` })) const resultCount = await db.knexRaw<{ count: number }>( trx, - `SELECT count(*) as count -FROM variables -LEFT JOIN active_datasets as d on variables.datasetId = d.id -LEFT JOIN namespaces on d.namespace = namespaces.name -WHERE ${whereClause}` + `-- sql + SELECT count(*) as count + FROM variables + LEFT JOIN active_datasets as d on variables.datasetId = d.id + LEFT JOIN namespaces on d.namespace = namespaces.name + LEFT JOIN chart_configs on variables.grapherConfigIdAdmin = chart_configs.id + WHERE ${whereClause} + ` ) return { rows: results, numTotalRows: resultCount[0].count } } @@ -1175,14 +1248,25 @@ patchRouteWithRWTransaction( const variableIds = new Set(patchesList.map((patch) => patch.id)) const configsAndIds = await db.knexRaw< - Pick - >(trx, `SELECT id, grapherConfigAdmin FROM variables where id IN (?)`, [ - [...variableIds.values()], - ]) + Pick & { + grapherConfigAdmin: DbRawChartConfig["patch"] + } + >( + trx, + `-- sql + SELECT v.id, cc.patch AS grapherConfigAdmin + FROM variables v + LEFT JOIN chart_configs cc ON v.grapherConfigIdAdmin = cc.id + WHERE v.id IN (?) + `, + [[...variableIds.values()]] + ) const configMap = new Map( configsAndIds.map((item: any) => [ item.id, - item.grapherConfigAdmin ? JSON.parse(item.grapherConfig) : {}, + item.grapherConfigAdmin + ? JSON.parse(item.grapherConfigAdmin) + : {}, ]) ) // console.log("ids", configsAndIds.map((item : any) => item.id)) @@ -1192,11 +1276,32 @@ patchRouteWithRWTransaction( } for (const [variableId, newConfig] of configMap.entries()) { - await db.knexRaw( - trx, - `UPDATE variables SET grapherConfigAdmin = ? where id = ?`, - [JSON.stringify(newConfig), variableId] - ) + let grapherConfigIdAdmin = ( + await db.knexRawFirst< + Pick + >( + trx, + `-- sql + SELECT grapherConfigIdAdmin + FROM variables + WHERE id = ? + `, + [variableId] + ) + )?.grapherConfigIdAdmin + + if (grapherConfigIdAdmin) { + await updateExistingPatchConfig(trx, { + configId: grapherConfigIdAdmin, + config: newConfig, + }) + } else { + await insertNewGrapherConfigForVariable(trx, { + type: "admin", + variableId, + config: newConfig, + }) + } } return { success: true } @@ -1260,22 +1365,9 @@ getRouteWithROTransaction( await assignTagsForCharts(trx, charts) const grapherConfig = await getMergedGrapherConfigForVariable( - variableId, - trx + trx, + variableId ) - if ( - grapherConfig && - (!grapherConfig.dimensions || grapherConfig.dimensions.length === 0) - ) { - const dimensions: OwidChartDimensionInterface[] = [ - { - variableId: variableId, - property: DimensionProperty.y, - display: variable.display, - }, - ] - grapherConfig.dimensions = dimensions - } const variablesWithCharts: OwidVariableWithSource & { charts: Record @@ -1292,106 +1384,158 @@ getRouteWithROTransaction( } ) +function makeConfigValidForIndicator({ + config, + variableId, +}: { + config: GrapherInterface + variableId: number +}): GrapherInterface { + const updatedConfig = { ...config } + + // if no schema is given, assume it's the latest + if (!updatedConfig.$schema) { + updatedConfig.$schema = DEFAULT_GRAPHER_CONFIG_SCHEMA + } + + // check if the given dimensions are correct + if (updatedConfig.dimensions && updatedConfig.dimensions.length >= 1) { + // make sure there is only a single entry + updatedConfig.dimensions = updatedConfig.dimensions.slice(0, 1) + // make sure the variable id matches + updatedConfig.dimensions[0].variableId = variableId + } + + // fill dimensions if not given to make the updatedConfig plottable + if (!updatedConfig.dimensions || updatedConfig.dimensions.length === 0) { + updatedConfig.dimensions = [ + { property: DimensionProperty.y, variableId }, + ] + } + + return updatedConfig +} + +async function findChartsThatInheritFromIndicator( + trx: db.KnexReadonlyTransaction, + variableId: number +): Promise<{ chartId: number; patchConfig: GrapherInterface }[]> { + const charts = await db.knexRaw<{ + chartId: number + patchConfig: string + }>( + trx, + `-- sql + SELECT c.id as chartId, cc.patch as patchConfig + FROM inheriting_charts ic + JOIN charts c ON c.id = ic.chartId + JOIN chart_configs cc ON cc.id = c.configId + WHERE ic.variableId = ? + `, + [variableId] + ) + return charts.map((chart) => ({ + ...chart, + patchConfig: parseChartConfig(chart.patchConfig), + })) +} + postRouteWithRWTransaction( apiRouter, - "/variables/:variableId/grapherConfigETL/new", + "/variables/:variableId/grapherConfigETL/update", async (req, res, trx) => { const variableId = expectInt(req.params.variableId) - // const user = res.locals.user - const config = req.body + const configETL = makeConfigValidForIndicator({ + config: req.body, + variableId, + }) - // if no schema is given, assume it's the latest - if (!config.$schema) { - config.$schema = defaultGrapherConfig.$schema + const variable = await getGrapherConfigsForVariable(trx, variableId) + if (!variable) { + throw new JsonError(`Variable with id ${variableId} not found`, 500) } - // check if the given dimensions are correct - if (config.dimensions && config.dimensions.length >= 1) { - // make sure there is only a single entry - config.dimensions = config.dimensions.slice(0, 1) - // make sure the variable id matches - config.dimensions[0].variableId = variableId + if (variable.grapherConfigIdETL) { + await updateExistingPatchConfig(trx, { + configId: variable.grapherConfigIdETL, + config: configETL, + }) + } else { + await insertNewGrapherConfigForVariable(trx, { + type: "etl", + variableId, + config: configETL, + }) } - // fill dimensions if not given to make the config plottable - if (!config.dimensions || config.dimensions.length === 0) { - config.dimensions = [{ property: DimensionProperty.y, variableId }] + // update all charts that inherit from the indicator + const inheritingCharts = await findChartsThatInheritFromIndicator( + trx, + variableId + ) + for (const child of inheritingCharts) { + const fullConfig = mergeGrapherConfigs( + defaultGrapherConfig, + configETL, + variable.grapherConfigAdmin ?? {}, + child.patchConfig + ) + await db.knexRaw( + trx, + `-- sql + UPDATE chart_configs cc + JOIN charts c ON c.configId = cc.id + SET cc.full = ? + WHERE c.id = ? + `, + [JSON.stringify(fullConfig), child.chartId] + ) } + } +) - // ETL configs inherit from the default - const patchConfigETL = diffGrapherConfigs(config, defaultGrapherConfig) - const fullConfigETL = mergeGrapherConfigs( - defaultGrapherConfig, - patchConfigETL - ) +deleteRouteWithRWTransaction( + apiRouter, + "/variables/:variableId/grapherConfigETL/delete", + async (req, res, trx) => { + const variableId = expectInt(req.params.variableId) - // insert chart config into the database - const configId = await getBinaryUUID(trx) - await db.knexRaw( - trx, - `-- sql - INSERT INTO chart_configs (id, patch, full) - VALUES (?, ?, ?) - `, - [ - configId, - JSON.stringify(patchConfigETL), - JSON.stringify(fullConfigETL), - ] - ) + const variable = await getGrapherConfigsForVariable(trx, variableId) + if (!variable) { + throw new JsonError(`Variable with id ${variableId} not found`, 500) + } - // make a reference to the config from the variables table + // remove reference in the variables table await db.knexRaw( trx, `-- sql UPDATE variables - SET grapherConfigIdETL = ? + SET grapherConfigIdETL = NULL WHERE id = ? `, - [configId, variableId] + [variableId] ) - // grab the admin-authored indicator chart if there is one - let patchConfigAdmin: GrapherInterface = {} - const row = await db.knexRawFirst<{ - adminConfig: DbRawChartConfig["full"] // TODO: type - }>( + // delete row in the chart_configs table + await db.knexRaw( trx, `-- sql - SELECT cc.patch as adminConfig - FROM chart_configs cc - JOIN variables v ON cc.id = v.grapherConfigIdAdmin - WHERE v.id = ? + DELETE FROM chart_configs + WHERE id = ? `, - [variableId] + [variable.grapherConfigIdETL] ) - if (row) { - patchConfigAdmin = parseChartConfig(row.adminConfig) - } - // find all charts that inherit from the indicator - const children = await db.knexRaw<{ - chartId: number - patchConfig: string - }>( + // update all charts that inherit from the indicator + const inheritingCharts = await findChartsThatInheritFromIndicator( trx, - `-- sql - SELECT c.id as chartId, cc.patch as patchConfig - FROM inheriting_charts ic - JOIN charts c ON c.id = ic.chartId - JOIN chart_configs cc ON cc.id = c.configId - WHERE ic.variableId = ? - `, - [variableId] + variableId ) - - for (const child of children) { - const patchConfigChild = JSON.parse(child.patchConfig) - const fullConfigChild = mergeGrapherConfigs( + for (const child of inheritingCharts) { + const fullConfig = mergeGrapherConfigs( defaultGrapherConfig, - patchConfigETL, - patchConfigAdmin, - patchConfigChild + variable.grapherConfigAdmin ?? {}, + child.patchConfig ) await db.knexRaw( trx, @@ -1401,9 +1545,11 @@ postRouteWithRWTransaction( SET cc.full = ? WHERE c.id = ? `, - [JSON.stringify(fullConfigChild), child.chartId] + [JSON.stringify(fullConfig), child.chartId] ) } + + return { success: true } } ) diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index 9046c9cc82c..5d77cfd9658 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -150,8 +150,8 @@ export async function renderDataPageV2( knex: db.KnexReadWriteTransaction ) { const grapherConfigForVariable = await getMergedGrapherConfigForVariable( - variableId, - knex + knex, + variableId ) // Only merge the grapher config on the indicator if the caller tells us to do so - // this is true for preview pages for datapages on the indicator level but false diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index b69285f95d6..120c0b9f47f 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -49,6 +49,7 @@ import { OwidGdoc, OwidGdocDataInsightInterface, DbRawPost, + mergeGrapherConfigs, } from "@ourworldindata/utils" import { extractFormattingOptions } from "../serverUtils/wordpressUtils.js" import { @@ -800,7 +801,16 @@ export const renderExplorerPage = async ( if (requiredVariableIds.length) { partialGrapherConfigRows = await knexRaw( knex, - `SELECT id, grapherConfigETL, grapherConfigAdmin FROM variables WHERE id IN (?)`, + `-- sql + SELECT + v.id, + cc_etl.patch AS grapherConfigETL, + cc_admin.patch AS grapherConfigAdmin + FROM variables v + LEFT JOIN chart_configs cc_admin ON cc_admin.id=v.grapherConfigIdAdmin + LEFT JOIN chart_configs cc_etl ON cc_etl.id=v.grapherConfigIdETL + WHERE v.id IN (?) + `, [requiredVariableIds] ) @@ -840,8 +850,7 @@ export const renderExplorerPage = async ( config: row.grapherConfigETL as string, }) : {} - // TODO(inheritance): use mergeGrapherConfigs instead - return mergePartialGrapherConfigs(etlConfig, adminConfig) + return mergeGrapherConfigs(etlConfig, adminConfig) }) const wpContent = transformedProgram.wpBlockId diff --git a/db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts b/db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts new file mode 100644 index 00000000000..ba18080240d --- /dev/null +++ b/db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts @@ -0,0 +1,184 @@ +import { defaultGrapherConfig } from "@ourworldindata/grapher" +import { DimensionProperty, GrapherInterface } from "@ourworldindata/types" +import { omit } from "@ourworldindata/utils" +import { MigrationInterface, QueryRunner } from "typeorm" + +export class MoveIndicatorChartsToTheChartConfigsTable1721296631522 + implements MigrationInterface +{ + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`-- sql + ALTER TABLE variables + ADD COLUMN grapherConfigIdAdmin binary(16) UNIQUE AFTER sort, + ADD COLUMN grapherConfigIdETL binary(16) UNIQUE AFTER grapherConfigIdAdmin, + ADD CONSTRAINT fk_variables_grapherConfigIdAdmin + FOREIGN KEY (grapherConfigIdAdmin) + REFERENCES chart_configs (id) + ON DELETE RESTRICT + ON UPDATE RESTRICT, + ADD CONSTRAINT fk_variables_grapherConfigIdETL + FOREIGN KEY (grapherConfigIdETL) + REFERENCES chart_configs (id) + ON DELETE RESTRICT + ON UPDATE RESTRICT + `) + + // note that we copy the ETL-authored configs to the chart_configs table, + // but drop the admin-authored configs + + const variables = await queryRunner.query(`-- sql + SELECT id, grapherConfigETL + FROM variables + WHERE grapherConfigETL IS NOT NULL + `) + + for (const { id: variableId, grapherConfigETL } of variables) { + let config: GrapherInterface = JSON.parse(grapherConfigETL) + + // if the config has no schema, assume it's the default version + if (!config.$schema) { + config.$schema = defaultGrapherConfig.$schema + } + + // check if the given dimensions are correct + if (config.dimensions && config.dimensions.length >= 1) { + // make sure there is only a single entry + config.dimensions = config.dimensions.slice(0, 1) + // make sure the variable id matches + config.dimensions[0].variableId = variableId + } + + // fill dimensions if not given to make the config plottable + if (!config.dimensions || config.dimensions.length === 0) { + config.dimensions = [ + { property: DimensionProperty.y, variableId }, + ] + } + + // we have v3 configs in the database (the current version is v4); + // turn these into v4 configs by removing the `data` property + // which was the breaking change that lead to v4 + // (we don't have v2 or v1 configs in the database, so we don't need to handle those) + if ( + config.$schema === + "https://files.ourworldindata.org/schemas/grapher-schema.003.json" + ) { + config = omit(config, "data") + config.$schema = defaultGrapherConfig.$schema + } + + // insert config into the chart_configs table + const configId = await getBinaryUUID(queryRunner) + await queryRunner.query( + `-- sql + INSERT INTO chart_configs (id, patch) + VALUES (?, ?) + `, + [configId, JSON.stringify(config)] + ) + + // update reference in the variables table + await queryRunner.query( + `-- sql + UPDATE variables + SET grapherConfigIdETL = ? + WHERE id = ? + `, + [configId, variableId] + ) + } + + // drop `grapherConfigAdmin` and `grapherConfigETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + DROP COLUMN grapherConfigAdmin, + DROP COLUMN grapherConfigETL + `) + + // add a view that lists all charts that inherit from an indicator + await queryRunner.query(`-- sql + CREATE VIEW inheriting_charts 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, + 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 + cc.full ->> '$.type' != 'ScatterPlot' + GROUP BY + c.id + HAVING + COUNT(distinct yd.variableId) = 1 + ) + SELECT + variableId, + chartId + FROM + single_y_indicator_charts + ORDER BY + variableId + ) + `) + } + + public async down(queryRunner: QueryRunner): Promise { + // drop view + await queryRunner.query(`-- sql + DROP VIEW inheriting_charts + `) + + // add back the `grapherConfigAdmin` and `grapherConfigETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + ADD COLUMN grapherConfigAdmin json AFTER sort, + ADD COLUMN grapherConfigETL json AFTER grapherConfigAdmin + `) + + // copy configs from the chart_configs table to the variables table + await queryRunner.query(`-- sql + UPDATE variables v + JOIN chart_configs cc ON v.grapherConfigIdETL = cc.id + SET v.grapherConfigETL = cc.patch + `) + + // remove constraints on the `grapherConfigIdAdmin` and `grapherConfigIdETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + DROP CONSTRAINT fk_variables_grapherConfigIdAdmin, + DROP CONSTRAINT fk_variables_grapherConfigIdETL + `) + + // drop rows from the chart_configs table + await queryRunner.query(`-- sql + DELETE FROM chart_configs + WHERE id IN ( + SELECT grapherConfigIdETL FROM variables + WHERE grapherConfigIdETL IS NOT NULL + ) + `) + + // remove the `grapherConfigIdAdmin` and `grapherConfigIdETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + DROP COLUMN grapherConfigIdAdmin, + DROP COLUMN grapherConfigIdETL + `) + } +} + +const getBinaryUUID = async (queryRunner: QueryRunner): Promise => { + const rows = await queryRunner.query(`SELECT UUID_TO_BIN(UUID(), 1) AS id`) + return rows[0].id +} diff --git a/db/model/ChartConfigs.ts b/db/model/ChartConfigs.ts new file mode 100644 index 00000000000..0a0a8a73fd1 --- /dev/null +++ b/db/model/ChartConfigs.ts @@ -0,0 +1,21 @@ +import { DbInsertChartConfig, GrapherInterface } from "@ourworldindata/types" + +import * as db from "../db.js" + +export async function updateExistingPatchConfig( + knex: db.KnexReadWriteTransaction, + { + configId, + config, + }: { configId: DbInsertChartConfig["id"]; config: GrapherInterface } +): Promise { + await db.knexRaw( + knex, + `-- sql + UPDATE chart_configs + SET patch = ? + WHERE id = ? + `, + [JSON.stringify(config), configId] + ) +} diff --git a/db/model/Variable.ts b/db/model/Variable.ts index 8bcd74abe0e..eee9cc8c758 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -1,7 +1,14 @@ import _ from "lodash" import { Writable } from "stream" import * as db from "../db.js" -import { retryPromise, isEmpty } from "@ourworldindata/utils" +import { + retryPromise, + isEmpty, + excludeUndefined, + omitUndefinedValues, + mergeGrapherConfigs, + omitNullableValues, +} from "@ourworldindata/utils" import { getVariableDataRoute, getVariableMetadataRoute, @@ -20,33 +27,165 @@ import { GrapherInterface, DbRawVariable, VariablesTableName, + DbRawChartConfig, + parseChartConfig, + DbEnrichedChartConfig, + DbEnrichedVariable, } from "@ourworldindata/types" -import { knexRaw } from "../db.js" +import { knexRaw, knexRawFirst } from "../db.js" -//export type Field = keyof VariableRow +type VariableWithGrapherConfig = { + variableId: DbEnrichedVariable["id"] + configId?: DbEnrichedChartConfig["id"] + config?: DbEnrichedChartConfig["patch"] +} -export async function getMergedGrapherConfigForVariable( +export async function getGrapherConfigAdminForVariable( + knex: db.KnexReadonlyTransaction, + variableId: number +): Promise { + return getGrapherConfigForVariable(knex, variableId, "admin") +} + +export async function getGrapherConfigETLForVariable( + knex: db.KnexReadonlyTransaction, + variableId: number +): Promise { + return getGrapherConfigForVariable(knex, variableId, "admin") +} + +async function getGrapherConfigForVariable( + knex: db.KnexReadonlyTransaction, variableId: number, - knex: db.KnexReadonlyTransaction -): Promise { - const rows: Pick< - DbRawVariable, - "grapherConfigAdmin" | "grapherConfigETL" - >[] = await knexRaw( + type: "admin" | "etl" +): Promise { + const column = + type === "admin" ? "grapherConfigIdAdmin" : "grapherConfigIdETL" + + const row = await knexRawFirst<{ + variableId: DbRawVariable["id"] + configId: DbRawChartConfig["id"] + config?: DbRawChartConfig["patch"] + }>( knex, - `SELECT grapherConfigAdmin, grapherConfigETL FROM variables WHERE id = ?`, + `-- sql + SELECT + v.id AS variableId, + ?? AS configId, + cc.patch AS config, + FROM variables v + LEFT JOIN chart_configs cc ON ?? = cc.id + WHERE v.id = ? + `, + [`v.${column}`, `v.${column}`, variableId] + ) + + if (!row) return + + return omitNullableValues({ + ...row, + config: row.config ? parseChartConfig(row.config) : undefined, + }) +} + +export async function getGrapherConfigsForVariable( + knex: db.KnexReadonlyTransaction, + variableId: number +): Promise< + | (Pick< + DbEnrichedVariable, + "grapherConfigIdAdmin" | "grapherConfigIdETL" + > & { + variableId: DbEnrichedVariable["id"] + grapherConfigAdmin?: DbEnrichedChartConfig["patch"] + grapherConfigETL?: DbEnrichedChartConfig["patch"] + }) + | undefined +> { + const variable = await knexRawFirst< + Pick & { + variableId: DbRawVariable["id"] + grapherConfigAdmin?: DbRawChartConfig["patch"] + grapherConfigETL?: DbRawChartConfig["patch"] + } + >( + knex, + `-- sql + SELECT + v.id AS variableId, + v.grapherConfigIdAdmin, + v.grapherConfigIdETL, + cc_admin.patch AS grapherConfigAdmin, + cc_etl.patch AS grapherConfigETL + FROM variables v + LEFT JOIN chart_configs cc_admin ON v.grapherConfigIdAdmin = cc_admin.id + LEFT JOIN chart_configs cc_etl ON v.grapherConfigIdETL = cc_etl.id + WHERE v.id = ? + `, [variableId] ) - if (!rows.length) return - const row = rows[0] - const grapherConfigAdmin = row.grapherConfigAdmin - ? JSON.parse(row.grapherConfigAdmin) - : undefined - const grapherConfigETL = row.grapherConfigETL - ? JSON.parse(row.grapherConfigETL) - : undefined - // TODO(inheritance): use mergeGrapherConfigs instead - return _.merge({}, grapherConfigAdmin, grapherConfigETL) + + if (!variable) return + + return omitUndefinedValues({ + ...variable, + grapherConfigAdmin: variable.grapherConfigAdmin + ? parseChartConfig(variable.grapherConfigAdmin) + : undefined, + grapherConfigETL: variable.grapherConfigETL + ? parseChartConfig(variable.grapherConfigETL) + : undefined, + }) +} + +export async function getMergedGrapherConfigForVariable( + knex: db.KnexReadonlyTransaction, + variableId: number +): Promise { + const variable = await getGrapherConfigsForVariable(knex, variableId) + if (!variable) return + if (!variable.grapherConfigETL && !variable.grapherConfigAdmin) return + return mergeGrapherConfigs( + variable.grapherConfigETL ?? {}, + variable.grapherConfigAdmin ?? {} + ) +} + +export async function insertNewGrapherConfigForVariable( + knex: db.KnexReadonlyTransaction, + { + type, + variableId, + config, + }: { + type: "admin" | "etl" + variableId: number + config: GrapherInterface + } +): Promise { + // insert chart config into the database + const configId = await db.getBinaryUUID(knex) + await db.knexRaw( + knex, + `-- sql + INSERT INTO chart_configs (id, patch) + VALUES (?, ?) + `, + [configId, JSON.stringify(config)] + ) + + // make a reference to the config from the variables table + const column = + type === "admin" ? "grapherConfigIdAdmin" : "grapherConfigIdETL" + await db.knexRaw( + knex, + `-- sql + UPDATE variables + SET ?? = ? + WHERE id = ? + `, + [column, configId, variableId] + ) } // TODO: these are domain functions and should live somewhere else diff --git a/packages/@ourworldindata/grapher/src/index.ts b/packages/@ourworldindata/grapher/src/index.ts index 9e9d08b9902..ed27bd627f5 100644 --- a/packages/@ourworldindata/grapher/src/index.ts +++ b/packages/@ourworldindata/grapher/src/index.ts @@ -26,6 +26,7 @@ export { grapherInterfaceWithHiddenTabsOnly, CONTINENTS_INDICATOR_ID, POPULATION_INDICATOR_ID_USED_IN_ADMIN, + DEFAULT_GRAPHER_CONFIG_SCHEMA, } from "./core/GrapherConstants" export { getVariableDataRoute, diff --git a/packages/@ourworldindata/types/src/dbTypes/Variables.ts b/packages/@ourworldindata/types/src/dbTypes/Variables.ts index ce241a997c8..f085efcbf00 100644 --- a/packages/@ourworldindata/types/src/dbTypes/Variables.ts +++ b/packages/@ourworldindata/types/src/dbTypes/Variables.ts @@ -20,8 +20,8 @@ export interface DbInsertVariable { descriptionShort?: string | null dimensions?: JsonString | null display: JsonString - grapherConfigAdmin?: JsonString | null - grapherConfigETL?: JsonString | null + grapherConfigIdAdmin?: Buffer | null + grapherConfigIdETL?: Buffer | null id?: number license?: JsonString | null licenses?: JsonString | null @@ -66,8 +66,6 @@ export type DbEnrichedVariable = Omit< | "dimensions" | "descriptionKey" | "originalMetadata" - | "grapherConfigAdmin" - | "grapherConfigETL" | "processingLog" | "sort" > & { @@ -77,8 +75,6 @@ export type DbEnrichedVariable = Omit< dimensions: VariableDisplayDimension | null descriptionKey: string[] | null originalMetadata: unknown | null - grapherConfigAdmin: GrapherInterface | null - grapherConfigETL: GrapherInterface | null processingLog: unknown | null sort: string[] | null } @@ -149,30 +145,6 @@ export function serializeVariableOriginalMetadata( return originalMetadata ? JSON.stringify(originalMetadata) : null } -export function parseVariableGrapherConfigAdmin( - grapherConfigAdmin: JsonString | null -): GrapherInterface { - return grapherConfigAdmin ? JSON.parse(grapherConfigAdmin) : null -} - -export function serializeVariableGrapherConfigAdmin( - grapherConfigAdmin: GrapherInterface | null -): JsonString | null { - return grapherConfigAdmin ? JSON.stringify(grapherConfigAdmin) : null -} - -export function parseVariableGrapherConfigETL( - grapherConfigETL: JsonString | null -): GrapherInterface { - return grapherConfigETL ? JSON.parse(grapherConfigETL) : null -} - -export function serializeVariableGrapherConfigETL( - grapherConfigETL: GrapherInterface | null -): JsonString | null { - return grapherConfigETL ? JSON.stringify(grapherConfigETL) : null -} - export function parseVariableProcessingLog( processingLog: JsonString | null ): any { @@ -204,10 +176,6 @@ export function parseVariablesRow(row: DbRawVariable): DbEnrichedVariable { dimensions: parseVariableDimensions(row.dimensions), descriptionKey: parseVariableDescriptionKey(row.descriptionKey), originalMetadata: parseVariableOriginalMetadata(row.originalMetadata), - grapherConfigAdmin: parseVariableGrapherConfigAdmin( - row.grapherConfigAdmin - ), - grapherConfigETL: parseVariableGrapherConfigETL(row.grapherConfigETL), processingLog: parseVariableProcessingLog(row.processingLog), sort: parseVariableSort(row.sort), } @@ -224,12 +192,6 @@ export function serializeVariablesRow(row: DbEnrichedVariable): DbRawVariable { originalMetadata: serializeVariableOriginalMetadata( row.originalMetadata ), - grapherConfigAdmin: serializeVariableGrapherConfigAdmin( - row.grapherConfigAdmin - ), - grapherConfigETL: serializeVariableGrapherConfigETL( - row.grapherConfigETL - ), processingLog: serializeVariableProcessingLog(row.processingLog), sort: serializeVariableSort(row.sort), } diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index f888ef464bf..fca5baf4db7 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -634,10 +634,6 @@ export { serializeVariableOriginalMetadata, parseVariableLicenses, serializeVariableLicenses, - parseVariableGrapherConfigAdmin, - serializeVariableGrapherConfigAdmin, - parseVariableGrapherConfigETL, - serializeVariableGrapherConfigETL, parseVariableProcessingLog, serializeVariableProcessingLog, type License,