Skip to content

Commit

Permalink
🔨 (grapher) let charts inherit defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiamersmann committed Jul 18, 2024
1 parent a11bd8d commit bdbb4b6
Show file tree
Hide file tree
Showing 28 changed files with 876 additions and 103 deletions.
28 changes: 23 additions & 5 deletions adminSiteClient/ChartEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import {
ChartRedirect,
DimensionProperty,
Json,
GrapherInterface,
diffGrapherConfigs,
isEqual,
omit,
} from "@ourworldindata/utils"
import { computed, observable, runInAction, when } from "mobx"
import { BAKED_GRAPHER_URL } from "../settings/clientSettings.js"
Expand Down Expand Up @@ -97,6 +101,7 @@ export interface ChartEditorManager {
admin: Admin
grapher: Grapher
database: EditorDatabase
baseGrapherConfig: GrapherInterface
logs: Log[]
references: References | undefined
redirects: ChartRedirect[]
Expand Down Expand Up @@ -124,7 +129,7 @@ export class ChartEditor {
@observable.ref errorMessage?: { title: string; content: string }
@observable.ref previewMode: "mobile" | "desktop"
@observable.ref showStaticPreview = false
@observable.ref savedGrapherJson: string = ""
@observable.ref savedPatchConfig: GrapherInterface = {}

// This gets set when we save a new chart for the first time
// so the page knows to update the url
Expand All @@ -138,12 +143,25 @@ export class ChartEditor {
: "desktop"
when(
() => this.grapher.isReady,
() => (this.savedGrapherJson = JSON.stringify(this.grapher.object))
() => (this.savedPatchConfig = this.patchConfig)
)
}

@computed get fullConfig(): GrapherInterface {
return this.grapher.object
}

@computed get patchConfig(): GrapherInterface {
const { baseGrapherConfig } = this.manager
if (!baseGrapherConfig) return this.fullConfig
return diffGrapherConfigs(this.fullConfig, baseGrapherConfig)
}

@computed get isModified(): boolean {
return JSON.stringify(this.grapher.object) !== this.savedGrapherJson
return !isEqual(
omit(this.patchConfig, "version"),
omit(this.savedPatchConfig, "version")
)
}

@computed get grapher() {
Expand Down Expand Up @@ -238,12 +256,12 @@ export class ChartEditor {
if (isNewGrapher) {
this.newChartId = json.chartId
this.grapher.id = json.chartId
this.savedGrapherJson = JSON.stringify(this.grapher.object)
this.savedPatchConfig = json.savedPatch
} else {
runInAction(() => {
grapher.version += 1
this.logs.unshift(json.newLog)
this.savedGrapherJson = JSON.stringify(currentGrapherObject)
this.savedPatchConfig = json.savedPatch
})
}
} else onError?.()
Expand Down
9 changes: 6 additions & 3 deletions adminSiteClient/ChartEditorPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
ChartRedirect,
DimensionProperty,
} from "@ourworldindata/types"
import { Grapher } from "@ourworldindata/grapher"
import { defaultGrapherConfig, Grapher } from "@ourworldindata/grapher"
import { Admin } from "./Admin.js"
import {
ChartEditor,
Expand Down Expand Up @@ -104,7 +104,7 @@ export class ChartEditorPage
extends React.Component<{
grapherId?: number
newGrapherIndex?: number
grapherConfig?: any
grapherConfig?: GrapherInterface
}>
implements ChartEditorManager
{
Expand All @@ -124,7 +124,9 @@ export class ChartEditorPage

@observable simulateVisionDeficiency?: VisionDeficiency

fetchedGrapherConfig?: any
fetchedGrapherConfig?: GrapherInterface
// for now, every chart's previous config layer is the default layer
baseGrapherConfig = defaultGrapherConfig

async fetchGrapher(): Promise<void> {
const { grapherId } = this.props
Expand All @@ -144,6 +146,7 @@ export class ChartEditorPage

@action.bound private updateGrapher(): void {
const config = this.fetchedGrapherConfig ?? this.props.grapherConfig

const grapherConfig = {
...config,
// binds the grapher instance to this.grapher
Expand Down
4 changes: 2 additions & 2 deletions adminSiteClient/EditorHistoryTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> {
@action.bound copyYamlToClipboard() {
// Use the Clipboard API to copy the config into the users clipboard
const chartConfigObject = {
...this.props.editor.grapher.object,
...this.props.editor.patchConfig,
}
delete chartConfigObject.id
delete chartConfigObject.dimensions
Expand All @@ -149,7 +149,7 @@ export class EditorHistoryTab extends React.Component<{ editor: ChartEditor }> {
// Avoid modifying the original JSON object
// Due to mobx memoizing computed values, the JSON can be mutated.
const chartConfigObject = {
...this.props.editor.grapher.object,
...this.props.editor.patchConfig,
}
return (
<div>
Expand Down
1 change: 1 addition & 0 deletions adminSiteClient/GrapherConfigGridEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
selectedRowContent.id
)

// TODO(inheritance): use mergeGrapherConfigs instead
const mergedConfig = merge(grapherConfig, finalConfigLayer)
this.loadGrapherJson(mergedConfig)
}
Expand Down
195 changes: 134 additions & 61 deletions adminSiteServer/apiRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import {
Json,
checkIsGdocPostExcludingFragments,
checkIsPlainObjectWithGuard,
mergeGrapherConfigs,
diffGrapherConfigs,
} from "@ourworldindata/utils"
import { applyPatch } from "../adminShared/patchHelper.js"
import {
Expand Down Expand Up @@ -85,6 +87,7 @@ import {
DbRawChartConfig,
} from "@ourworldindata/types"
import {
defaultGrapherConfig,
getVariableDataRoute,
getVariableMetadataRoute,
} from "@ourworldindata/grapher"
Expand Down Expand Up @@ -268,6 +271,110 @@ 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"]
}

// compute patch and full configs
const baseConfig = defaultGrapherConfig
const patchConfig = diffGrapherConfigs(config, baseConfig)
const fullConfig = mergeGrapherConfigs(baseConfig, patchConfig)

// insert patch & full configs into the chart_configs table
const configId = await db.getBinaryUUID(knex)
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"]
}

// compute patch and full configs
const baseConfig = getBaseLayerConfig()
const patchConfig = diffGrapherConfigs(config, baseConfig)
const fullConfig = mergeGrapherConfigs(baseConfig, 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,
Expand Down Expand Up @@ -348,64 +455,17 @@ 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 = await db.getBinaryUUID(knex)
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
Expand Down Expand Up @@ -460,7 +520,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 (
Expand All @@ -478,7 +538,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) => {
Expand Down Expand Up @@ -755,7 +818,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 }
})
Expand All @@ -778,10 +841,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],
}
}
)

Expand Down
1 change: 1 addition & 0 deletions baker/GrapherBaker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,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 ?? {}
Expand Down
Loading

0 comments on commit bdbb4b6

Please sign in to comment.