🔨 (grapher) let chart configs inherit default values / TAS-565

.github/workflows/check-default-grapher-config.yml

@@ -0,0 +1,54 @@
+name: Check default grapher config
+on:
+    push:
+        branches:
+            - "**"
+            - "!master"
+        paths:
+            - "packages/@ourworldindata/grapher/src/schema/**"
+
+jobs:
+    commit-default-config:
+        runs-on: ubuntu-latest
+
+        steps:
+            - name: Clone repository
+              uses: actions/checkout@v4
+              with:
+                  ref: ${{ github.head_ref }}
+
+            - uses: ./.github/actions/setup-node-yarn-deps
+            - uses: ./.github/actions/build-tsc
+
+            - uses: hustcer/setup-nu@v3
+              with:
+                  version: "0.80" # Don't use 0.80 here, as it was a float number and will be convert to 0.8, you can use v0.80/0.80.0 or '0.80'
+
+            # Turn all yaml files in the schema directory into json (should only be one)
+            - name: Convert yaml schema to json
+              run: |
+                  (ls packages/@ourworldindata/grapher/src/schema/*.yaml
+                  | each {|yaml|
+                      open $yaml.name
+                      | to json
+                      | save -f ($yaml.name
+                          | path parse
+                          | upsert extension "json"
+                          | path join) })
+              shell: nu {0}
+
+            # Construct default config objects for all grapher schemas in the schema directory (should only be one)
+            - name: Generate default grapher config
+              run: |
+                  (ls packages/@ourworldindata/grapher/src/schema/grapher-schema.*.json
+                  | each {|json|
+                      node itsJustJavascript/devTools/schema/generate-default-object-from-schema.js $json.name --save-ts packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts })
+              shell: nu {0}
+
+            - name: Run prettier
+              run: yarn fixPrettierAll
+
+            - uses: stefanzweifel/git-auto-commit-action@v5
+              with:
+                  commit_message: "🤖 update default grapher config"
+                  file_pattern: "packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts"

adminSiteClient/GrapherConfigGridEditor.tsx

@@ -347,6 +347,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
             selectedRowContent.id
         )
 
+        // TODO(inheritance): use mergeGrapherConfigs instead
         const mergedConfig = merge(grapherConfig, finalConfigLayer)
         this.loadGrapherJson(mergedConfig)
     }

adminSiteServer/apiRouter.ts

@@ -45,6 +45,8 @@ import {
     Json,
     checkIsGdocPostExcludingFragments,
     checkIsPlainObjectWithGuard,
+    mergeGrapherConfigs,
+    diffGrapherConfigs,
 } from "@ourworldindata/utils"
 import { applyPatch } from "../adminShared/patchHelper.js"
 import {
@@ -86,6 +88,7 @@ import {
 } from "@ourworldindata/types"
 import { uuidv7 } from "uuidv7"
 import {
+    defaultGrapherConfig,
     getVariableDataRoute,
     getVariableMetadataRoute,
 } from "@ourworldindata/grapher"
@@ -269,6 +272,120 @@ const expectChartById = async (
     throw new JsonError(`No chart found for id ${chartId}`, 404)
 }
 
+const saveNewChart = async (
+    knex: db.KnexReadWriteTransaction,
+    { config, user }: { config: GrapherInterface; user: DbPlainUser }
+): Promise<GrapherInterface> => {
+    // if the schema version is missing, assume it's the latest
+    if (!config.$schema) {
+        config.$schema = defaultGrapherConfig.$schema
+    }
+
+    // if isPublished is missing, add it
+    if (!config.isPublished) {
+        config.isPublished = false
+    }
+
+    // compute patch and full configs
+    const parentConfig = defaultGrapherConfig
+    const patchConfig = diffGrapherConfigs(config, parentConfig)
+    const fullConfig = mergeGrapherConfigs(parentConfig, patchConfig)
+
+    // insert patch & full configs into the chart_configs table
+    const configId = uuidv7()
+    await db.knexRaw(
+        knex,
+        `-- sql
+            INSERT INTO chart_configs (id, patch, full)
+            VALUES (?, ?, ?)
+        `,
+        [configId, JSON.stringify(patchConfig), JSON.stringify(fullConfig)]
+    )
+
+    // add a new chart to the charts table
+    const result = await db.knexRawInsert(
+        knex,
+        `-- sql
+            INSERT INTO charts (configId, lastEditedAt, lastEditedByUserId)
+            VALUES (?, ?, ?)
+        `,
+        [configId, new Date(), user.id]
+    )
+
+    // The chart config itself has an id field that should store the id of the chart - update the chart now so this is true
+    const chartId = result.insertId
+    patchConfig.id = chartId
+    fullConfig.id = chartId
+    await db.knexRaw(
+        knex,
+        `-- sql
+            UPDATE chart_configs cc
+            JOIN charts c ON c.configId = cc.id
+            SET
+                cc.patch=JSON_SET(cc.patch, '$.id', ?),
+                cc.full=JSON_SET(cc.full, '$.id', ?)
+            WHERE c.id = ?
+        `,
+        [chartId, chartId, chartId]
+    )
+
+    return patchConfig
+}
+
+const updateExistingChart = async (
+    knex: db.KnexReadWriteTransaction,
+    {
+        config,
+        user,
+        chartId,
+    }: { config: GrapherInterface; user: DbPlainUser; chartId: number }
+): Promise<GrapherInterface> => {
+    // make sure that the id of the incoming config matches the chart id
+    config.id = chartId
+
+    // if the schema version is missing, assume it's the latest
+    if (!config.$schema) {
+        config.$schema = defaultGrapherConfig.$schema
+    }
+
+    // if isPublished is missing, add it
+    if (!config.isPublished) {
+        config.isPublished = false
+    }
+
+    // compute patch and full configs
+    const parentConfig = defaultGrapherConfig
+    const patchConfig = diffGrapherConfigs(config, parentConfig)
+    const fullConfig = mergeGrapherConfigs(parentConfig, patchConfig)
+
+    // update configs
+    await db.knexRaw(
+        knex,
+        `-- sql
+            UPDATE chart_configs cc
+            JOIN charts c ON c.configId = cc.id
+            SET
+                cc.patch=?,
+                cc.full=?
+            WHERE c.id = ?
+        `,
+        [JSON.stringify(patchConfig), JSON.stringify(fullConfig), chartId]
+    )
+
+    // update charts row
+    await db.knexRaw(
+        knex,
+        `-- sql
+            UPDATE charts
+            SET lastEditedAt=?, lastEditedByUserId=?
+            WHERE id = ?
+        `,
+        [new Date(), user.id, chartId]
+    )
+
+    return patchConfig
+}
+
 const saveGrapher = async (
     knex: db.KnexReadWriteTransaction,
     user: DbPlainUser,
@@ -349,68 +466,23 @@ const saveGrapher = async (
     else newConfig.version = 1
 
     // Execute the actual database update or creation
-    const now = new Date()
-    let chartId = existingConfig && existingConfig.id
-    const newJsonConfig = JSON.stringify(newConfig)
+    let chartId: number
     if (existingConfig) {
-        await db.knexRaw(
-            knex,
-            `-- sql
-                UPDATE chart_configs cc
-                JOIN charts c ON c.configId = cc.id
-                SET
-                    cc.patch=?,
-                    cc.full=?
-                WHERE c.id = ?
-            `,
-            [newJsonConfig, newJsonConfig, chartId]
-        )
-        await db.knexRaw(
-            knex,
-            `-- sql
-                UPDATE charts
-                SET lastEditedAt=?, lastEditedByUserId=?
-                WHERE id = ?
-            `,
-            [now, user.id, chartId]
-        )
+        chartId = existingConfig.id!
+        newConfig = await updateExistingChart(knex, {
+            config: newConfig,
+            user,
+            chartId,
+        })
     } else {
-        const configId = uuidv7()
-        await db.knexRaw(
-            knex,
-            `-- sql
-                INSERT INTO chart_configs (id, patch, full)
-                VALUES (?, ?, ?)
-            `,
-            [configId, newJsonConfig, newJsonConfig]
-        )
-        const result = await db.knexRawInsert(
-            knex,
-            `-- sql
-                INSERT INTO charts (configId, lastEditedAt, lastEditedByUserId)
-                VALUES (?, ?, ?)
-            `,
-            [configId, now, user.id]
-        )
-        chartId = result.insertId
-        // The chart config itself has an id field that should store the id of the chart - update the chart now so this is true
-        newConfig.id = chartId
-        await db.knexRaw(
-            knex,
-            `-- sql
-                UPDATE chart_configs cc
-                JOIN charts c ON c.configId = cc.id
-                SET
-                    cc.patch=JSON_SET(cc.patch, '$.id', ?),
-                    cc.full=JSON_SET(cc.full, '$.id', ?)
-                WHERE c.id = ?
-            `,
-            [chartId, chartId, chartId]
-        )
+        newConfig = await saveNewChart(knex, {
+            config: newConfig,
+            user,
+        })
+        chartId = newConfig.id!
     }
 
     // Record this change in version history
-
     const chartRevisionLog = {
         chartId: chartId as number,
         userId: user.id,
@@ -461,7 +533,7 @@ const saveGrapher = async (
         await db.knexRaw(
             knex,
             `UPDATE charts SET publishedAt=?, publishedByUserId=? WHERE id = ? `,
-            [now, user.id, chartId]
+            [new Date(), user.id, chartId]
         )
         await triggerStaticBuild(user, `Publishing chart ${newConfig.slug}`)
     } else if (
@@ -479,7 +551,10 @@ const saveGrapher = async (
     } else if (newConfig.isPublished)
         await triggerStaticBuild(user, `Updating chart ${newConfig.slug}`)
 
-    return chartId
+    return {
+        chartId,
+        savedPatch: newConfig,
+    }
 }
 
 getRouteWithROTransaction(apiRouter, "/charts.json", async (req, res, trx) => {
@@ -756,7 +831,7 @@ apiRouter.get(
 )
 
 postRouteWithRWTransaction(apiRouter, "/charts", async (req, res, trx) => {
-    const chartId = await saveGrapher(trx, res.locals.user, req.body)
+    const { chartId } = await saveGrapher(trx, res.locals.user, req.body)
 
     return { success: true, chartId: chartId }
 })
@@ -779,10 +854,20 @@ putRouteWithRWTransaction(
     async (req, res, trx) => {
         const existingConfig = await expectChartById(trx, req.params.chartId)
 
-        await saveGrapher(trx, res.locals.user, req.body, existingConfig)
+        const { chartId, savedPatch } = await saveGrapher(
+            trx,
+            res.locals.user,
+            req.body,
+            existingConfig
+        )
 
         const logs = await getLogsByChartId(trx, existingConfig.id as number)
-        return { success: true, chartId: existingConfig.id, newLog: logs[0] }
+        return {
+            success: true,
+            chartId,
+            savedPatch,
+            newLog: logs[0],
+        }
     }
 )
 

baker/GrapherBaker.tsx

@@ -142,6 +142,7 @@ export async function renderDataPageV2(
     // this is true for preview pages for datapages on the indicator level but false
     // if we are on Grapher pages. Once we have a good way in the grapher admin for how
     // to use indicator level defaults, we should reconsider how this works here.
+    // TODO(inheritance): use mergeGrapherConfigs instead
     const grapher = useIndicatorGrapherConfigs
         ? mergePartialGrapherConfigs(grapherConfigForVariable, pageGrapher)
         : pageGrapher ?? {}

baker/siteRenderers.tsx

@@ -801,6 +801,7 @@ export const renderExplorerPage = async (
                       config: row.grapherConfigETL as string,
                   })
                 : {}
+            // TODO(inheritance): use mergeGrapherConfigs instead
             return mergePartialGrapherConfigs(etlConfig, adminConfig)
         })