From f5875779ba1f90c5c10ad06a5186197089a8f878 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Tue, 9 Jul 2024 17:39:14 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20(db)=20add=20chart=5Fconfigs=20t?= =?UTF-8?q?able?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteServer/apiRouter.ts | 310 ++++++++++++------ adminSiteServer/testPageRouter.tsx | 88 +++-- baker/GrapherBaker.tsx | 28 +- baker/GrapherBakingUtils.ts | 7 +- baker/GrapherImageBaker.tsx | 16 +- baker/SiteBaker.tsx | 18 +- baker/algolia/indexChartsToAlgolia.ts | 15 +- baker/countryProfiles.tsx | 23 +- baker/redirects.ts | 8 +- baker/siteRenderers.tsx | 35 +- baker/sitemap.ts | 21 +- baker/updateChartEntities.ts | 38 ++- db/db.ts | 8 +- .../1719842654592-AddChartConfigsTable.ts | 136 ++++++++ db/model/Chart.ts | 212 +++++++----- db/model/Gdoc/GdocPost.ts | 23 +- db/model/Post.ts | 25 +- db/model/SuggestedChartRevision.ts | 4 +- db/tests/basic.test.ts | 27 +- .../types/src/dbTypes/ChartConfigs.ts | 46 +++ .../types/src/dbTypes/ChartRevisions.ts | 2 +- .../types/src/dbTypes/Charts.ts | 32 +- .../src/dbTypes/SuggestedChartRevisions.ts | 2 +- packages/@ourworldindata/types/src/index.ts | 16 +- 24 files changed, 778 insertions(+), 362 deletions(-) create mode 100644 db/migration/1719842654592-AddChartConfigsTable.ts create mode 100644 packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 58ecda1740d..83ebd4f32f1 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -72,7 +72,7 @@ import { PostsTableName, DbRawPost, DbPlainChartSlugRedirect, - DbRawChart, + DbPlainChart, DbInsertChartRevision, serializeChartConfig, DbRawOrigin, @@ -83,6 +83,7 @@ import { DbPlainDataset, DbInsertUser, FlatTagGraph, + DbRawChartConfig, } from "@ourworldindata/types" import { getVariableDataRoute, @@ -273,6 +274,16 @@ const expectChartById = async ( throw new JsonError(`No chart found for id ${chartId}`, 404) } +const getBinaryUUID = async ( + knex: db.KnexReadonlyTransaction +): Promise => { + const { id } = (await db.knexRawFirst<{ id: string }>( + knex, + `SELECT UUID_TO_BIN(UUID(), 1) AS id` + ))! + return id +} + const saveGrapher = async ( knex: db.KnexReadWriteTransaction, user: DbPlainUser, @@ -294,9 +305,17 @@ const saveGrapher = async ( } async function isSlugUsedInOtherGrapher() { - const rows = await db.knexRaw>( + const rows = await db.knexRaw>( knex, - `SELECT id FROM charts WHERE id != ? AND config->>"$.isPublished" = "true" AND JSON_EXTRACT(config, "$.slug") = ?`, + `-- sql + SELECT c.id + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + WHERE + c.id != ? + AND cc.full ->> "$.isPublished" = "true" + AND cc.full ->> "$.slug" = ? + `, // -1 is a placeholder ID that will never exist; but we cannot use NULL because // in that case we would always get back an empty resultset [existingConfig ? existingConfig.id : -1, newConfig.slug] @@ -348,25 +367,59 @@ const saveGrapher = async ( const now = new Date() let chartId = existingConfig && existingConfig.id const newJsonConfig = JSON.stringify(newConfig) - if (existingConfig) + if (existingConfig) { await db.knexRaw( knex, - `UPDATE charts SET config=?, updatedAt=?, lastEditedAt=?, lastEditedByUserId=? WHERE id = ?`, - [newJsonConfig, now, now, user.id, chartId] + `-- 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] + ) + } else { + const configId = await getBinaryUUID(knex) + await db.knexRaw( + knex, + `-- sql + INSERT INTO chart_configs (id, patch, full) + VALUES (?, ?, ?) + `, + [configId, newJsonConfig, newJsonConfig] ) - else { const result = await db.knexRawInsert( knex, - `INSERT INTO charts (config, createdAt, updatedAt, lastEditedAt, lastEditedByUserId) VALUES (?, ?, ?, ?, ?)`, - [newJsonConfig, now, now, now, user.id] + `-- 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, `UPDATE charts SET config=? WHERE id = ?`, [ - JSON.stringify(newConfig), - 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] + ) } // Record this change in version history @@ -447,11 +500,12 @@ getRouteWithROTransaction(apiRouter, "/charts.json", async (req, res, trx) => { const charts = await db.knexRaw( trx, `-- sql - SELECT ${oldChartFieldList} FROM charts - JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId - LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId - ORDER BY charts.lastEditedAt DESC LIMIT ? - `, + SELECT ${oldChartFieldList} FROM charts + JOIN chart_configs ON chart_configs.id = charts.configId + JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId + LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId + ORDER BY charts.lastEditedAt DESC LIMIT ? + `, [limit] ) @@ -467,36 +521,37 @@ getRouteWithROTransaction(apiRouter, "/charts.csv", async (req, res, trx) => { const charts = await db.knexRaw( trx, `-- sql - SELECT - charts.id, - charts.config->>"$.version" AS version, - CONCAT("${BAKED_BASE_URL}/grapher/", charts.config->>"$.slug") AS url, - CONCAT("${ADMIN_BASE_URL}", "/admin/charts/", charts.id, "/edit") AS editUrl, - charts.config->>"$.slug" AS slug, - charts.config->>"$.title" AS title, - charts.config->>"$.subtitle" AS subtitle, - charts.config->>"$.sourceDesc" AS sourceDesc, - charts.config->>"$.note" AS note, - charts.config->>"$.type" AS type, - charts.config->>"$.internalNotes" AS internalNotes, - charts.config->>"$.variantName" AS variantName, - charts.config->>"$.isPublished" AS isPublished, - charts.config->>"$.tab" AS tab, - JSON_EXTRACT(charts.config, "$.hasChartTab") = true AS hasChartTab, - JSON_EXTRACT(charts.config, "$.hasMapTab") = true AS hasMapTab, - charts.config->>"$.originUrl" AS originUrl, - charts.lastEditedAt, - charts.lastEditedByUserId, - lastEditedByUser.fullName AS lastEditedBy, - charts.publishedAt, - charts.publishedByUserId, - publishedByUser.fullName AS publishedBy - FROM charts - JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId - LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId - ORDER BY charts.lastEditedAt DESC - LIMIT ? - `, + SELECT + charts.id, + chart_configs.full->>"$.version" AS version, + CONCAT("${BAKED_BASE_URL}/grapher/", chart_configs.full->>"$.slug") AS url, + CONCAT("${ADMIN_BASE_URL}", "/admin/charts/", charts.id, "/edit") AS editUrl, + chart_configs.full->>"$.slug" AS slug, + chart_configs.full->>"$.title" AS title, + chart_configs.full->>"$.subtitle" AS subtitle, + chart_configs.full->>"$.sourceDesc" AS sourceDesc, + chart_configs.full->>"$.note" AS note, + chart_configs.full->>"$.type" AS type, + chart_configs.full->>"$.internalNotes" AS internalNotes, + chart_configs.full->>"$.variantName" AS variantName, + chart_configs.full->>"$.isPublished" AS isPublished, + chart_configs.full->>"$.tab" AS tab, + JSON_EXTRACT(chart_configs.full, "$.hasChartTab") = true AS hasChartTab, + JSON_EXTRACT(chart_configs.full, "$.hasMapTab") = true AS hasMapTab, + chart_configs.full->>"$.originUrl" AS originUrl, + charts.lastEditedAt, + charts.lastEditedByUserId, + lastEditedByUser.fullName AS lastEditedBy, + charts.publishedAt, + charts.publishedByUserId, + publishedByUser.fullName AS publishedBy + FROM charts + JOIN chart_configs ON chart_configs.id = charts.configId + JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId + LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId + ORDER BY charts.lastEditedAt DESC + LIMIT ? + `, [limit] ) // note: retrieving references is VERY slow. @@ -770,7 +825,18 @@ deleteRouteWithRWTransaction( `DELETE FROM suggested_chart_revisions WHERE chartId=?`, [chart.id] ) - await db.knexRaw(trx, `DELETE FROM charts WHERE id=?`, [chart.id]) + + const row = await db.knexRawFirst<{ configId: number }>( + trx, + `SELECT configId FROM charts WHERE id = ?`, + [chart.id] + ) + if (row) { + await db.knexRaw(trx, `DELETE FROM charts WHERE id=?`, [chart.id]) + await db.knexRaw(trx, `DELETE FROM chart_configs WHERE id=?`, [ + row.configId, + ]) + } if (chart.isPublished) await triggerStaticBuild( @@ -1092,7 +1158,7 @@ getRouteWithROTransaction( trx ): Promise> => { const context: OperationContext = { - grapherConfigFieldName: "config", + grapherConfigFieldName: "chart_configs.full", whitelistedColumnNamesAndTypes: chartBulkUpdateAllowedColumnNamesAndTypes, } @@ -1110,21 +1176,25 @@ getRouteWithROTransaction( const whereClause = filterSExpr?.toSql() ?? "true" const resultsWithStringGrapherConfigs = await db.knexRaw( trx, - `SELECT charts.id as id, - charts.config as config, - charts.createdAt as createdAt, - charts.updatedAt as updatedAt, - charts.lastEditedAt as lastEditedAt, - charts.publishedAt as publishedAt, - lastEditedByUser.fullName as lastEditedByUser, - publishedByUser.fullName as publishedByUser -FROM charts -LEFT JOIN users lastEditedByUser ON lastEditedByUser.id=charts.lastEditedByUserId -LEFT JOIN users publishedByUser ON publishedByUser.id=charts.publishedByUserId -WHERE ${whereClause} -ORDER BY charts.id DESC -LIMIT 50 -OFFSET ${offset.toString()}` + `-- sql + SELECT + charts.id as id, + chart_configs.full as config, + charts.createdAt as createdAt, + charts.updatedAt as updatedAt, + charts.lastEditedAt as lastEditedAt, + charts.publishedAt as publishedAt, + lastEditedByUser.fullName as lastEditedByUser, + publishedByUser.fullName as publishedByUser + FROM charts + LEFT JOIN chart_configs ON chart_configs.id = charts.configId + LEFT JOIN users lastEditedByUser ON lastEditedByUser.id=charts.lastEditedByUserId + LEFT JOIN users publishedByUser ON publishedByUser.id=charts.publishedByUserId + WHERE ${whereClause} + ORDER BY charts.id DESC + LIMIT 50 + OFFSET ${offset.toString()} + ` ) const results = resultsWithStringGrapherConfigs.map((row: any) => ({ @@ -1133,9 +1203,12 @@ OFFSET ${offset.toString()}` })) const resultCount = await db.knexRaw<{ count: number }>( trx, - `SELECT count(*) as count -FROM charts -WHERE ${whereClause}` + `-- sql + SELECT count(*) as count + FROM charts + JOIN chart_configs ON chart_configs.id = charts.configId + WHERE ${whereClause} + ` ) return { rows: results, numTotalRows: resultCount[0].count } } @@ -1149,10 +1222,17 @@ patchRouteWithRWTransaction( const chartIds = new Set(patchesList.map((patch) => patch.id)) const configsAndIds = await db.knexRaw< - Pick - >(trx, `SELECT id, config FROM charts where id IN (?)`, [ - [...chartIds.values()], - ]) + Pick & { config: DbRawChartConfig["full"] } + >( + trx, + `-- sql + SELECT c.id, cc.full as config + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + WHERE c.id IN (?) + `, + [[...chartIds.values()]] + ) const configMap = new Map( configsAndIds.map((item: any) => [ item.id, @@ -1320,13 +1400,14 @@ getRouteWithROTransaction( const charts = await db.knexRaw( trx, `-- sql - SELECT ${oldChartFieldList} - FROM charts - JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId - LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId - JOIN chart_dimensions cd ON cd.chartId = charts.id - WHERE cd.variableId = ? - GROUP BY charts.id + SELECT ${oldChartFieldList} + FROM charts + JOIN chart_configs ON chart_configs.id = charts.configId + JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId + LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId + JOIN chart_dimensions cd ON cd.chartId = charts.id + WHERE cd.variableId = ? + GROUP BY charts.id `, [variableId] ) @@ -1546,15 +1627,16 @@ getRouteWithROTransaction( const charts = await db.knexRaw( trx, `-- sql - SELECT ${oldChartFieldList} - FROM charts - JOIN chart_dimensions AS cd ON cd.chartId = charts.id - JOIN variables AS v ON cd.variableId = v.id - JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId - LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId - WHERE v.datasetId = ? - GROUP BY charts.id - `, + SELECT ${oldChartFieldList} + FROM charts + JOIN chart_configs ON chart_configs.id = charts.configId + JOIN chart_dimensions AS cd ON cd.chartId = charts.id + JOIN variables AS v ON cd.variableId = v.id + JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId + LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId + WHERE v.datasetId = ? + GROUP BY charts.id + `, [datasetId] ) @@ -1727,16 +1809,18 @@ postRouteWithRWTransaction( if (req.body.republish) { await db.knexRaw( trx, - ` - UPDATE charts - SET config = JSON_SET(config, "$.version", config->"$.version" + 1) - WHERE id IN ( - SELECT DISTINCT chart_dimensions.chartId - FROM chart_dimensions - JOIN variables ON variables.id = chart_dimensions.variableId - WHERE variables.datasetId = ? - ) - `, + `-- sql + UPDATE chart_configs cc + JOIN charts c ON c.configId = cc.id + SET + cc.patch = JSON_SET(cc.patch, "$.version", cc.patch->"$.version" + 1), + cc.full = JSON_SET(cc.full, "$.version", cc.full->"$.version" + 1) + WHERE c.id IN ( + SELECT DISTINCT chart_dimensions.chartId + FROM chart_dimensions + JOIN variables ON variables.id = chart_dimensions.variableId + WHERE variables.datasetId = ? + )`, [datasetId] ) } @@ -1758,9 +1842,16 @@ getRouteWithROTransaction( redirects: await db.knexRaw( trx, `-- sql - SELECT r.id, r.slug, r.chart_id as chartId, JSON_UNQUOTE(JSON_EXTRACT(charts.config, "$.slug")) AS chartSlug - FROM chart_slug_redirects AS r JOIN charts ON charts.id = r.chart_id - ORDER BY r.id DESC` + SELECT + r.id, + r.slug, + r.chart_id as chartId, + JSON_UNQUOTE(JSON_EXTRACT(chart_configs.full, "$.slug")) AS chartSlug + FROM chart_slug_redirects AS r + JOIN charts ON charts.id = r.chart_id + JOIN chart_configs ON chart_configs.id = charts.configId + ORDER BY r.id DESC + ` ), }) ) @@ -1932,14 +2023,15 @@ getRouteWithROTransaction( const charts = await db.knexRaw( trx, `-- sql - SELECT ${oldChartFieldList} FROM charts - LEFT JOIN chart_tags ct ON ct.chartId=charts.id - JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId - LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId - WHERE ct.tagId ${tagId === UNCATEGORIZED_TAG_ID ? "IS NULL" : "= ?"} - GROUP BY charts.id - ORDER BY charts.updatedAt DESC - `, + SELECT ${oldChartFieldList} FROM charts + JOIN chart_configs ON chart_configs.id = charts.configId + LEFT JOIN chart_tags ct ON ct.chartId=charts.id + JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId + LEFT JOIN users publishedByUser ON publishedByUser.id = charts.publishedByUserId + WHERE ct.tagId ${tagId === UNCATEGORIZED_TAG_ID ? "IS NULL" : "= ?"} + GROUP BY charts.id + ORDER BY charts.updatedAt DESC + `, uncategorized ? [] : [tagId] ) tag.charts = charts diff --git a/adminSiteServer/testPageRouter.tsx b/adminSiteServer/testPageRouter.tsx index 5163d3cf385..4f41243620f 100644 --- a/adminSiteServer/testPageRouter.tsx +++ b/adminSiteServer/testPageRouter.tsx @@ -24,12 +24,12 @@ import { import { grapherToSVG } from "../baker/GrapherImageBaker.js" import { ChartTypeName, - ChartsTableName, - DbRawChart, + DbRawChartConfig, + DbPlainChart, EntitySelectionMode, GrapherTabOption, StackMode, - parseChartsRow, + parseChartConfig, } from "@ourworldindata/types" import { ExplorerAdminServer } from "../explorerAdminServer/ExplorerAdminServer.js" import { GIT_CMS_DIR } from "../gitCms/GitCmsConstants.js" @@ -134,27 +134,28 @@ async function propsFromQueryParams( let query = knex .table("charts") - .whereRaw("publishedAt IS NOT NULL") - .orderBy("id", "DESC") + .join({ cc: "chart_configs" }, "charts.configId", "cc.id") + .whereRaw("charts.publishedAt IS NOT NULL") + .orderBy("charts.id", "DESC") console.error(query.toSQL()) let tab = params.tab if (params.type) { if (params.type === ChartTypeName.WorldMap) { - query = query.andWhereRaw(`config->>"$.hasMapTab" = "true"`) + query = query.andWhereRaw(`cc.full->>"$.hasMapTab" = "true"`) tab = tab || GrapherTabOption.map } else { if (params.type === "LineChart") { query = query.andWhereRaw( `( - config->"$.type" = "LineChart" - OR config->"$.type" IS NULL - ) AND COALESCE(config->>"$.hasChartTab", "true") = "true"` + cc.full->"$.type" = "LineChart" + OR cc.full->"$.type" IS NULL + ) AND COALESCE(cc.full->>"$.hasChartTab", "true") = "true"` ) } else { query = query.andWhereRaw( - `config->"$.type" = :type AND COALESCE(config->>"$.hasChartTab", "true") = "true"`, + `cc.full->"$.type" = :type AND COALESCE(cc.full->>"$.hasChartTab", "true") = "true"`, { type: params.type } ) } @@ -164,27 +165,27 @@ async function propsFromQueryParams( if (params.logLinear) { query = query.andWhereRaw( - `config->>'$.yAxis.canChangeScaleType' = "true" OR config->>'$.xAxis.canChangeScaleType' = "true"` + `cc.full->>'$.yAxis.canChangeScaleType' = "true" OR cc.full->>'$.xAxis.canChangeScaleType' = "true"` ) tab = GrapherTabOption.chart } if (params.comparisonLines) { query = query.andWhereRaw( - `config->'$.comparisonLines[0].yEquals' != ''` + `cc.full->'$.comparisonLines[0].yEquals' != ''` ) tab = GrapherTabOption.chart } if (params.stackMode) { - query = query.andWhereRaw(`config->'$.stackMode' = :stackMode`, { + query = query.andWhereRaw(`cc.full->'$.stackMode' = :stackMode`, { stackMode: params.stackMode, }) tab = GrapherTabOption.chart } if (params.relativeToggle) { - query = query.andWhereRaw(`config->>'$.hideRelativeToggle' = "false"`) + query = query.andWhereRaw(`cc.full->>'$.hideRelativeToggle' = "false"`) tab = GrapherTabOption.chart } @@ -193,7 +194,7 @@ async function propsFromQueryParams( // have a visible categorial legend, and can leave out some that have one. // But in practice it seems to work reasonably well. query = query.andWhereRaw( - `json_length(config->'$.map.colorScale.customCategoryColors') > 1` + `json_length(cc.full->'$.map.colorScale.customCategoryColors') > 1` ) tab = GrapherTabOption.map } @@ -219,13 +220,13 @@ async function propsFromQueryParams( const mode = params.addCountryMode if (mode === EntitySelectionMode.MultipleEntities) { query = query.andWhereRaw( - `config->'$.addCountryMode' IS NULL OR config->'$.addCountryMode' = :mode`, + `cc.full->'$.addCountryMode' IS NULL OR cc.full->'$.addCountryMode' = :mode`, { mode: EntitySelectionMode.MultipleEntities, } ) } else { - query = query.andWhereRaw(`config->'$.addCountryMode' = :mode`, { + query = query.andWhereRaw(`cc.full->'$.addCountryMode' = :mode`, { mode, }) } @@ -236,10 +237,10 @@ async function propsFromQueryParams( } if (tab === GrapherTabOption.map) { - query = query.andWhereRaw(`config->>"$.hasMapTab" = "true"`) + query = query.andWhereRaw(`cc.full->>"$.hasMapTab" = "true"`) } else if (tab === GrapherTabOption.chart) { query = query.andWhereRaw( - `COALESCE(config->>"$.hasChartTab", "true") = "true"` + `COALESCE(cc.full->>"$.hasChartTab", "true") = "true"` ) } @@ -277,7 +278,7 @@ async function propsFromQueryParams( const chartsQuery = query .clone() - .select("id", "slug") + .select(knex.raw('charts.id, cc.full ->> "$.slug" as slug')) .limit(perPage) .offset(perPage * (page - 1)) @@ -467,13 +468,26 @@ getPlainRouteWithROTransaction( "/embeds/:id", async (req, res, trx) => { const id = req.params.id - const chartRaw: DbRawChart = await trx - .table(ChartsTableName) - .where({ id: id }) - .first() - const chartEnriched = parseChartsRow(chartRaw) - const viewProps = await getViewPropsFromQueryParams(req.query) - if (chartEnriched) { + const chartRaw = await db.knexRawFirst< + Pick & { config: DbRawChartConfig["full"] } + >( + trx, + `--sql + select ca.id, cc.full as config + from charts ca + join chart_configs cc + on ca.configId = cc.id + where ca.id = ? + `, + [id] + ) + + if (chartRaw) { + const chartEnriched = { + ...chartRaw, + config: parseChartConfig(chartRaw.config), + } + const viewProps = await getViewPropsFromQueryParams(req.query) const charts = [ { id: chartEnriched.id, @@ -639,9 +653,15 @@ getPlainRouteWithROTransaction( testPageRouter, "/previews", async (req, res, trx) => { - const rows = await db.knexRaw( + const rows = await db.knexRaw<{ config: DbRawChartConfig["full"] }>( trx, - `SELECT config FROM charts LIMIT 200` + `--sql + SELECT cc.full as config + FROM charts ca + JOIN chart_configs cc + ON ca.configId = cc.id + LIMIT 200 + ` ) const charts = rows.map((row: any) => JSON.parse(row.config)) @@ -653,9 +673,15 @@ getPlainRouteWithROTransaction( testPageRouter, "/embedVariants", async (req, res, trx) => { - const rows = await db.knexRaw( + const rows = await db.knexRaw<{ config: DbRawChartConfig["full"] }>( trx, - `SELECT config FROM charts WHERE id=64` + `--sql + SELECT cc.full as config + FROM charts ca + JOIN chart_configs cc + ON ca.configId = cc.id + WHERE ca.id=64 + ` ) const charts = rows.map((row: any) => JSON.parse(row.config)) const viewProps = getViewPropsFromQueryParams(req.query) diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index dd8708e8a52..50d16876d0e 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -43,6 +43,8 @@ import { FaqDictionary, ImageMetadata, OwidGdocBaseInterface, + DbPlainChart, + DbRawChartConfig, } from "@ourworldindata/types" import ProgressBar from "progress" import { @@ -482,16 +484,24 @@ export const bakeSingleGrapherChart = async ( export const bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers = // TODO: this transaction is only RW because somewhere inside it we fetch images async (bakedSiteDir: string, knex: db.KnexReadWriteTransaction) => { - const chartsToBake: { id: number; config: string; slug: string }[] = - await knexRaw( - knex, - `-- sql - SELECT - id, config, config->>'$.slug' as slug - FROM charts WHERE JSON_EXTRACT(config, "$.isPublished")=true - ORDER BY JSON_EXTRACT(config, "$.slug") ASC + const chartsToBake = await knexRaw< + Pick & { + config: DbRawChartConfig["full"] + slug: string + } + >( + knex, + `-- sql + SELECT + c.id, + cc.full as config, + cc.full->>'$.slug' as slug + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE JSON_EXTRACT(cc.full, "$.isPublished")=true + ORDER BY JSON_EXTRACT(cc.full, "$.slug") ASC ` - ) + ) const newSlugs = chartsToBake.map((row) => row.slug) await fs.mkdirp(bakedSiteDir + "/grapher") diff --git a/baker/GrapherBakingUtils.ts b/baker/GrapherBakingUtils.ts index f44c67083c0..b7f80eee83c 100644 --- a/baker/GrapherBakingUtils.ts +++ b/baker/GrapherBakingUtils.ts @@ -82,7 +82,12 @@ export const bakeGrapherUrls = async ( const rows = await db.knexRaw<{ version: number }>( knex, - `SELECT charts.config->>"$.version" AS version FROM charts WHERE charts.id=?`, + `-- sql + SELECT cc.full->>"$.version" AS version + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE c.id=? + `, [chartId] ) if (!rows.length) { diff --git a/baker/GrapherImageBaker.tsx b/baker/GrapherImageBaker.tsx index 173ae4449df..4ecbea8d799 100644 --- a/baker/GrapherImageBaker.tsx +++ b/baker/GrapherImageBaker.tsx @@ -1,7 +1,8 @@ import { DbPlainChartSlugRedirect, - DbRawChart, + DbPlainChart, GrapherInterface, + DbRawChartConfig, } from "@ourworldindata/types" import { Grapher, GrapherProgrammaticInterface } from "@ourworldindata/grapher" import { MultipleOwidVariableDataDimensionsMap } from "@ourworldindata/utils" @@ -79,9 +80,16 @@ export async function getPublishedGraphersBySlug( const graphersById: Map = new Map() // Select all graphers that are published - const sql = `SELECT id, config FROM charts WHERE config->>"$.isPublished" = "true"` - - const query = db.knexRaw>(knex, sql) + const sql = `-- sql + SELECT c.id, cc.full as config + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE cc.full ->> "$.isPublished" = 'true' + ` + + const query = db.knexRaw< + Pick & { config: DbRawChartConfig["full"] } + >(knex, sql) for (const row of await query) { const grapher = JSON.parse(row.config) diff --git a/baker/SiteBaker.tsx b/baker/SiteBaker.tsx index 4f9c51798c2..6b4f54a55c9 100644 --- a/baker/SiteBaker.tsx +++ b/baker/SiteBaker.tsx @@ -700,19 +700,21 @@ export class SiteBaker { knex, `-- sql SELECT - config ->> '$.slug' as slug, - config ->> '$.subtitle' as subtitle, - config ->> '$.note' as note + cc.full ->> '$.slug' as slug, + cc.full ->> '$.subtitle' as subtitle, + cc.full ->> '$.note' as note FROM - charts + charts c + JOIN + chart_configs cc ON c.configId = cc.id WHERE - JSON_EXTRACT(config, "$.isPublished") = true + JSON_EXTRACT(cc.full, "$.isPublished") = true AND ( - JSON_EXTRACT(config, "$.subtitle") LIKE "%#dod:%" - OR JSON_EXTRACT(config, "$.note") LIKE "%#dod:%" + JSON_EXTRACT(cc.full, "$.subtitle") LIKE "%#dod:%" + OR JSON_EXTRACT(cc.full, "$.note") LIKE "%#dod:%" ) ORDER BY - JSON_EXTRACT(config, "$.slug") ASC + JSON_EXTRACT(cc.full, "$.slug") ASC ` ) diff --git a/baker/algolia/indexChartsToAlgolia.ts b/baker/algolia/indexChartsToAlgolia.ts index 0813dddf929..360cb41e481 100644 --- a/baker/algolia/indexChartsToAlgolia.ts +++ b/baker/algolia/indexChartsToAlgolia.ts @@ -123,19 +123,20 @@ const getChartsRecords = async ( `-- sql WITH indexable_charts_with_entity_names AS ( SELECT c.id, - config ->> "$.slug" AS slug, - config ->> "$.title" AS title, - config ->> "$.variantName" AS variantName, - config ->> "$.subtitle" AS subtitle, - JSON_LENGTH(config ->> "$.dimensions") AS numDimensions, + cc.full ->> "$.slug" AS slug, + cc.full ->> "$.title" AS title, + cc.full ->> "$.variantName" AS variantName, + cc.full ->> "$.subtitle" AS subtitle, + JSON_LENGTH(cc.full ->> "$.dimensions") AS numDimensions, c.publishedAt, c.updatedAt, JSON_ARRAYAGG(e.name) AS entityNames FROM charts c + LEFT JOIN chart_configs cc ON c.configId = cc.id LEFT JOIN charts_x_entities ce ON c.id = ce.chartId LEFT JOIN entities e ON ce.entityId = e.id - WHERE config ->> "$.isPublished" = 'true' - AND is_indexable IS TRUE + WHERE cc.full ->> "$.isPublished" = 'true' + AND c.is_indexable IS TRUE GROUP BY c.id ) SELECT c.id, diff --git a/baker/countryProfiles.tsx b/baker/countryProfiles.tsx index 0a65489212e..f263d714d36 100644 --- a/baker/countryProfiles.tsx +++ b/baker/countryProfiles.tsx @@ -7,6 +7,8 @@ import { DbEnrichedVariable, VariablesTableName, parseVariablesRow, + DbRawChartConfig, + parseChartConfig, } from "@ourworldindata/types" import * as lodash from "lodash" import { @@ -45,13 +47,20 @@ const countryIndicatorGraphers = async ( trx: db.KnexReadonlyTransaction ): Promise => bakeCache(countryIndicatorGraphers, async () => { - const graphers = ( - await trx - .table("charts") - .whereRaw( - "publishedAt is not null and config->>'$.isPublished' = 'true' and is_indexable is true" - ) - ).map((c: any) => JSON.parse(c.config)) as GrapherInterface[] + const configs = await db.knexRaw<{ config: DbRawChartConfig["full"] }>( + trx, + `-- sql + SELECT cc.full as config + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + WHERE + c.publishedAt is not null + AND cc.full->>'$.isPublished' = 'true' + AND c.is_indexable is true + ` + ) + + const graphers = configs.map((c: any) => parseChartConfig(c.config)) return graphers.filter(checkShouldShowIndicator) }) diff --git a/baker/redirects.ts b/baker/redirects.ts index 3bdfedd75f8..e5f21465124 100644 --- a/baker/redirects.ts +++ b/baker/redirects.ts @@ -88,9 +88,11 @@ export const getGrapherRedirectsMap = async ( }>( knex, `-- sql - SELECT chart_slug_redirects.slug as oldSlug, charts.config ->> "$.slug" as newSlug - FROM chart_slug_redirects INNER JOIN charts ON charts.id=chart_id - ` + SELECT chart_slug_redirects.slug as oldSlug, chart_configs.full ->> "$.slug" as newSlug + FROM chart_slug_redirects + INNER JOIN charts ON charts.id=chart_id + INNER JOIN chart_configs ON chart_configs.id=charts.configId + ` )) as Array<{ oldSlug: string; newSlug: string }> return new Map( diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index 40fc5d4ee18..1a9d9145562 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -50,7 +50,12 @@ import { DbRawPost, } from "@ourworldindata/utils" import { extractFormattingOptions } from "../serverUtils/wordpressUtils.js" -import { FormattingOptions, GrapherInterface } from "@ourworldindata/types" +import { + DbPlainChart, + DbRawChartConfig, + FormattingOptions, + GrapherInterface, +} from "@ourworldindata/types" import { CountryProfileSpec } from "../site/countryProfileProjects.js" import { formatPost } from "./formatWordpressPost.js" import { @@ -106,15 +111,16 @@ export const renderChartsPage = async ( knex, `-- sql SELECT - id, - config->>"$.slug" AS slug, - config->>"$.title" AS title, - config->>"$.variantName" AS variantName - FROM charts + c.id, + cc.full->>"$.slug" AS slug, + cc.full->>"$.title" AS title, + cc.full->>"$.variantName" AS variantName + FROM charts c + JOIN chart_configs cc ON c.configId=cc.id WHERE - is_indexable IS TRUE - AND publishedAt IS NOT NULL - AND config->>"$.isPublished" = "true" + c.is_indexable IS TRUE + AND c.publishedAt IS NOT NULL + AND cc.full->>"$.isPublished" = "true" ` ) @@ -724,9 +730,16 @@ export const renderExplorerPage = async ( type ChartRow = { id: number; config: string } let grapherConfigRows: ChartRow[] = [] if (requiredGrapherIds.length) - grapherConfigRows = await knexRaw( + grapherConfigRows = await knexRaw< + Pick & { config: DbRawChartConfig["full"] } + >( knex, - `SELECT id, config FROM charts WHERE id IN (?)`, + `-- sql + SELECT c.id, cc.full as config + FROM charts c + JOIN chart_configs cc ON c.configId=cc.id + WHERE c.id IN (?) + `, [requiredGrapherIds] ) diff --git a/baker/sitemap.ts b/baker/sitemap.ts index c02c799f866..a5dffcdeffb 100644 --- a/baker/sitemap.ts +++ b/baker/sitemap.ts @@ -7,7 +7,7 @@ import { dayjs, countries, queryParamsToStr, - ChartsTableName, + DbPlainChart, } from "@ourworldindata/utils" import * as db from "../db/db.js" import urljoin from "url-join" @@ -80,13 +80,18 @@ export const makeSitemap = async ( publishedDataInsights.length ) - const charts = (await knex - .table(ChartsTableName) - .select(knex.raw(`updatedAt, config->>"$.slug" AS slug`)) - .whereRaw('config->"$.isPublished" = true')) as { - updatedAt: Date - slug: string - }[] + const charts = await db.knexRaw< + Pick & { slug: string } + >( + knex, + `-- sql + SELECT c.updatedAt, cc.full->>"$.slug" AS slug + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + WHERE + cc.full->"$.isPublished" = true + ` + ) const explorers = await explorerAdminServer.getAllPublishedExplorers() diff --git a/baker/updateChartEntities.ts b/baker/updateChartEntities.ts index 412b80ec361..6589978f6d9 100644 --- a/baker/updateChartEntities.ts +++ b/baker/updateChartEntities.ts @@ -6,13 +6,13 @@ import { Grapher } from "@ourworldindata/grapher" import { - ChartsTableName, ChartsXEntitiesTableName, - DbRawChart, + DbPlainChart, GrapherInterface, GrapherTabOption, MultipleOwidVariableDataDimensionsMap, OwidVariableDataMetadataDimensions, + DbRawChartConfig, } from "@ourworldindata/types" import * as db from "../db/db.js" import pMap from "p-map" @@ -41,13 +41,15 @@ const preFetchCommonVariables = async ( const commonVariables = (await db.knexRaw( trx, `-- sql - SELECT variableId, COUNT(variableId) AS useCount - FROM chart_dimensions cd - JOIN charts c ON cd.chartId = c.id - WHERE config ->> "$.isPublished" = "true" - GROUP BY variableId - ORDER BY COUNT(variableId) DESC - LIMIT ??`, + SELECT variableId, COUNT(variableId) AS useCount + FROM chart_dimensions cd + JOIN charts c ON cd.chartId = c.id + JOIN chart_configs cc ON c.configId = cc.id + WHERE cc.full ->> "$.isPublished" = "true" + GROUP BY variableId + ORDER BY COUNT(variableId) DESC + LIMIT ?? + `, [VARIABLES_TO_PREFETCH] )) as { variableId: number; useCount: number }[] @@ -123,13 +125,17 @@ const obtainAvailableEntitiesForAllGraphers = async ( ) => { const entityNameToIdMap = await mapEntityNamesToEntityIds(trx) - const allPublishedGraphers = (await trx - .select("id", "config") - .from(ChartsTableName) - .whereRaw("config ->> '$.isPublished' = 'true'")) as Pick< - DbRawChart, - "id" | "config" - >[] + const allPublishedGraphers = await db.knexRaw< + Pick & { config: DbRawChartConfig["full"] } + >( + trx, + `-- sql + SELECT c.id, cc.full as config + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE cc.full ->> "$.isPublished" = 'true' + ` + ) const availableEntitiesByChartId = new Map() await pMap( diff --git a/db/db.ts b/db/db.ts index 27185b2e62b..bec9dd9c14d 100644 --- a/db/db.ts +++ b/db/db.ts @@ -292,10 +292,12 @@ export const getTotalNumberOfCharts = ( ): Promise => { return knexRawFirst<{ count: number }>( knex, + `-- sql + SELECT COUNT(*) AS count + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE cc.full ->> "$.isPublished" = "true" ` - SELECT COUNT(*) AS count - FROM charts - WHERE config->"$.isPublished" = TRUE` ).then((res) => res?.count ?? 0) } diff --git a/db/migration/1719842654592-AddChartConfigsTable.ts b/db/migration/1719842654592-AddChartConfigsTable.ts new file mode 100644 index 00000000000..8637ba706bf --- /dev/null +++ b/db/migration/1719842654592-AddChartConfigsTable.ts @@ -0,0 +1,136 @@ +import { MigrationInterface, QueryRunner } from "typeorm" + +export class AddChartConfigsTable1719842654592 implements MigrationInterface { + private async createChartConfigsTable( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query(`-- sql + CREATE TABLE chart_configs ( + id binary(16) NOT NULL DEFAULT (UUID_TO_BIN(UUID(), 1)) PRIMARY KEY, + uuid varchar(36) GENERATED ALWAYS AS (BIN_TO_UUID(id, 1)) STORED, + patch json NOT NULL, + full json NOT NULL, + createdAt datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, + updatedAt datetime DEFAULT NULL ON UPDATE CURRENT_TIMESTAMP, + UNIQUE INDEX idx_chart_configs_uuid (uuid) + ) + `) + } + + private async createConfigIdColumnInChartsTable( + queryRunner: QueryRunner + ): Promise { + // add a new `configId` column to the charts table + // that points to the `chart_configs` table + await queryRunner.query(`-- sql + ALTER TABLE charts + ADD COLUMN configId binary(16) UNIQUE AFTER type, + ADD CONSTRAINT charts_configId + FOREIGN KEY (configId) + REFERENCES chart_configs (id) + ON DELETE RESTRICT + ON UPDATE RESTRICT + `) + } + + private async moveConfigsToChartConfigsTable( + queryRunner: QueryRunner + ): Promise { + // make sure that the config's id matches the table's primary key + await queryRunner.query(`-- sql + UPDATE charts + SET config = JSON_REPLACE(config, '$.id', id) + WHERE id != config ->> "$.id"; + `) + + // insert all the configs into the `chart_configs` table + const configs = await queryRunner.query("SELECT config from charts") + for (const { config } of configs) { + await queryRunner.query( + "INSERT INTO chart_configs (patch, full) VALUES (?, ?)", + [config, config] + ) + } + + // update the `configId` column in the `charts` table + await queryRunner.query(`-- sql + UPDATE charts ca + JOIN chart_configs cc + ON ca.id = JSON_UNQUOTE(JSON_EXTRACT(cc.full, '$.id')) + SET ca.configId = cc.id + `) + + // now that the `configId` column is filled, make it NOT NULL + await queryRunner.query(`-- sql + ALTER TABLE charts + MODIFY COLUMN configId binary(16) NOT NULL; + `) + + // update `createdAt` and `updatedAt` of the chart_configs table + await queryRunner.query(`-- sql + UPDATE chart_configs cc + JOIN charts ca + ON cc.id = ca.configId + SET + cc.createdAt = ca.createdAt, + cc.updatedAt = ca.updatedAt + `) + } + + private async dropConfigColumnFromChartsTable( + queryRunner: QueryRunner + ): Promise { + await queryRunner.query(`-- sql + ALTER TABLE charts + DROP COLUMN slug, + DROP COLUMN type, + DROP COLUMN config + `) + } + + public async up(queryRunner: QueryRunner): Promise { + await this.createChartConfigsTable(queryRunner) + await this.createConfigIdColumnInChartsTable(queryRunner) + await this.moveConfigsToChartConfigsTable(queryRunner) + await this.dropConfigColumnFromChartsTable(queryRunner) + } + + public async down(queryRunner: QueryRunner): Promise { + // add back the config column and its virtual columns + await queryRunner.query(`-- sql + ALTER TABLE charts + ADD COLUMN config JSON AFTER configId, + ADD COLUMN slug VARCHAR(255) GENERATED ALWAYS AS (JSON_UNQUOTE(JSON_EXTRACT(config, '$.slug'))) VIRTUAL AFTER config, + ADD COLUMN type VARCHAR(255) GENERATED ALWAYS AS (COALESCE(JSON_UNQUOTE(JSON_EXTRACT(config, '$.type')), 'LineChart')) VIRTUAL AFTER slug + `) + + await queryRunner.query(`-- sql + CREATE INDEX idx_charts_slug ON charts (slug) + `) + + // recover configs + await queryRunner.query(`-- sql + UPDATE charts c + JOIN chart_configs cc ON c.configId = cc.id + SET c.config = cc.full + `) + + // make the config column NOT NULL + await queryRunner.query(`-- sql + ALTER TABLE charts + MODIFY COLUMN config JSON NOT NULL; + `) + + // drop the `charts.configId` column + await queryRunner.query(`-- sql + ALTER TABLE charts + DROP FOREIGN KEY charts_configId, + DROP COLUMN configId + `) + + // drop the `chart_configs` table + await queryRunner.query(`-- sql + DROP TABLE chart_configs + `) + } +} diff --git a/db/model/Chart.ts b/db/model/Chart.ts index f33950f5dbb..848caa98855 100644 --- a/db/model/Chart.ts +++ b/db/model/Chart.ts @@ -12,12 +12,12 @@ import { ChartTypeName, RelatedChart, DbPlainPostLink, - DbRawChart, - DbEnrichedChart, - parseChartsRow, + DbPlainChart, parseChartConfig, ChartRedirect, DbPlainTag, + DbRawChartConfig, + DbEnrichedChartConfig, } from "@ourworldindata/types" import { OpenAI } from "openai" import { @@ -42,12 +42,11 @@ export async function mapSlugsToIds( const rows = await db.knexRaw<{ id: number; slug: string }>( knex, `-- sql - SELECT - id, - JSON_UNQUOTE(JSON_EXTRACT(config, "$.slug")) AS slug - FROM charts - WHERE config->>"$.isPublished" = "true" -` + SELECT c.id, cc.full ->> "$.slug" AS slug + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + WHERE cc.full ->> "$.isPublished" = "true" + ` ) const slugToId: { [slug: string]: number } = {} @@ -72,16 +71,17 @@ export async function mapSlugsToConfigs( .knexRaw<{ slug: string; config: string; id: number }>( knex, `-- sql -SELECT csr.slug AS slug, c.config AS config, c.id AS id -FROM chart_slug_redirects csr -JOIN charts c -ON csr.chart_id = c.id -WHERE c.config -> "$.isPublished" = true -UNION -SELECT c.slug AS slug, c.config AS config, c.id AS id -FROM charts c -WHERE c.config -> "$.isPublished" = true -` + SELECT csr.slug AS slug, cc.full AS config, c.id AS id + FROM chart_slug_redirects csr + JOIN charts c ON csr.chart_id = c.id + JOIN chart_configs cc ON cc.id = c.configId + WHERE cc.full ->> "$.isPublished" = "true" + UNION + SELECT cc.full ->> "$.slug" AS slug, cc.full AS config, c.id AS id + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + WHERE cc.full ->> "$.isPublished" = "true" + ` ) .then((results) => results.map((result) => ({ @@ -94,31 +94,42 @@ WHERE c.config -> "$.isPublished" = true export async function getEnrichedChartBySlug( knex: db.KnexReadonlyTransaction, slug: string -): Promise { - let chart = await db.knexRawFirst( +): Promise<(DbPlainChart & { config: DbEnrichedChartConfig["full"] }) | null> { + let chart = await db.knexRawFirst< + DbPlainChart & { config: DbRawChartConfig["full"] } + >( knex, - `SELECT * FROM charts WHERE config ->> '$.slug' = ?`, + `-- sql + SELECT c.*, cc.full as config + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE cc.full ->> "$.slug" = ? + `, [slug] ) if (!chart) { - chart = await db.knexRawFirst( + chart = await db.knexRawFirst< + DbPlainChart & { config: DbRawChartConfig["full"] } + >( knex, `-- sql - SELECT - c.* - FROM - chart_slug_redirects csr - JOIN charts c ON csr.chart_id = c.id - WHERE - csr.slug = ?`, + SELECT + c.*, cc.full as config + FROM + chart_slug_redirects csr + JOIN charts c ON csr.chart_id = c.id + JOIN chart_configs cc ON c.configId = cc.id + WHERE + csr.slug = ? + `, [slug] ) } if (!chart) return null - const enrichedChart = parseChartsRow(chart) + const enrichedChart = { ...chart, config: parseChartConfig(chart.config) } return enrichedChart } @@ -126,10 +137,17 @@ export async function getEnrichedChartBySlug( export async function getRawChartById( knex: db.KnexReadonlyTransaction, id: number -): Promise { - const chart = await db.knexRawFirst( +): Promise<(DbPlainChart & { config: DbRawChartConfig["full"] }) | null> { + const chart = await db.knexRawFirst< + DbPlainChart & { config: DbRawChartConfig["full"] } + >( knex, - `SELECT * FROM charts WHERE id = ?`, + `-- sql + SELECT c.*, cc.full AS config + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE id = ? + `, [id] ) if (!chart) return null @@ -139,19 +157,24 @@ export async function getRawChartById( export async function getEnrichedChartById( knex: db.KnexReadonlyTransaction, id: number -): Promise { +): Promise<(DbPlainChart & { config: DbEnrichedChartConfig["full"] }) | null> { const rawChart = await getRawChartById(knex, id) if (!rawChart) return null - return parseChartsRow(rawChart) + return { ...rawChart, config: parseChartConfig(rawChart.config) } } export async function getChartSlugById( knex: db.KnexReadonlyTransaction, id: number ): Promise { - const chart = await db.knexRawFirst>( + const chart = await db.knexRawFirst<{ slug: string }>( knex, - `SELECT config ->> '$.slug' AS slug FROM charts WHERE id = ?`, + `-- sql + SELECT cc.full ->> "$.slug" + FROM chart_configs cc + JOIN charts c ON c.configId = cc.id + WHERE c.id = ? + `, [id] ) if (!chart) return null @@ -161,10 +184,20 @@ export async function getChartSlugById( export const getChartConfigById = async ( knex: db.KnexReadonlyTransaction, grapherId: number -): Promise | undefined> => { - const grapher = await db.knexRawFirst>( +): Promise< + | (Pick & { config: DbEnrichedChartConfig["full"] }) + | undefined +> => { + const grapher = await db.knexRawFirst< + Pick & { config: DbRawChartConfig["full"] } + >( knex, - `SELECT id, config FROM charts WHERE id=?`, + `-- sql + SELECT c.id, cc.full as config + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE c.id=? + `, [grapherId] ) @@ -179,16 +212,24 @@ export const getChartConfigById = async ( export async function getChartConfigBySlug( knex: db.KnexReadonlyTransaction, slug: string -): Promise> { - const row = await db.knexRawFirst>( +): Promise< + Pick & { config: DbEnrichedChartConfig["full"] } +> { + const row = await db.knexRawFirst< + Pick & { config: DbRawChartConfig["full"] } + >( knex, - `SELECT id, config FROM charts WHERE JSON_EXTRACT(config, "$.slug") = ?`, + `-- sql + SELECT c.id, cc.full as config + FROM charts c + JOIN chart_configs cc ON c.configId = cc.id + WHERE cc.full ->> "$.slug" = ?`, [slug] ) if (!row) throw new JsonError(`No chart found for slug ${slug}`, 404) - return { id: row.id, config: JSON.parse(row.config) } + return { id: row.id, config: parseChartConfig(row.config) } } export async function setChartTags( @@ -287,11 +328,16 @@ export async function getGptTopicSuggestions( ): Promise[]> { if (!OPENAI_API_KEY) throw new JsonError("No OPENAI_API_KEY env found", 500) - const chartConfigOnly: Pick | undefined = await knex - .table("charts") - .select("config") - .where({ id: chartId }) - .first() + const chartConfigOnly = await db.knexRawFirst<{ config: string }>( + knex, + `-- sql + SELECT cc.full as config + FROM chart_configs cc + JOIN charts c ON c.configId = cc.id + WHERE c.id = ? + `, + [chartId] + ) if (!chartConfigOnly) throw new JsonError(`No chart found for id ${chartId}`, 404) const enrichedChartConfig = parseChartConfig(chartConfigOnly.config) @@ -380,15 +426,15 @@ export interface OldChartFieldList { export const oldChartFieldList = ` charts.id, - charts.config->>"$.title" AS title, - charts.config->>"$.slug" AS slug, - charts.config->>"$.type" AS type, - charts.config->>"$.internalNotes" AS internalNotes, - charts.config->>"$.variantName" AS variantName, - charts.config->>"$.isPublished" AS isPublished, - charts.config->>"$.tab" AS tab, - JSON_EXTRACT(charts.config, "$.hasChartTab") = true AS hasChartTab, - JSON_EXTRACT(charts.config, "$.hasMapTab") = true AS hasMapTab, + chart_configs.full->>"$.title" AS title, + chart_configs.full->>"$.slug" AS slug, + chart_configs.full->>"$.type" AS type, + chart_configs.full->>"$.internalNotes" AS internalNotes, + chart_configs.full->>"$.variantName" AS variantName, + chart_configs.full->>"$.isPublished" AS isPublished, + chart_configs.full->>"$.tab" AS tab, + JSON_EXTRACT(chart_configs.full, "$.hasChartTab") = true AS hasChartTab, + JSON_EXTRACT(chart_configs.full, "$.hasMapTab") = true AS hasMapTab, charts.lastEditedAt, charts.lastEditedByUserId, lastEditedByUser.fullName AS lastEditedBy, @@ -417,15 +463,18 @@ export const getMostViewedGrapherIdsByChartType = async ( ): Promise => { const ids = await db.knexRaw<{ id: number }>( knex, - `SELECT c.id - FROM analytics_pageviews a - JOIN charts c ON c.slug = SUBSTRING_INDEX(a.url, '/', -1) - WHERE a.url LIKE "https://ourworldindata.org/grapher/%" - AND c.type = ? - AND c.config ->> "$.isPublished" = "true" - and (c.config ->> "$.hasChartTab" = "true" or c.config ->> "$.hasChartTab" is null) - ORDER BY a.views_365d DESC - LIMIT ?`, + `-- sql + SELECT c.id + FROM analytics_pageviews a + JOIN chart_configs cc ON cc.full ->> "$.slug" = SUBSTRING_INDEX(a.url, '/', -1) + JOIN charts c ON c.configId = cc.id + WHERE a.url LIKE "https://ourworldindata.org/grapher/%" + AND cc.full ->> "$.type" = ? + AND cc.full ->> "$.isPublished" = "true" + and (cc.full ->> "$.hasChartTab" = "true" or cc.full ->> "$.hasChartTab" is null) + ORDER BY a.views_365d DESC + LIMIT ? + `, [chartType, count] ) return ids.map((row) => row.id) @@ -444,19 +493,20 @@ export const getRelatedChartsForVariable = async ( return db.knexRaw( knex, `-- sql - SELECT - charts.config->>"$.slug" AS slug, - charts.config->>"$.title" AS title, - charts.config->>"$.variantName" AS variantName, - MAX(chart_tags.keyChartLevel) as keyChartLevel - FROM charts - INNER JOIN chart_tags ON charts.id=chart_tags.chartId - WHERE JSON_CONTAINS(config->'$.dimensions', '{"variableId":${variableId}}') - AND charts.config->>"$.isPublished" = "true" - ${excludeChartIds} - GROUP BY charts.id - ORDER BY title ASC - ` + SELECT + chart_configs.full->>"$.slug" AS slug, + chart_configs.full->>"$.title" AS title, + chart_configs.full->>"$.variantName" AS variantName, + MAX(chart_tags.keyChartLevel) as keyChartLevel + FROM charts + JOIN chart_configs ON charts.configId=chart_configs.id + INNER JOIN chart_tags ON charts.id=chart_tags.chartId + WHERE JSON_CONTAINS(chart_configs.full->'$.dimensions', '{"variableId":${variableId}}') + AND chart_configs.full->>"$.isPublished" = "true" + ${excludeChartIds} + GROUP BY charts.id + ORDER BY title ASC + ` ) } diff --git a/db/model/Gdoc/GdocPost.ts b/db/model/Gdoc/GdocPost.ts index 712625c11d7..420553d3024 100644 --- a/db/model/Gdoc/GdocPost.ts +++ b/db/model/Gdoc/GdocPost.ts @@ -179,17 +179,18 @@ export class GdocPost extends GdocBase implements OwidGdocPostInterface { }>( knex, `-- sql - SELECT DISTINCT - charts.config->>"$.slug" AS slug, - charts.config->>"$.title" AS title, - charts.config->>"$.variantName" AS variantName, - chart_tags.keyChartLevel - FROM charts - INNER JOIN chart_tags ON charts.id=chart_tags.chartId - WHERE chart_tags.tagId IN (?) - AND charts.config->>"$.isPublished" = "true" - ORDER BY title ASC - `, + SELECT DISTINCT + chart_configs.full->>"$.slug" AS slug, + chart_configs.full->>"$.title" AS title, + chart_configs.full->>"$.variantName" AS variantName, + chart_tags.keyChartLevel + FROM charts + JOIN chart_configs ON charts.configId=chart_configs.id + INNER JOIN chart_tags ON charts.id=chart_tags.chartId + WHERE chart_tags.tagId IN (?) + AND chart_configs.full->>"$.isPublished" = "true" + ORDER BY title ASC + `, [this.tags.map((tag) => tag.id)] ) diff --git a/db/model/Post.ts b/db/model/Post.ts index adf1a3e1f7f..9628f47e4c4 100644 --- a/db/model/Post.ts +++ b/db/model/Post.ts @@ -233,15 +233,16 @@ export const getPostRelatedCharts = async ( knex, `-- sql SELECT DISTINCT - charts.config->>"$.slug" AS slug, - charts.config->>"$.title" AS title, - charts.config->>"$.variantName" AS variantName, + chart_configs.full->>"$.slug" AS slug, + chart_configs.full->>"$.title" AS title, + chart_configs.full->>"$.variantName" AS variantName, chart_tags.keyChartLevel FROM charts + JOIN chart_configs ON charts.configId=chart_configs.id INNER JOIN chart_tags ON charts.id=chart_tags.chartId INNER JOIN post_tags ON chart_tags.tagId=post_tags.tag_id WHERE post_tags.post_id=${postId} - AND charts.config->>"$.isPublished" = "true" + AND chart_configs.full->>"$.isPublished" = "true" ORDER BY title ASC ` ) @@ -357,7 +358,8 @@ export const getWordpressPostReferencesByChartId = async ( FROM posts p JOIN posts_links pl ON p.id = pl.sourceId - JOIN charts c ON pl.target = c.slug + JOIN chart_configs cc ON pl.target = cc.full ->> "$.slug" + JOIN charts c ON c.configId = cc.id OR pl.target IN ( SELECT cr.slug @@ -405,7 +407,8 @@ export const getGdocsPostReferencesByChartId = async ( FROM posts_gdocs pg JOIN posts_gdocs_links pgl ON pg.id = pgl.sourceId - JOIN charts c ON pgl.target = c.slug + JOIN chart_configs cc ON pgl.target = cc.full ->> "$.slug" + JOIN charts c ON c.configId = cc.id OR pgl.target IN ( SELECT cr.slug @@ -490,7 +493,7 @@ export const getRelatedResearchAndWritingForVariable = async ( SELECT DISTINCT pl.target AS linkTargetSlug, pl.componentType AS componentType, - COALESCE(csr.slug, c.slug) AS chartSlug, + COALESCE(csr.slug, cc.full ->> "$.slug") AS chartSlug, p.title AS title, p.slug AS postSlug, COALESCE(csr.chart_id, c.id) AS chartId, @@ -510,7 +513,8 @@ export const getRelatedResearchAndWritingForVariable = async ( FROM posts_links pl JOIN posts p ON pl.sourceId = p.id - LEFT JOIN charts c ON pl.target = c.slug + LEFT JOIN chart_configs cc on pl.target = cc.full ->> "$.slug" + LEFT JOIN charts c ON cc.id = c.configId LEFT JOIN chart_slug_redirects csr ON pl.target = csr.slug LEFT JOIN chart_dimensions cd ON cd.chartId = COALESCE(csr.chart_id, c.id) LEFT JOIN analytics_pageviews pv ON pv.url = CONCAT('https://ourworldindata.org/', p.slug) @@ -545,7 +549,7 @@ export const getRelatedResearchAndWritingForVariable = async ( SELECT DISTINCT pl.target AS linkTargetSlug, pl.componentType AS componentType, - COALESCE(csr.slug, c.slug) AS chartSlug, + COALESCE(csr.slug, cc.full ->> "$.slug") AS chartSlug, p.content ->> '$.title' AS title, p.slug AS postSlug, COALESCE(csr.chart_id, c.id) AS chartId, @@ -565,7 +569,8 @@ export const getRelatedResearchAndWritingForVariable = async ( FROM posts_gdocs_links pl JOIN posts_gdocs p ON pl.sourceId = p.id - LEFT JOIN charts c ON pl.target = c.slug + LEFT JOIN chart_configs cc ON pl.target = cc.full ->> "$.slug" + LEFT JOIN charts c ON c.configId = cc.id LEFT JOIN chart_slug_redirects csr ON pl.target = csr.slug JOIN chart_dimensions cd ON cd.chartId = COALESCE(csr.chart_id, c.id) LEFT JOIN analytics_pageviews pv ON pv.url = CONCAT('https://ourworldindata.org/', p.slug) diff --git a/db/model/SuggestedChartRevision.ts b/db/model/SuggestedChartRevision.ts index 928e8ce4a48..936384bc371 100644 --- a/db/model/SuggestedChartRevision.ts +++ b/db/model/SuggestedChartRevision.ts @@ -99,7 +99,7 @@ const selectFields = `scr.id, scr.chartId, scr.updatedAt, scr.createdAt, updatedByUser.id as updatedById, createdByUser.fullName as createdByFullName, updatedByUser.fullName as updatedByFullName, - c.config as existingConfig, c.updatedAt as chartUpdatedAt, + cc.full as existingConfig, c.updatedAt as chartUpdatedAt, c.createdAt as chartCreatedAt ` @@ -118,6 +118,7 @@ export async function getQueryEnrichedSuggestedChartRevisions( SELECT ${selectFields} FROM suggested_chart_revisions as scr LEFT JOIN charts c on c.id = scr.chartId + LEFT JOIN chart_configs cc on cc.id = c.configId LEFT JOIN users createdByUser on createdByUser.id = scr.createdBy LEFT JOIN users updatedByUser on updatedByUser.id = scr.updatedBy ${status ? "WHERE scr.status = ?" : ""} @@ -144,6 +145,7 @@ export async function getQueryEnrichedSuggestedChartRevision( SELECT ${selectFields} FROM suggested_chart_revisions as scr LEFT JOIN charts c on c.id = scr.chartId + LEFT JOIN chart_configs cc on cc.id = c.configId LEFT JOIN users createdByUser on createdByUser.id = scr.createdBy LEFT JOIN users updatedByUser on updatedByUser.id = scr.updatedBy WHERE scr.id = ? diff --git a/db/tests/basic.test.ts b/db/tests/basic.test.ts index 488d72e817a..c3d56e2492f 100644 --- a/db/tests/basic.test.ts +++ b/db/tests/basic.test.ts @@ -13,10 +13,12 @@ import { import { deleteUser, insertUser, updateUser } from "../model/User.js" import { ChartsTableName, + ChartConfigsTableName, DbInsertChart, DbPlainUser, - DbRawChart, + DbPlainChart, UsersTableName, + DbInsertChartConfig, } from "@ourworldindata/types" let knexInstance: Knex | undefined = undefined @@ -65,17 +67,34 @@ function sleep(time: number, value: any): Promise { }) } +const getBinaryUUID = async ( + knex: KnexReadonlyTransaction +): Promise => { + const { id } = (await knexRawFirst<{ id: string }>( + knex, + `SELECT UUID_TO_BIN(UUID(), 1) AS id` + ))! + return id +} + test("timestamps are automatically created and updated", async () => { await knexReadWriteTransaction( async (trx) => { + const configId = await getBinaryUUID(trx) + const chartConfig: DbInsertChartConfig = { + id: configId, + patch: "{}", + full: "{}", + } const chart: DbInsertChart = { - config: "{}", + configId, lastEditedAt: new Date(), lastEditedByUserId: 1, is_indexable: 0, } + await trx.table(ChartConfigsTableName).insert(chartConfig) await trx.table(ChartsTableName).insert(chart) - const created = await knexRawFirst( + const created = await knexRawFirst( trx, "select * from charts where id = 1", [] @@ -89,7 +108,7 @@ test("timestamps are automatically created and updated", async () => { .table(ChartsTableName) .where({ id: 1 }) .update({ is_indexable: 1 }) - const updated = await knexRawFirst( + const updated = await knexRawFirst( trx, "select * from charts where id = 1", [] diff --git a/packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts b/packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts new file mode 100644 index 00000000000..15f2d897ac2 --- /dev/null +++ b/packages/@ourworldindata/types/src/dbTypes/ChartConfigs.ts @@ -0,0 +1,46 @@ +import { JsonString } from "../domainTypes/Various.js" +import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" + +export const ChartConfigsTableName = "chart_configs" +export interface DbInsertChartConfig { + id: string + uuid?: string + patch: JsonString + full: JsonString + createdAt?: Date + updatedAt?: Date | null +} +export type DbRawChartConfig = Required + +export type DbEnrichedChartConfig = Omit & { + patch: GrapherInterface + full: GrapherInterface +} + +export function parseChartConfig(config: JsonString): GrapherInterface { + return JSON.parse(config) +} + +export function serializeChartConfig(config: GrapherInterface): JsonString { + return JSON.stringify(config) +} + +export function parseChartConfigsRow( + row: DbRawChartConfig +): DbEnrichedChartConfig { + return { + ...row, + patch: parseChartConfig(row.patch), + full: parseChartConfig(row.full), + } +} + +export function serializeChartsRow( + row: DbEnrichedChartConfig +): DbRawChartConfig { + return { + ...row, + patch: serializeChartConfig(row.patch), + full: serializeChartConfig(row.full), + } +} diff --git a/packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts b/packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts index 57db30588d3..4d295fb5802 100644 --- a/packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts +++ b/packages/@ourworldindata/types/src/dbTypes/ChartRevisions.ts @@ -1,6 +1,6 @@ import { JsonString } from "../domainTypes/Various.js" import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" -import { parseChartConfig, serializeChartConfig } from "./Charts.js" +import { parseChartConfig, serializeChartConfig } from "./ChartConfigs.js" export const ChartRevisionsTableName = "chart_revisions" export interface DbInsertChartRevision { diff --git a/packages/@ourworldindata/types/src/dbTypes/Charts.ts b/packages/@ourworldindata/types/src/dbTypes/Charts.ts index 4a587a2eae1..844a64f33e6 100644 --- a/packages/@ourworldindata/types/src/dbTypes/Charts.ts +++ b/packages/@ourworldindata/types/src/dbTypes/Charts.ts @@ -1,9 +1,6 @@ -import { JsonString } from "../domainTypes/Various.js" -import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" - export const ChartsTableName = "charts" export interface DbInsertChart { - config: JsonString + configId: string createdAt?: Date id?: number is_indexable?: number @@ -11,31 +8,6 @@ export interface DbInsertChart { lastEditedByUserId: number publishedAt?: Date | null publishedByUserId?: number | null - slug?: string | null - type?: string | null updatedAt?: Date | null } -export type DbRawChart = Required - -export type DbEnrichedChart = Omit & { - config: GrapherInterface -} - -export function parseChartConfig(config: JsonString): GrapherInterface { - return JSON.parse(config) -} - -export function serializeChartConfig(config: GrapherInterface): JsonString { - return JSON.stringify(config) -} - -export function parseChartsRow(row: DbRawChart): DbEnrichedChart { - return { ...row, config: parseChartConfig(row.config) } -} - -export function serializeChartsRow(row: DbEnrichedChart): DbRawChart { - return { - ...row, - config: serializeChartConfig(row.config), - } -} +export type DbPlainChart = Required diff --git a/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts b/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts index 5248fea42c3..c4a3217e695 100644 --- a/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts +++ b/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts @@ -1,6 +1,6 @@ import { JsonString } from "../domainTypes/Various.js" import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" -import { parseChartConfig, serializeChartConfig } from "./Charts.js" +import { parseChartConfig, serializeChartConfig } from "./ChartConfigs.js" export interface SuggestedChartRevisionsExperimental { gpt: { diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index 7dc9ee05c91..530b86fe082 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -413,6 +413,15 @@ export { type DbPlainAnalyticsPageview, AnalyticsPageviewsTableName, } from "./dbTypes/AnalyticsPageviews.js" +export { + type DbInsertChartConfig, + type DbRawChartConfig, + type DbEnrichedChartConfig, + parseChartConfigsRow, + parseChartConfig, + serializeChartConfig, + ChartConfigsTableName, +} from "./dbTypes/ChartConfigs.js" export { type DbPlainChartDimension, type DbInsertChartDimension, @@ -428,13 +437,8 @@ export { } from "./dbTypes/ChartRevisions.js" export { type DbInsertChart, - type DbRawChart, - type DbEnrichedChart, + type DbPlainChart, ChartsTableName, - parseChartConfig, - serializeChartConfig, - parseChartsRow, - serializeChartsRow, } from "./dbTypes/Charts.js" export { type DbPlainChartSlugRedirect,