Skip to content

Commit

Permalink
🔨 remove default layer from chart configs
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiamersmann committed Nov 1, 2024
1 parent 6567b91 commit f05a6f0
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 53 deletions.
16 changes: 13 additions & 3 deletions adminSiteClient/AbstractChartEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,19 @@ export abstract class AbstractChartEditor<
return mergeGrapherConfigs(parentConfig ?? {}, patchConfig)
}

/** live-updating full config */
/** live-updating config */
@computed get liveConfig(): GrapherInterface {
return this.grapher.object
}

@computed get liveConfigWithDefaults(): GrapherInterface {
return mergeGrapherConfigs(defaultGrapherConfig, this.liveConfig)
}

/** full config: patch config merged with parent config */
@computed get fullConfig(): GrapherInterface {
return mergeGrapherConfigs(defaultGrapherConfig, this.grapher.object)
if (!this.activeParentConfig) return this.liveConfig
return mergeGrapherConfigs(this.activeParentConfig, this.liveConfig)
}

/** parent config currently applied to grapher */
Expand All @@ -103,7 +113,7 @@ export abstract class AbstractChartEditor<
/** patch config of the chart that is written to the db on save */
@computed get patchConfig(): GrapherInterface {
return diffGrapherConfigs(
this.fullConfig,
this.liveConfigWithDefaults,
this.activeParentConfigWithDefaults ?? defaultGrapherConfig
)
}
Expand Down
2 changes: 1 addition & 1 deletion adminSiteClient/ChartEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class ChartEditor extends AbstractChartEditor<ChartEditorManager> {

/** parent variable id, derived from the config */
@computed get parentVariableId(): number | undefined {
return getParentVariableIdFromChartConfig(this.fullConfig)
return getParentVariableIdFromChartConfig(this.liveConfig)
}

@computed get availableTabs(): EditorTab[] {
Expand Down
17 changes: 4 additions & 13 deletions adminSiteServer/apiRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ import {
} from "@ourworldindata/types"
import { uuidv7 } from "uuidv7"
import {
defaultGrapherConfig,
migrateGrapherConfigToLatestVersion,
getVariableDataRoute,
getVariableMetadataRoute,
Expand Down Expand Up @@ -324,14 +323,10 @@ const saveNewChart = async (
const parent = shouldInherit
? await getParentByChartConfig(knex, config)
: undefined
const fullParentConfig = mergeGrapherConfigs(
defaultGrapherConfig,
parent?.config ?? {}
)

// compute patch and full configs
const patchConfig = diffGrapherConfigs(config, fullParentConfig)
const fullConfig = mergeGrapherConfigs(fullParentConfig, patchConfig)
const patchConfig = diffGrapherConfigs(config, parent?.config ?? {})
const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig)
const fullConfigStringified = serializeChartConfig(fullConfig)

// insert patch & full configs into the chart_configs table
Expand Down Expand Up @@ -420,14 +415,10 @@ const updateExistingChart = async (
const parent = shouldInherit
? await getParentByChartConfig(knex, config)
: undefined
const fullParentConfig = mergeGrapherConfigs(
defaultGrapherConfig,
parent?.config ?? {}
)

// compute patch and full configs
const patchConfig = diffGrapherConfigs(config, fullParentConfig)
const fullConfig = mergeGrapherConfigs(fullParentConfig, patchConfig)
const patchConfig = diffGrapherConfigs(config, parent?.config ?? {})
const fullConfig = mergeGrapherConfigs(parent?.config ?? {}, patchConfig)
const fullConfigStringified = serializeChartConfig(fullConfig)

const chartConfigId = await db.knexRawFirst<Pick<DbPlainChart, "configId">>(
Expand Down
116 changes: 116 additions & 0 deletions db/migration/1730455806132-RemoveDefaultConfigLayer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
import { defaultGrapherConfig } from "@ourworldindata/grapher"
import { mergeGrapherConfigs } from "@ourworldindata/utils"
import { MigrationInterface, QueryRunner } from "typeorm"

export class RemoveDefaultConfigLayer1730455806132
implements MigrationInterface
{
private async recomputeFullChartConfigs(
queryRunner: QueryRunner,
{ useDefaultLayer }: { useDefaultLayer: boolean }
): Promise<void> {
// recompute all full configs, without the default layer
const charts = await queryRunner.query(`
-- sql
SELECT
cc.id AS configId,
cc.patch AS patchConfig,
cc_etl.patch AS etlConfig,
cc_admin.patch AS adminConfig,
c.isInheritanceEnabled
FROM charts c
JOIN chart_configs cc ON cc.id = c.configId
LEFT JOIN charts_x_parents cxp ON cxp.chartId = c.id
LEFT JOIN variables v ON v.id = cxp.variableId
LEFT JOIN chart_configs cc_etl ON cc_etl.id = v.grapherConfigIdETL
LEFT JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin
`)
for (const chart of charts) {
const patchConfig = JSON.parse(chart.patchConfig)

const etlConfig = chart.etlConfig
? JSON.parse(chart.etlConfig)
: undefined
const adminConfig = chart.adminConfig
? JSON.parse(chart.adminConfig)
: undefined

const fullConfig = mergeGrapherConfigs(
useDefaultLayer ? defaultGrapherConfig : {},
chart.isInheritanceEnabled ? etlConfig ?? {} : {},
chart.isInheritanceEnabled ? adminConfig ?? {} : {},
patchConfig
)

await queryRunner.query(
`
-- sql
UPDATE chart_configs cc
SET cc.full = ?
WHERE cc.id = ?
`,
[JSON.stringify(fullConfig), chart.configId]
)
}
}

private async updateChartsXParentsView(
queryRunner: QueryRunner
): Promise<void> {
// identical to the current view definition,
// but uses `COALESCE(cc.full ->> '$.type', 'LineChart')`
// instead of `cc.full ->> '$.type'` since the full config
// might not have a type
await queryRunner.query(`-- sql
ALTER VIEW charts_x_parents AS (
WITH y_dimensions AS (
SELECT
*
FROM
chart_dimensions
WHERE
property = 'y'
),
single_y_indicator_charts AS (
SELECT
c.id as chartId,
cc.patch as patchConfig,
-- should only be one
max(yd.variableId) as variableId
FROM
charts c
JOIN chart_configs cc ON cc.id = c.configId
JOIN y_dimensions yd ON c.id = yd.chartId
WHERE
-- scatter plots can't inherit settings
COALESCE(cc.full ->> '$.type', 'LineChart') != 'ScatterPlot'
GROUP BY
c.id
HAVING
-- restrict to single y-variable charts
COUNT(distinct yd.variableId) = 1
)
SELECT
variableId,
chartId
FROM
single_y_indicator_charts
ORDER BY
variableId
)
`)
}

public async up(queryRunner: QueryRunner): Promise<void> {
await this.recomputeFullChartConfigs(queryRunner, {
useDefaultLayer: false,
})
await this.updateChartsXParentsView(queryRunner)
}

public async down(queryRunner: QueryRunner): Promise<void> {
await this.recomputeFullChartConfigs(queryRunner, {
useDefaultLayer: true,
})
}
}
2 changes: 0 additions & 2 deletions db/model/Variable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
import {
getVariableDataRoute,
getVariableMetadataRoute,
defaultGrapherConfig,
} from "@ourworldindata/grapher"
import pl from "nodejs-polars"
import { uuidv7 } from "uuidv7"
Expand Down Expand Up @@ -262,7 +261,6 @@ export async function updateAllChartsThatInheritFromIndicator(

for (const chart of inheritingCharts) {
const fullConfig = mergeGrapherConfigs(
defaultGrapherConfig,
patchConfigETL ?? {},
patchConfigAdmin ?? {},
chart.patchConfig
Expand Down
8 changes: 1 addition & 7 deletions devTools/svgTester/dump-data.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#! /usr/bin/env node

import { getPublishedGraphersBySlug } from "../../baker/GrapherImageBaker.js"
import { defaultGrapherConfig } from "@ourworldindata/grapher"
import { diffGrapherConfigs } from "@ourworldindata/utils"

import { TransactionCloseMode, knexReadonlyTransaction } from "../../db/db.js"

Expand All @@ -26,11 +24,7 @@ async function main(parsedArgs: parseArgs.ParsedArgs) {
const allGraphers = [...graphersBySlug.values()]
const saveJobs: utils.SaveGrapherSchemaAndDataJob[] = allGraphers.map(
(grapher) => {
// since we're not baking defaults, we also exclude them here
return {
config: diffGrapherConfigs(grapher, defaultGrapherConfig),
outDir,
}
return { config: grapher, outDir }
}
)

Expand Down
2 changes: 1 addition & 1 deletion packages/@ourworldindata/grapher/src/schema/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ The schema is versioned. There is one yaml file with a version number. For nonbr
edit the yaml file as is. A github action will then generate a .latest.yaml and two json files (one .latest.json and one with the version number.json)
and upload them to S3 so they can be accessed publicly.

If you update the default value of an existing property or you add a new property with a default value, make sure to regenerate the default object from the schema and save it to `defaultGrapherConfig.ts` (see below). You should also write a migration to update the `chart_configs.full` column in the database for all stand-alone charts.
If you update the default value of an existing property or you add a new property with a default value, make sure to regenerate the default object from the schema and save it to `defaultGrapherConfig.ts` (see below).

Breaking changes should be done by renaming the schema file to an increased version number. Make sure to also rename the authorative url
inside the schema file (the "$id" field at the top level) to point to the new version number json. Then write the migrations from the last to
Expand Down
2 changes: 1 addition & 1 deletion packages/@ourworldindata/utils/src/Util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1968,7 +1968,7 @@ export function traverseObjects<T extends Record<string, any>>(
}

export function getParentVariableIdFromChartConfig(
config: GrapherInterface // could be a patch config
config: GrapherInterface
): number | undefined {
const { type = ChartTypeName.LineChart, dimensions } = config

Expand Down
9 changes: 1 addition & 8 deletions site/DataPageV2.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
defaultGrapherConfig,
getVariableDataRoute,
getVariableMetadataRoute,
GrapherProgrammaticInterface,
Expand Down Expand Up @@ -105,12 +104,6 @@ export const DataPageV2 = (props: {
dataApiUrl: DATA_API_URL,
}

// We bake the Grapher config without defaults
const grapherConfigToBake = diffGrapherConfigs(
grapherConfig,
defaultGrapherConfig
)

// Only embed the tags that are actually used by the datapage, instead of the complete JSON object with ~240 properties
const minimalTagToSlugMap = pick(
tagToSlugMap,
Expand Down Expand Up @@ -190,7 +183,7 @@ export const DataPageV2 = (props: {
<script
dangerouslySetInnerHTML={{
__html: `window._OWID_GRAPHER_CONFIG = ${serializeJSONForHTML(
grapherConfigToBake
grapherConfig
)}`,
}}
/>
Expand Down
13 changes: 2 additions & 11 deletions site/ExplorerPage.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
defaultGrapherConfig,
GRAPHER_PAGE_BODY_CLASS,
LoadingIndicator,
} from "@ourworldindata/grapher"
Expand Down Expand Up @@ -82,24 +81,16 @@ export const ExplorerPage = (props: ExplorerPageSettings) => {
/>
) : undefined

// We bake the given Grapher configs without defaults
const grapherConfigsToBake = grapherConfigs.map((config) =>
diffGrapherConfigs(config, defaultGrapherConfig)
)
const partialGrapherConfigsToBake = partialGrapherConfigs.map((config) =>
diffGrapherConfigs(config, defaultGrapherConfig)
)

const inlineJs = `const explorerProgram = ${serializeJSONForHTML(
program.toJson(),
EMBEDDED_EXPLORER_DELIMITER
)};
const grapherConfigs = ${serializeJSONForHTML(
grapherConfigsToBake,
grapherConfigs,
EMBEDDED_EXPLORER_GRAPHER_CONFIGS
)};
const partialGrapherConfigs = ${serializeJSONForHTML(
partialGrapherConfigsToBake,
partialGrapherConfigs,
EMBEDDED_EXPLORER_PARTIAL_GRAPHER_CONFIGS
)};
const urlMigrationSpec = ${
Expand Down
7 changes: 1 addition & 6 deletions site/GrapherPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
getVariableMetadataRoute,
GRAPHER_PAGE_BODY_CLASS,
LoadingIndicator,
defaultGrapherConfig,
} from "@ourworldindata/grapher"
import {
PostReference,
Expand All @@ -13,7 +12,6 @@ import {
uniq,
SiteFooterContext,
Url,
diffGrapherConfigs,
} from "@ourworldindata/utils"
import { MarkdownTextWrap } from "@ourworldindata/components"
import React from "react"
Expand Down Expand Up @@ -66,11 +64,8 @@ export const GrapherPage = (props: {
const imageWidth = "1200"
const imageHeight = "628"

// We bake the Grapher config without defaults
const grapherToBake = diffGrapherConfigs(grapher, defaultGrapherConfig)

const script = `const jsonConfig = ${serializeJSONForHTML({
...grapherToBake,
...grapher,
adminBaseUrl: ADMIN_BASE_URL,
bakedGrapherURL: BAKED_GRAPHER_URL,
dataApiUrl: DATA_API_URL,
Expand Down

0 comments on commit f05a6f0

Please sign in to comment.