From cbdb44016181ea45823772ccd3dab1c6a50438a9 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Fri, 19 Jul 2024 10:09:16 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=89=20(grapher)=20make=20charts=20inhe?= =?UTF-8?q?rit=20indicator-level=20settings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/AbstractChartEditor.ts | 99 ++++ adminSiteClient/AdminApp.tsx | 12 + adminSiteClient/ChartEditor.ts | 199 ++----- adminSiteClient/ChartEditorContext.ts | 7 + adminSiteClient/ChartEditorPage.tsx | 510 ++-------------- adminSiteClient/ChartEditorView.tsx | 553 ++++++++++++++++++ adminSiteClient/DimensionCard.tsx | 9 +- adminSiteClient/EditorBasicTab.tsx | 43 +- adminSiteClient/EditorCustomizeTab.tsx | 30 +- adminSiteClient/EditorDataTab.tsx | 10 +- adminSiteClient/EditorExportTab.tsx | 12 +- adminSiteClient/EditorFeatures.tsx | 6 +- adminSiteClient/EditorMapTab.tsx | 6 +- adminSiteClient/EditorReferencesTab.tsx | 7 +- adminSiteClient/EditorTextTab.tsx | 38 +- adminSiteClient/GrapherConfigGridEditor.tsx | 7 +- adminSiteClient/IndicatorChartEditor.ts | 39 ++ adminSiteClient/IndicatorChartEditorPage.tsx | 72 +++ adminSiteClient/SaveButtons.tsx | 65 +- adminSiteClient/VariableSelector.tsx | 19 +- adminSiteServer/apiRouter.ts | 241 ++++++-- baker/GrapherBaker.tsx | 9 +- baker/siteRenderers.tsx | 16 +- ...veIndicatorChartsToTheChartConfigsTable.ts | 184 ++++++ db/model/Chart.ts | 67 ++- db/model/ChartConfigs.ts | 68 +++ db/model/Variable.ts | 334 ++++++++++- packages/@ourworldindata/grapher/src/index.ts | 1 + .../src/schema/grapher-schema.004.yaml | 3 +- .../types/src/dbTypes/InheritingCharts.ts | 8 + .../types/src/dbTypes/Variables.ts | 43 +- packages/@ourworldindata/types/src/index.ts | 4 - packages/@ourworldindata/utils/src/Util.ts | 27 +- .../utils/src/grapherConfigUtils.ts | 11 +- packages/@ourworldindata/utils/src/index.ts | 2 +- site/DataPageV2.tsx | 7 +- 36 files changed, 1939 insertions(+), 829 deletions(-) create mode 100644 adminSiteClient/AbstractChartEditor.ts create mode 100644 adminSiteClient/ChartEditorContext.ts create mode 100644 adminSiteClient/ChartEditorView.tsx create mode 100644 adminSiteClient/IndicatorChartEditor.ts create mode 100644 adminSiteClient/IndicatorChartEditorPage.tsx create mode 100644 db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts create mode 100644 db/model/ChartConfigs.ts create mode 100644 packages/@ourworldindata/types/src/dbTypes/InheritingCharts.ts diff --git a/adminSiteClient/AbstractChartEditor.ts b/adminSiteClient/AbstractChartEditor.ts new file mode 100644 index 00000000000..7d8bba7e312 --- /dev/null +++ b/adminSiteClient/AbstractChartEditor.ts @@ -0,0 +1,99 @@ +import { + isEqual, + omit, + GrapherInterface, + diffGrapherConfigs, + mergeGrapherConfigs, +} from "@ourworldindata/utils" +import { computed, observable, when } from "mobx" +import { EditorFeatures } from "./EditorFeatures.js" +import { Admin } from "./Admin.js" +import { defaultGrapherConfig, Grapher } from "@ourworldindata/grapher" + +export type EditorTab = + | "basic" + | "data" + | "text" + | "customize" + | "map" + | "scatter" + | "marimekko" + | "revisions" + | "refs" + | "export" + +export interface AbstractChartEditorManager { + admin: Admin + patchConfig: GrapherInterface + parentConfig?: GrapherInterface +} + +export abstract class AbstractChartEditor< + Manager extends AbstractChartEditorManager = AbstractChartEditorManager, +> { + manager: Manager + + @observable.ref grapher = new Grapher() + @observable.ref currentRequest: Promise | undefined // Whether the current chart state is saved or not + @observable.ref tab: EditorTab = "basic" + @observable.ref errorMessage?: { title: string; content: string } + @observable.ref previewMode: "mobile" | "desktop" + @observable.ref showStaticPreview = false + @observable.ref savedPatchConfig: GrapherInterface = {} + @observable.ref parentConfig: GrapherInterface | undefined = undefined + + constructor(props: { manager: Manager }) { + this.manager = props.manager + this.previewMode = + localStorage.getItem("editorPreviewMode") === "mobile" + ? "mobile" + : "desktop" + + when( + () => this.manager.parentConfig !== undefined, + () => (this.parentConfig = this.manager.parentConfig) + ) + + when( + () => this.grapher.isReady, + () => (this.savedPatchConfig = this.patchConfig) + ) + } + + @computed get grapherConfig(): GrapherInterface { + const { patchConfig, parentConfig } = this.manager + if (!parentConfig) return patchConfig + return mergeGrapherConfigs(parentConfig, patchConfig) + } + + @computed get liveConfig(): GrapherInterface { + return this.grapher.object + } + + @computed get patchConfig(): GrapherInterface { + const { liveConfig, parentConfig } = this + if (!parentConfig) return liveConfig + // TODO: explain + const liveConfigWithDefaults = mergeGrapherConfigs( + defaultGrapherConfig, + liveConfig + ) + return diffGrapherConfigs(liveConfigWithDefaults, parentConfig) + } + + @computed get isModified(): boolean { + return !isEqual( + omit(this.patchConfig, "version"), + omit(this.savedPatchConfig, "version") + ) + } + + @computed get features(): EditorFeatures { + return new EditorFeatures(this) + } + + abstract get isNewGrapher(): boolean + abstract get availableTabs(): EditorTab[] + + abstract saveGrapher(props?: { onError?: () => void }): Promise +} diff --git a/adminSiteClient/AdminApp.tsx b/adminSiteClient/AdminApp.tsx index 56804458a34..014ae1609f1 100644 --- a/adminSiteClient/AdminApp.tsx +++ b/adminSiteClient/AdminApp.tsx @@ -41,6 +41,7 @@ import { BulkGrapherConfigEditorPage } from "./BulkGrapherConfigEditor.js" import { GdocsIndexPage, GdocsMatchProps } from "./GdocsIndexPage.js" import { GdocsPreviewPage } from "./GdocsPreviewPage.js" import { GdocsStoreProvider } from "./GdocsStore.js" +import { IndicatorChartEditorPage } from "./IndicatorChartEditorPage.js" @observer class AdminErrorMessage extends React.Component<{ admin: Admin }> { @@ -154,6 +155,17 @@ export class AdminApp extends React.Component<{ path="/charts" component={ChartIndexPage} /> + ( + + )} + /> { ) } -export interface Namespace { - name: string - description?: string - isArchived: boolean -} - -// This contains the dataset/variable metadata for the entire database -// Used for variable selector interface - -export interface NamespaceData { - datasets: Dataset[] -} - -export class EditorDatabase { - @observable.ref namespaces: Namespace[] - @observable.ref variableUsageCounts: Map = new Map() - @observable dataByNamespace: Map = new Map() - - constructor(json: any) { - this.namespaces = json.namespaces - } -} - -export type FieldWithDetailReferences = - | "subtitle" - | "note" - | "axisLabelX" - | "axisLabelY" - -export type DetailReferences = Record - -export interface DimensionErrorMessage { - displayName?: string -} - -export interface ChartEditorManager { - admin: Admin - grapher: Grapher - database: EditorDatabase - parentGrapherConfig: GrapherInterface +export interface ChartEditorManager extends AbstractChartEditorManager { logs: Log[] references: References | undefined redirects: ChartRedirect[] pageviews?: RawPageview allTopics: Topic[] - details: DetailDictionary - invalidDetailReferences: DetailReferences - errorMessages: Partial> - errorMessagesForDimensions: Record< - DimensionProperty, - DimensionErrorMessage[] - > -} - -interface VariableIdUsageRecord { - variableId: number - usageCount: number } -export class ChartEditor { - manager: ChartEditorManager - // Whether the current chart state is saved or not - @observable.ref currentRequest: Promise | undefined - @observable.ref tab: EditorTab = "basic" - @observable.ref errorMessage?: { title: string; content: string } - @observable.ref previewMode: "mobile" | "desktop" - @observable.ref showStaticPreview = false - @observable.ref savedPatchConfig: GrapherInterface = {} - +export class ChartEditor extends AbstractChartEditor { // This gets set when we save a new chart for the first time // so the page knows to update the url @observable.ref newChartId?: number - constructor(props: { manager: ChartEditorManager }) { - this.manager = props.manager - this.previewMode = - localStorage.getItem("editorPreviewMode") === "mobile" - ? "mobile" - : "desktop" - when( - () => this.grapher.isReady, - () => (this.savedPatchConfig = this.patchConfig) - ) - } - - @computed get fullConfig(): GrapherInterface { - return this.grapher.object - } - - @computed get patchConfig(): GrapherInterface { - const { parentGrapherConfig } = this.manager - if (!parentGrapherConfig) return this.fullConfig - return diffGrapherConfigs(this.fullConfig, parentGrapherConfig) - } - - @computed get isModified(): boolean { - return !isEqual( - omit(this.patchConfig, "version"), - omit(this.savedPatchConfig, "version") - ) - } - - @computed get grapher() { - return this.manager.grapher - } - - @computed get database() { - return this.manager.database - } - @computed get logs() { return this.manager.logs } @@ -192,10 +79,6 @@ export class ChartEditor { return this.manager.allTopics } - @computed get details() { - return this.manager.details - } - @computed get availableTabs(): EditorTab[] { const tabs: EditorTab[] = ["basic", "data", "text", "customize"] if (this.grapher.hasMapTab) tabs.push("map") @@ -211,36 +94,47 @@ export class ChartEditor { return this.grapher.id === undefined } - @computed get features() { - return new EditorFeatures(this) + async fetchParentConfig(indicatorId: number): Promise { + const indicatorChart = await this.manager.admin.getJSON( + `/api/variables/mergedGrapherConfig/${indicatorId}.json` + ) + return mergeGrapherConfigs(defaultGrapherConfig, indicatorChart) } - async loadVariableUsageCounts(): Promise { - const data = (await this.manager.admin.getJSON( - `/api/variables.usages.json` - )) as VariableIdUsageRecord[] - const finalData = new Map( - data.map(({ variableId, usageCount }: VariableIdUsageRecord) => [ - variableId, - +usageCount, - ]) + @action.bound async updateParentConfig() { + const newIndicatorId = getParentIndicatorIdFromChartConfig( + this.liveConfig ) - runInAction(() => (this.database.variableUsageCounts = finalData)) + const currentIndicatorId = this.parentConfig?.dimensions?.[0].variableId + if (newIndicatorId !== currentIndicatorId) { + this.parentConfig = newIndicatorId + ? await this.fetchParentConfig(newIndicatorId) + : undefined + } + + //TODO: important not to use this.patchConfig here + const patchConfig = diffGrapherConfigs( + this.liveConfig, + this.parentConfig ?? {} + ) + const config = mergeGrapherConfigs(this.parentConfig ?? {}, patchConfig) + + this.grapher.updateFromObject(config) + // TODO: what does this do? is this necessary? + this.grapher.updateAuthoredVersion({ + ...this.grapher.toObject(), + }) } async saveGrapher({ onError, }: { onError?: () => void } = {}): Promise { - const { grapher, isNewGrapher } = this - const currentGrapherObject = this.grapher.object + const { grapher, isNewGrapher, patchConfig } = this // Chart title and slug may be autocalculated from data, in which case they won't be in props // But the server will need to know what we calculated in order to do its job - if (!currentGrapherObject.title) - currentGrapherObject.title = grapher.displayTitle - - if (!currentGrapherObject.slug) - currentGrapherObject.slug = grapher.displaySlug + if (!patchConfig.title) patchConfig.title = grapher.displayTitle + if (!patchConfig.slug) patchConfig.slug = grapher.displaySlug const targetUrl = isNewGrapher ? "/api/charts" @@ -248,7 +142,7 @@ export class ChartEditor { const json = await this.manager.admin.requestJSON( targetUrl, - currentGrapherObject, + patchConfig, isNewGrapher ? "POST" : "PUT" ) @@ -268,9 +162,9 @@ export class ChartEditor { } async saveAsNewGrapher(): Promise { - const currentGrapherObject = this.grapher.object + const { patchConfig } = this - const chartJson = { ...currentGrapherObject } + const chartJson = { ...patchConfig } delete chartJson.id delete chartJson.isPublished @@ -312,3 +206,8 @@ export class ChartEditor { } } } +export function isChartEditorInstance( + editor: AbstractChartEditor +): editor is ChartEditor { + return editor instanceof ChartEditor +} diff --git a/adminSiteClient/ChartEditorContext.ts b/adminSiteClient/ChartEditorContext.ts new file mode 100644 index 00000000000..2d56f6930b2 --- /dev/null +++ b/adminSiteClient/ChartEditorContext.ts @@ -0,0 +1,7 @@ +import { DimensionProperty } from "@ourworldindata/types" +import React from "react" + +export const ChartEditorContext = React.createContext({ + errorMessages: {}, + errorMessagesForDimensions: {}, +}) diff --git a/adminSiteClient/ChartEditorPage.tsx b/adminSiteClient/ChartEditorPage.tsx index b16eeac4229..e6108add8d0 100644 --- a/adminSiteClient/ChartEditorPage.tsx +++ b/adminSiteClient/ChartEditorPage.tsx @@ -1,190 +1,75 @@ import React from "react" import { observer } from "mobx-react" +import { observable, computed, runInAction, action } from "mobx" import { - observable, - computed, - runInAction, - autorun, - action, - reaction, - IReactionDisposer, -} from "mobx" -import { Prompt, Redirect } from "react-router-dom" -import { - Bounds, - capitalize, + getParentIndicatorIdFromChartConfig, + isEmpty, + mergeGrapherConfigs, RawPageview, - DetailDictionary, - get, - set, - groupBy, - extractDetailsFromSyntax, - getIndexableKeys, } from "@ourworldindata/utils" -import { - Topic, - GrapherInterface, - GrapherStaticFormat, - ChartRedirect, - DimensionProperty, -} from "@ourworldindata/types" -import { defaultGrapherConfig, Grapher } from "@ourworldindata/grapher" +import { Topic, GrapherInterface, ChartRedirect } from "@ourworldindata/types" +import { defaultGrapherConfig } from "@ourworldindata/grapher" import { Admin } from "./Admin.js" import { ChartEditor, - EditorDatabase, Log, References, ChartEditorManager, - Dataset, - getFullReferencesCount, - DetailReferences, - FieldWithDetailReferences, } from "./ChartEditor.js" -import { EditorBasicTab } from "./EditorBasicTab.js" -import { EditorDataTab } from "./EditorDataTab.js" -import { EditorTextTab } from "./EditorTextTab.js" -import { EditorCustomizeTab } from "./EditorCustomizeTab.js" -import { EditorScatterTab } from "./EditorScatterTab.js" -import { EditorMapTab } from "./EditorMapTab.js" -import { EditorHistoryTab } from "./EditorHistoryTab.js" -import { EditorReferencesTab } from "./EditorReferencesTab.js" -import { SaveButtons } from "./SaveButtons.js" -import { LoadingBlocker } from "./Forms.js" -import { AdminLayout } from "./AdminLayout.js" import { AdminAppContext, AdminAppContextType } from "./AdminAppContext.js" -import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" -import { faMobile, faDesktop } from "@fortawesome/free-solid-svg-icons" -import { - VisionDeficiency, - VisionDeficiencySvgFilters, - VisionDeficiencyDropdown, - VisionDeficiencyEntity, -} from "./VisionDeficiencies.js" -import { EditorMarimekkoTab } from "./EditorMarimekkoTab.js" -import { EditorExportTab } from "./EditorExportTab.js" -import { runDetailsOnDemand } from "../site/detailsOnDemand.js" - -@observer -class TabBinder extends React.Component<{ editor: ChartEditor }> { - dispose!: IReactionDisposer - componentDidMount(): void { - //window.addEventListener("hashchange", this.onHashChange) - this.onHashChange() +import { ChartEditorView, ChartEditorViewManager } from "./ChartEditorView.js" - this.dispose = autorun(() => { - //setTimeout(() => window.location.hash = `#${tab}-tab`, 100) - }) - } - - componentWillUnmount(): void { - //window.removeEventListener("hashchange", this.onHashChange) - this.dispose() - } - - render(): null { - return null - } - - @action.bound onHashChange(): void { - const match = window.location.hash.match(/#(.+?)-tab/) - if (match) { - const tab = match[1] - if ( - this.props.editor.grapher && - this.props.editor.availableTabs.includes(tab) - ) - this.props.editor.tab = tab - } - } -} +// TODO: what happens to parent config if indicator IDs are updated? @observer export class ChartEditorPage extends React.Component<{ grapherId?: number - newGrapherIndex?: number grapherConfig?: GrapherInterface }> - implements ChartEditorManager + implements ChartEditorManager, ChartEditorViewManager { - @observable.ref grapher = new Grapher() - @observable.ref database = new EditorDatabase({}) @observable logs: Log[] = [] @observable references: References | undefined = undefined @observable redirects: ChartRedirect[] = [] @observable pageviews?: RawPageview = undefined @observable allTopics: Topic[] = [] - @observable details: DetailDictionary = {} - - @observable.ref grapherElement?: React.ReactElement static contextType = AdminAppContext context!: AdminAppContextType - @observable simulateVisionDeficiency?: VisionDeficiency - - fetchedGrapherConfig?: GrapherInterface - // for now, every chart's parent config is the default layer - parentGrapherConfig = defaultGrapherConfig + patchConfig: GrapherInterface = {} + parentConfig: GrapherInterface | undefined = undefined - async fetchGrapher(): Promise { - const { grapherId } = this.props + async fetchGrapherConfigs(): Promise { + const { grapherId, grapherConfig } = this.props if (grapherId !== undefined) { - this.fetchedGrapherConfig = await this.context.admin.getJSON( - `/api/charts/${grapherId}.config.json` + this.patchConfig = await this.context.admin.getJSON( + `/api/charts/${grapherId}.patchConfig.json` ) + } else if (grapherConfig) { + this.patchConfig = grapherConfig } - this.updateGrapher() - } - @observable private _isDbSet = false - @observable private _isGrapherSet = false - @computed get isReady(): boolean { - return this._isDbSet && this._isGrapherSet - } - - @action.bound private updateGrapher(): void { - const config = this.fetchedGrapherConfig ?? this.props.grapherConfig - const grapherConfig = { - ...config, - // binds the grapher instance to this.grapher - getGrapherInstance: (grapher: Grapher) => { - this.grapher = grapher - }, - dataApiUrlForAdmin: - this.context.admin.settings.DATA_API_FOR_ADMIN_UI, // passed this way because clientSettings are baked and need a recompile to be updated - bounds: this.bounds, - staticFormat: this.staticFormat, - } - this.grapher.renderToStatic = !!this.editor?.showStaticPreview - this.grapherElement = - this._isGrapherSet = true - } - - @action.bound private setDb(json: any): void { - this.database = new EditorDatabase(json) - this._isDbSet = true - } - - async fetchData(): Promise { - const { admin } = this.context - - const [namespaces, variables] = await Promise.all([ - admin.getJSON(`/api/editorData/namespaces.json`), - admin.getJSON(`/api/editorData/variables.json`), - ]) - - this.setDb(namespaces) - - const groupedByNamespace = groupBy( - variables.datasets, - (d) => d.namespace - ) - for (const namespace in groupedByNamespace) { - this.database.dataByNamespace.set(namespace, { - datasets: groupedByNamespace[namespace] as Dataset[], - }) + if (grapherId !== undefined) { + const parentConfig = await this.context.admin.getJSON( + `/api/charts/${grapherId}.parentConfig.json` + ) + if (!isEmpty(parentConfig)) { + this.parentConfig = parentConfig + } + } else if (grapherConfig) { + const parentIndicatorId = + getParentIndicatorIdFromChartConfig(grapherConfig) + if (parentIndicatorId) { + const indicatorChart = await this.context.admin.getJSON( + `/api/variables/mergedGrapherConfig/${parentIndicatorId}.json` + ) + this.parentConfig = mergeGrapherConfigs( + defaultGrapherConfig, + indicatorChart + ) + } } } @@ -236,136 +121,16 @@ export class ChartEditorPage runInAction(() => (this.allTopics = json.topics)) } - async fetchDetails(): Promise { - await runDetailsOnDemand() - - runInAction(() => { - if (window.details) this.details = window.details - }) - } - - @computed private get isMobilePreview(): boolean { - return this.editor?.previewMode === "mobile" - } - - @computed private get bounds(): Bounds { - return this.isMobilePreview - ? new Bounds(0, 0, 380, 525) - : this.grapher.defaultBounds - } - - @computed private get staticFormat(): GrapherStaticFormat { - return this.isMobilePreview - ? GrapherStaticFormat.square - : GrapherStaticFormat.landscape - } - - // unvalidated terms extracted from the subtitle and note fields - // these may point to non-existent details e.g. ["not_a_real_term", "pvotery"] - @computed - get currentDetailReferences(): DetailReferences { - return { - subtitle: extractDetailsFromSyntax(this.grapher.currentSubtitle), - note: extractDetailsFromSyntax(this.grapher.note), - axisLabelX: extractDetailsFromSyntax( - this.grapher.xAxisConfig.label ?? "" - ), - axisLabelY: extractDetailsFromSyntax( - this.grapher.yAxisConfig.label ?? "" - ), - } - } - - // the actual Detail objects, indexed by category.term - @computed get currentlyReferencedDetails(): GrapherInterface["details"] { - const grapherConfigDetails: GrapherInterface["details"] = {} - const allReferences = Object.values(this.currentDetailReferences).flat() - - allReferences.forEach((term) => { - const detail = get(this.details, term) - if (detail) { - set(grapherConfigDetails, term, detail) - } - }) - - return grapherConfigDetails - } - - @computed - get invalidDetailReferences(): ChartEditorManager["invalidDetailReferences"] { - const { subtitle, note, axisLabelX, axisLabelY } = - this.currentDetailReferences - return { - subtitle: subtitle.filter((term) => !this.details[term]), - note: note.filter((term) => !this.details[term]), - axisLabelX: axisLabelX.filter((term) => !this.details[term]), - axisLabelY: axisLabelY.filter((term) => !this.details[term]), - } - } - - @computed get errorMessages(): ChartEditorManager["errorMessages"] { - const { invalidDetailReferences } = this - - const errorMessages: ChartEditorManager["errorMessages"] = {} - - // add error messages for each field with invalid detail references - getIndexableKeys(invalidDetailReferences).forEach( - (key: FieldWithDetailReferences) => { - const references = invalidDetailReferences[key] - if (references.length) { - errorMessages[key] = - `Invalid detail(s) specified: ${references.join(", ")}` - } - } - ) - - return errorMessages - } - - @computed - get errorMessagesForDimensions(): ChartEditorManager["errorMessagesForDimensions"] { - const errorMessages: ChartEditorManager["errorMessagesForDimensions"] = - { - [DimensionProperty.y]: [], - [DimensionProperty.x]: [], - [DimensionProperty.color]: [], - [DimensionProperty.size]: [], - [DimensionProperty.table]: [], // not used - } - - this.grapher.dimensionSlots.forEach((slot) => { - slot.dimensions.forEach((dimension, dimensionIndex) => { - const details = extractDetailsFromSyntax( - dimension.display.name ?? "" - ) - const hasDetailsInDisplayName = details.length > 0 - - // add error message if details are referenced in the display name - if (hasDetailsInDisplayName) { - errorMessages[slot.property][dimensionIndex] = { - displayName: "Detail syntax is not supported", - } - } - }) - }) - - return errorMessages - } - @computed get admin(): Admin { return this.context.admin } - @computed get editor(): ChartEditor | undefined { - if (!this.isReady) return undefined - + @computed get editor(): ChartEditor { return new ChartEditor({ manager: this }) } @action.bound refresh(): void { - void this.fetchGrapher() - void this.fetchDetails() - void this.fetchData() + void this.fetchGrapherConfigs() void this.fetchLogs() void this.fetchRefs() void this.fetchRedirects() @@ -376,207 +141,18 @@ export class ChartEditorPage // this.fetchTopics() } - disposers: IReactionDisposer[] = [] - componentDidMount(): void { this.refresh() - - this.disposers.push( - reaction( - () => this.editor && this.editor.previewMode, - () => { - if (this.editor) { - localStorage.setItem( - "editorPreviewMode", - this.editor.previewMode - ) - } - this.updateGrapher() - } - ) - ) } + // TODO(inheritance) // This funny construction allows the "new chart" link to work by forcing an update // even if the props don't change - UNSAFE_componentWillReceiveProps(): void { - setTimeout(() => this.refresh(), 0) - } - - componentWillUnmount(): void { - this.disposers.forEach((dispose) => dispose()) - } + // UNSAFE_componentWillReceiveProps(): void { + // setTimeout(() => this.refresh(), 0) + // } render(): React.ReactElement { - return ( - -
- {(this.editor === undefined || - this.editor.currentRequest) && } - {this.editor !== undefined && this.renderReady(this.editor)} -
- - ) - } - - renderReady(editor: ChartEditor): React.ReactElement { - const { grapher, availableTabs } = editor - - return ( - - {!editor.newChartId && ( - - )} - {editor.newChartId && ( - - )} - -
- -
- {editor.tab === "basic" && ( - - )} - {editor.tab === "text" && ( - - )} - {editor.tab === "data" && ( - - )} - {editor.tab === "customize" && ( - - )} - {editor.tab === "scatter" && ( - - )} - {editor.tab === "marimekko" && ( - - )} - {editor.tab === "map" && ( - - )} - {editor.tab === "revisions" && ( - - )} - {editor.tab === "refs" && ( - - )} - {editor.tab === "export" && ( - - )} -
- {editor.tab !== "export" && } -
-
-
- {this.grapherElement} -
-
-
- - -
-
- Emulate vision deficiency:{" "} - - (this.simulateVisionDeficiency = - option.deficiency) - )} - /> -
-
- - {/* Include svg filters necessary for vision deficiency emulation */} - -
- - ) + return } } diff --git a/adminSiteClient/ChartEditorView.tsx b/adminSiteClient/ChartEditorView.tsx new file mode 100644 index 00000000000..b6342555515 --- /dev/null +++ b/adminSiteClient/ChartEditorView.tsx @@ -0,0 +1,553 @@ +import React from "react" +import { observer } from "mobx-react" +import { + observable, + computed, + runInAction, + action, + reaction, + IReactionDisposer, +} from "mobx" +import { Prompt, Redirect } from "react-router-dom" +import { + Bounds, + capitalize, + DetailDictionary, + get, + set, + groupBy, + extractDetailsFromSyntax, + getIndexableKeys, +} from "@ourworldindata/utils" +import { + GrapherInterface, + GrapherStaticFormat, + DimensionProperty, +} from "@ourworldindata/types" +import { Grapher } from "@ourworldindata/grapher" +import { Admin } from "./Admin.js" +import { getFullReferencesCount, isChartEditorInstance } from "./ChartEditor.js" +import { EditorBasicTab } from "./EditorBasicTab.js" +import { EditorDataTab } from "./EditorDataTab.js" +import { EditorTextTab } from "./EditorTextTab.js" +import { EditorCustomizeTab } from "./EditorCustomizeTab.js" +import { EditorScatterTab } from "./EditorScatterTab.js" +import { EditorMapTab } from "./EditorMapTab.js" +import { EditorHistoryTab } from "./EditorHistoryTab.js" +import { EditorReferencesTab } from "./EditorReferencesTab.js" +import { + SaveButtonsForChart, + SaveButtonsForIndicatorChart, +} from "./SaveButtons.js" +import { LoadingBlocker } from "./Forms.js" +import { AdminLayout } from "./AdminLayout.js" +import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" +import { faMobile, faDesktop } from "@fortawesome/free-solid-svg-icons" +import { + VisionDeficiency, + VisionDeficiencySvgFilters, + VisionDeficiencyDropdown, + VisionDeficiencyEntity, +} from "./VisionDeficiencies.js" +import { EditorMarimekkoTab } from "./EditorMarimekkoTab.js" +import { EditorExportTab } from "./EditorExportTab.js" +import { runDetailsOnDemand } from "../site/detailsOnDemand.js" +import { ChartEditorContext } from "./ChartEditorContext.js" +import { AbstractChartEditor } from "./AbstractChartEditor.js" +import { isIndicatorChartEditorInstance } from "./IndicatorChartEditor.js" + +interface Variable { + id: number + name: string +} + +export interface Dataset { + id: number + name: string + namespace: string + version: string | undefined + variables: Variable[] + isPrivate: boolean + nonRedistributable: boolean +} + +export interface Namespace { + name: string + description?: string + isArchived: boolean +} + +// This contains the dataset/variable metadata for the entire database +// Used for variable selector interface +export interface NamespaceData { + datasets: Dataset[] +} + +export class EditorDatabase { + @observable.ref namespaces: Namespace[] + @observable.ref variableUsageCounts: Map = new Map() + @observable dataByNamespace: Map = new Map() + + constructor(json: any) { + this.namespaces = json.namespaces + } +} + +export interface DimensionErrorMessage { + displayName?: string +} + +export type FieldWithDetailReferences = + | "subtitle" + | "note" + | "axisLabelX" + | "axisLabelY" + +export type DetailReferences = Record + +export interface ChartEditorViewManager { + admin: Admin + editor: Editor +} + +@observer +export class ChartEditorView< + Editor extends AbstractChartEditor, +> extends React.Component<{ + manager: ChartEditorViewManager +}> { + @observable.ref database = new EditorDatabase({}) + @observable details: DetailDictionary = {} + @observable.ref grapherElement?: React.ReactElement + + @observable simulateVisionDeficiency?: VisionDeficiency + + @computed private get manager(): ChartEditorViewManager { + return this.props.manager + } + + @observable private _isDbSet = false + @observable private _isGrapherSet = false + @computed get isReady(): boolean { + return this._isDbSet && this._isGrapherSet + } + + @action.bound private updateGrapher(): void { + const config = this.manager.editor.grapherConfig + const grapherConfig = { + ...config, + // binds the grapher instance to this.grapher + getGrapherInstance: (grapher: Grapher) => { + this.manager.editor.grapher = grapher + }, + dataApiUrlForAdmin: + this.manager.admin.settings.DATA_API_FOR_ADMIN_UI, // passed this way because clientSettings are baked and need a recompile to be updated + bounds: this.bounds, + staticFormat: this.staticFormat, + } + this.manager.editor.grapher.renderToStatic = + !!this.editor?.showStaticPreview + this.grapherElement = + this._isGrapherSet = true + } + + @action.bound private setDb(json: any): void { + this.database = new EditorDatabase(json) + this._isDbSet = true + } + + async fetchData(): Promise { + const { admin } = this.manager + + const [namespaces, variables] = await Promise.all([ + admin.getJSON(`/api/editorData/namespaces.json`), + admin.getJSON(`/api/editorData/variables.json`), + ]) + + this.setDb(namespaces) + + const groupedByNamespace = groupBy( + variables.datasets, + (d) => d.namespace + ) + for (const namespace in groupedByNamespace) { + this.database.dataByNamespace.set(namespace, { + datasets: groupedByNamespace[namespace] as Dataset[], + }) + } + + const usageData = (await admin.getJSON( + `/api/variables.usages.json` + )) as { + variableId: number + usageCount: number + }[] + this.database.variableUsageCounts = new Map( + usageData.map(({ variableId, usageCount }) => [ + variableId, + +usageCount, + ]) + ) + } + + async fetchDetails(): Promise { + await runDetailsOnDemand() + + runInAction(() => { + if (window.details) this.details = window.details + }) + } + + @computed private get isMobilePreview(): boolean { + return this.editor?.previewMode === "mobile" + } + + @computed private get bounds(): Bounds { + return this.isMobilePreview + ? new Bounds(0, 0, 380, 525) + : this.manager.editor.grapher.defaultBounds + } + + @computed private get staticFormat(): GrapherStaticFormat { + return this.isMobilePreview + ? GrapherStaticFormat.square + : GrapherStaticFormat.landscape + } + + // unvalidated terms extracted from the subtitle and note fields + // these may point to non-existent details e.g. ["not_a_real_term", "pvotery"] + @computed + get currentDetailReferences(): DetailReferences { + const { grapher } = this.manager.editor + return { + subtitle: extractDetailsFromSyntax(grapher.currentSubtitle), + note: extractDetailsFromSyntax(grapher.note), + axisLabelX: extractDetailsFromSyntax( + grapher.xAxisConfig.label ?? "" + ), + axisLabelY: extractDetailsFromSyntax( + grapher.yAxisConfig.label ?? "" + ), + } + } + + // the actual Detail objects, indexed by category.term + @computed get currentlyReferencedDetails(): GrapherInterface["details"] { + const grapherConfigDetails: GrapherInterface["details"] = {} + const allReferences = Object.values(this.currentDetailReferences).flat() + + allReferences.forEach((term) => { + const detail = get(this.details, term) + if (detail) { + set(grapherConfigDetails, term, detail) + } + }) + + return grapherConfigDetails + } + + @computed + get invalidDetailReferences(): DetailReferences { + const { subtitle, note, axisLabelX, axisLabelY } = + this.currentDetailReferences + return { + subtitle: subtitle.filter((term) => !this.details[term]), + note: note.filter((term) => !this.details[term]), + axisLabelX: axisLabelX.filter((term) => !this.details[term]), + axisLabelY: axisLabelY.filter((term) => !this.details[term]), + } + } + + @computed get errorMessages(): Partial< + Record + > { + const { invalidDetailReferences } = this + + const errorMessages: Partial< + Record + > = {} + + // add error messages for each field with invalid detail references + getIndexableKeys(invalidDetailReferences).forEach( + (key: FieldWithDetailReferences) => { + const references = invalidDetailReferences[key] + if (references.length) { + errorMessages[key] = + `Invalid detail(s) specified: ${references.join(", ")}` + } + } + ) + + return errorMessages + } + + @computed + get errorMessagesForDimensions(): Record< + DimensionProperty, + DimensionErrorMessage[] + > { + const errorMessages: Record< + DimensionProperty, + DimensionErrorMessage[] + > = { + [DimensionProperty.y]: [], + [DimensionProperty.x]: [], + [DimensionProperty.color]: [], + [DimensionProperty.size]: [], + [DimensionProperty.table]: [], // not used + } + + this.manager.editor.grapher.dimensionSlots.forEach((slot) => { + slot.dimensions.forEach((dimension, dimensionIndex) => { + const details = extractDetailsFromSyntax( + dimension.display.name ?? "" + ) + const hasDetailsInDisplayName = details.length > 0 + + // add error message if details are referenced in the display name + if (hasDetailsInDisplayName) { + errorMessages[slot.property][dimensionIndex] = { + displayName: "Detail syntax is not supported", + } + } + }) + }) + + return errorMessages + } + + @computed get editor(): Editor | undefined { + if (!this.isReady) return undefined + + return this.manager.editor + } + + @action.bound refresh(): void { + void this.fetchDetails() + void this.fetchData() + } + + componentDidMount(): void { + this.refresh() + this.updateGrapher() + + this.disposers.push( + reaction( + () => this.editor && this.editor.previewMode, + () => { + if (this.editor) { + localStorage.setItem( + "editorPreviewMode", + this.editor.previewMode + ) + } + this.updateGrapher() + } + ) + ) + } + + // TODO(inheritance) + // This funny construction allows the "new chart" link to work by forcing an update + // even if the props don't change + // UNSAFE_componentWillReceiveProps(): void { + // setTimeout(() => this.refresh(), 0) + // } + + disposers: IReactionDisposer[] = [] + componentWillUnmount(): void { + this.disposers.forEach((dispose) => dispose()) + } + + render(): React.ReactElement { + return ( + +
+ {(this.editor === undefined || + this.editor.currentRequest) && } + {this.editor !== undefined && this.renderReady(this.editor)} +
+ + ) + } + + renderReady(editor: Editor): React.ReactElement { + const { grapher, availableTabs } = editor + + const chartEditor = isChartEditorInstance(editor) ? editor : undefined + const indicatorChartEditor = isIndicatorChartEditorInstance(editor) + ? editor + : undefined + + const saveButtons = chartEditor ? ( + + ) : indicatorChartEditor ? ( + + ) : null + + return ( + + {!editor.isNewGrapher && ( + + )} + {chartEditor?.newChartId && ( + + )} +
+ +
+ {editor.tab === "basic" && ( + + )} + {editor.tab === "text" && ( + + )} + {editor.tab === "data" && ( + + )} + {editor.tab === "customize" && ( + + )} + {editor.tab === "scatter" && ( + + )} + {editor.tab === "marimekko" && ( + + )} + {editor.tab === "map" && ( + + )} + {chartEditor && chartEditor.tab === "revisions" && ( + + )} + {chartEditor && chartEditor.tab === "refs" && ( + + )} + {editor.tab === "export" && ( + + )} +
+ {editor.tab !== "export" && saveButtons} +
+
+
+ {this.grapherElement} +
+
+
+ + +
+
+ Emulate vision deficiency:{" "} + + (this.simulateVisionDeficiency = + option.deficiency) + )} + /> +
+
+ + {/* Include svg filters necessary for vision deficiency emulation */} + +
+ + ) + } +} diff --git a/adminSiteClient/DimensionCard.tsx b/adminSiteClient/DimensionCard.tsx index cbb6ab2d9a0..d45e7193d73 100644 --- a/adminSiteClient/DimensionCard.tsx +++ b/adminSiteClient/DimensionCard.tsx @@ -4,7 +4,7 @@ import { observer } from "mobx-react" import { ChartDimension } from "@ourworldindata/grapher" import { OwidVariableRoundingMode } from "@ourworldindata/types" import { startCase } from "@ourworldindata/utils" -import { ChartEditor, DimensionErrorMessage } from "./ChartEditor.js" +import { DimensionErrorMessage } from "./ChartEditorView.js" import { Toggle, BindAutoString, @@ -22,11 +22,14 @@ import { } from "@fortawesome/free-solid-svg-icons" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" import { OwidTable } from "@ourworldindata/core-table" +import { AbstractChartEditor } from "./AbstractChartEditor.js" @observer -export class DimensionCard extends React.Component<{ +export class DimensionCard< + Editor extends AbstractChartEditor, +> extends React.Component<{ dimension: ChartDimension - editor: ChartEditor + editor: Editor isDndEnabled?: boolean onChange: (dimension: ChartDimension) => void onEdit?: () => void diff --git a/adminSiteClient/EditorBasicTab.tsx b/adminSiteClient/EditorBasicTab.tsx index c2e4994e689..22effbadefc 100644 --- a/adminSiteClient/EditorBasicTab.tsx +++ b/adminSiteClient/EditorBasicTab.tsx @@ -30,7 +30,6 @@ import { } from "@ourworldindata/utils" import { FieldsRow, Section, SelectField, Toggle } from "./Forms.js" import { VariableSelector } from "./VariableSelector.js" -import { ChartEditor, ChartEditorManager } from "./ChartEditor.js" import { DimensionCard } from "./DimensionCard.js" import { DragDropContext, @@ -38,12 +37,21 @@ import { Draggable, DropResult, } from "react-beautiful-dnd" +import { ChartEditorContext } from "./ChartEditorContext.js" +import { AbstractChartEditor } from "./AbstractChartEditor.js" +import { EditorDatabase } from "./ChartEditorView.js" +import { isChartEditorInstance } from "./ChartEditor.js" @observer -class DimensionSlotView extends React.Component<{ +class DimensionSlotView< + Editor extends AbstractChartEditor, +> extends React.Component<{ slot: DimensionSlot - editor: ChartEditor + editor: Editor + database: EditorDatabase }> { + static contextType = ChartEditorContext + disposers: IReactionDisposer[] = [] @observable.ref isSelectingVariables: boolean = false @@ -53,8 +61,8 @@ class DimensionSlotView extends React.Component<{ } @computed - get errorMessages(): ChartEditorManager["errorMessagesForDimensions"] { - return this.props.editor.manager.errorMessagesForDimensions + get errorMessages() { + return this.context.errorMessagesForDimensions } @action.bound private onAddVariables(variableIds: OwidVariableId[]) { @@ -75,6 +83,7 @@ class DimensionSlotView extends React.Component<{ this.isSelectingVariables = false this.updateDimensionsAndRebuildTable(dimensionConfigs) + this.updateParentConfig() } @action.bound private onRemoveDimension(variableId: OwidVariableId) { @@ -83,10 +92,12 @@ class DimensionSlotView extends React.Component<{ (d) => d.variableId !== variableId ) ) + this.updateParentConfig() } @action.bound private onChangeDimension() { this.updateDimensionsAndRebuildTable() + this.updateParentConfig() } @action.bound private updateDefaults() { @@ -165,6 +176,12 @@ class DimensionSlotView extends React.Component<{ this.grapher.rebuildInputOwidTable() } + @action.bound private updateParentConfig() { + if (isChartEditorInstance(this.props.editor)) { + this.props.editor.updateParentConfig() + } + } + @action.bound private onDragEnd(result: DropResult) { const { source, destination } = result if (!destination) return @@ -176,6 +193,7 @@ class DimensionSlotView extends React.Component<{ ) this.updateDimensionsAndRebuildTable(dimensions) + this.updateParentConfig() } @computed get isDndEnabled() { @@ -267,6 +285,7 @@ class DimensionSlotView extends React.Component<{ {isSelectingVariables && ( (this.isSelectingVariables = false) @@ -280,7 +299,9 @@ class DimensionSlotView extends React.Component<{ } @observer -class VariablesSection extends React.Component<{ editor: ChartEditor }> { +class VariablesSection< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor; database: EditorDatabase }> { base: React.RefObject = React.createRef() @observable.ref isAddingVariable: boolean = false @@ -296,6 +317,7 @@ class VariablesSection extends React.Component<{ editor: ChartEditor }> { key={slot.name} slot={slot} editor={props.editor} + database={props.database} /> ))} @@ -305,7 +327,9 @@ class VariablesSection extends React.Component<{ editor: ChartEditor }> { } @observer -export class EditorBasicTab extends React.Component<{ editor: ChartEditor }> { +export class EditorBasicTab< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor; database: EditorDatabase }> { @action.bound onChartTypeChange(value: string) { const { grapher } = this.props.editor grapher.type = value as ChartTypeName @@ -371,7 +395,10 @@ export class EditorBasicTab extends React.Component<{ editor: ChartEditor }> { /> - + ) } diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx index 7cc973c0820..20f56b51571 100644 --- a/adminSiteClient/EditorCustomizeTab.tsx +++ b/adminSiteClient/EditorCustomizeTab.tsx @@ -1,7 +1,6 @@ import React from "react" import { observable, computed, action } from "mobx" import { observer } from "mobx-react" -import { ChartEditor } from "./ChartEditor.js" import { ComparisonLineConfig, ColorSchemeName, @@ -37,6 +36,8 @@ import { } from "./ColorSchemeDropdown.js" import { EditorColorScaleSection } from "./EditorColorScaleSection.js" import Select from "react-select" +import { ChartEditorContext } from "./ChartEditorContext.js" +import { AbstractChartEditor } from "./AbstractChartEditor.js" @observer export class ColorSchemeSelector extends React.Component<{ grapher: Grapher }> { @@ -103,7 +104,9 @@ interface SortOrderDropdownOption { } @observer -class SortOrderSection extends React.Component<{ editor: ChartEditor }> { +class SortOrderSection< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor }> { @computed get sortConfig(): SortConfig { return this.grapher._sortConfig } @@ -203,7 +206,9 @@ class SortOrderSection extends React.Component<{ editor: ChartEditor }> { } @observer -class FacetSection extends React.Component<{ editor: ChartEditor }> { +class FacetSection extends React.Component<{ + editor: Editor +}> { base: React.RefObject = React.createRef() @computed get grapher() { @@ -280,7 +285,9 @@ class FacetSection extends React.Component<{ editor: ChartEditor }> { } @observer -class TimelineSection extends React.Component<{ editor: ChartEditor }> { +class TimelineSection< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor }> { base: React.RefObject = React.createRef() @computed get grapher() { @@ -387,7 +394,9 @@ class TimelineSection extends React.Component<{ editor: ChartEditor }> { } @observer -class ComparisonLineSection extends React.Component<{ editor: ChartEditor }> { +class ComparisonLineSection< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor }> { @observable comparisonLines: ComparisonLineConfig[] = [] @action.bound onAddComparisonLine() { @@ -445,11 +454,16 @@ class ComparisonLineSection extends React.Component<{ editor: ChartEditor }> { } @observer -export class EditorCustomizeTab extends React.Component<{ - editor: ChartEditor +export class EditorCustomizeTab< + Editor extends AbstractChartEditor, +> extends React.Component<{ + editor: Editor }> { + // TODO + static contextType = ChartEditorContext + @computed get errorMessages() { - return this.props.editor.manager.errorMessages + return this.context.errorMessages } render() { diff --git a/adminSiteClient/EditorDataTab.tsx b/adminSiteClient/EditorDataTab.tsx index 14a81edf41b..af28179a3b3 100644 --- a/adminSiteClient/EditorDataTab.tsx +++ b/adminSiteClient/EditorDataTab.tsx @@ -9,7 +9,6 @@ import { } from "@ourworldindata/types" import { Grapher } from "@ourworldindata/grapher" import { ColorBox, SelectField, Section } from "./Forms.js" -import { ChartEditor } from "./ChartEditor.js" import { faArrowsAltV, faTimes } from "@fortawesome/free-solid-svg-icons" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" import { @@ -18,6 +17,7 @@ import { Droppable, DropResult, } from "react-beautiful-dnd" +import { AbstractChartEditor } from "./AbstractChartEditor.js" interface EntityItemProps extends React.HTMLProps { grapher: Grapher @@ -164,7 +164,9 @@ export class KeysSection extends React.Component<{ grapher: Grapher }> { } @observer -class MissingDataSection extends React.Component<{ editor: ChartEditor }> { +class MissingDataSection< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor }> { @computed get grapher() { return this.props.editor.grapher } @@ -208,7 +210,9 @@ class MissingDataSection extends React.Component<{ editor: ChartEditor }> { } @observer -export class EditorDataTab extends React.Component<{ editor: ChartEditor }> { +export class EditorDataTab< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor }> { render() { const { editor } = this.props const { grapher, features } = editor diff --git a/adminSiteClient/EditorExportTab.tsx b/adminSiteClient/EditorExportTab.tsx index f21574c51b3..5fe70b2181c 100644 --- a/adminSiteClient/EditorExportTab.tsx +++ b/adminSiteClient/EditorExportTab.tsx @@ -1,13 +1,13 @@ import { IReactionDisposer, action, autorun, computed, observable } from "mobx" import { observer } from "mobx-react" import React from "react" -import { ChartEditor } from "./ChartEditor.js" import { Section, Toggle } from "./Forms.js" import { Grapher } from "@ourworldindata/grapher" import { triggerDownloadFromBlob, GrapherStaticFormat, } from "@ourworldindata/utils" +import { AbstractChartEditor } from "./AbstractChartEditor.js" type ExportSettings = Required< Pick< @@ -55,18 +55,20 @@ const DEFAULT_SETTINGS: ExportSettings = { shouldIncludeDetailsInStaticExport: false, } -interface EditorExportTabProps { - editor: ChartEditor +interface EditorExportTabProps { + editor: Editor } @observer -export class EditorExportTab extends React.Component { +export class EditorExportTab< + Editor extends AbstractChartEditor, +> extends React.Component> { @observable private settings = DEFAULT_SETTINGS private originalSettings: Partial = DEFAULT_SETTINGS private originalGrapher: OriginalGrapher private disposers: IReactionDisposer[] = [] - constructor(props: EditorExportTabProps) { + constructor(props: EditorExportTabProps) { super(props) this.originalGrapher = this.grabRelevantPropertiesFromGrapher() } diff --git a/adminSiteClient/EditorFeatures.tsx b/adminSiteClient/EditorFeatures.tsx index 6156e13ba59..0de93aa88d0 100644 --- a/adminSiteClient/EditorFeatures.tsx +++ b/adminSiteClient/EditorFeatures.tsx @@ -1,11 +1,11 @@ import { computed } from "mobx" -import { ChartEditor } from "./ChartEditor.js" +import { AbstractChartEditor } from "./AbstractChartEditor.js" // Responsible for determining what parts of the editor should be shown, based on the // type of chart being edited export class EditorFeatures { - editor: ChartEditor - constructor(editor: ChartEditor) { + editor: AbstractChartEditor + constructor(editor: AbstractChartEditor) { this.editor = editor } diff --git a/adminSiteClient/EditorMapTab.tsx b/adminSiteClient/EditorMapTab.tsx index 25e4594fee8..8a167264423 100644 --- a/adminSiteClient/EditorMapTab.tsx +++ b/adminSiteClient/EditorMapTab.tsx @@ -9,9 +9,9 @@ import { ColumnSlug, isEmpty, ToleranceStrategy } from "@ourworldindata/utils" import { action, computed } from "mobx" import { observer } from "mobx-react" import React from "react" -import { ChartEditor } from "./ChartEditor.js" import { EditorColorScaleSection } from "./EditorColorScaleSection.js" import { NumberField, Section, SelectField, Toggle } from "./Forms.js" +import { AbstractChartEditor } from "./AbstractChartEditor.js" @observer class VariableSection extends React.Component<{ @@ -161,7 +161,9 @@ class TooltipSection extends React.Component<{ mapConfig: MapConfig }> { } @observer -export class EditorMapTab extends React.Component<{ editor: ChartEditor }> { +export class EditorMapTab< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor }> { @computed get grapher() { return this.props.editor.grapher } diff --git a/adminSiteClient/EditorReferencesTab.tsx b/adminSiteClient/EditorReferencesTab.tsx index 6051c991fcb..ef02ee79413 100644 --- a/adminSiteClient/EditorReferencesTab.tsx +++ b/adminSiteClient/EditorReferencesTab.tsx @@ -13,6 +13,7 @@ import { formatValue, ChartRedirect, } from "@ourworldindata/utils" +import { AbstractChartEditor } from "./AbstractChartEditor.js" const BASE_URL = BAKED_GRAPHER_URL.replace(/^https?:\/\//, "") @@ -214,8 +215,10 @@ export class EditorReferencesTab extends React.Component<{ } @observer -class AddRedirectForm extends React.Component<{ - editor: ChartEditor +class AddRedirectForm< + Editor extends AbstractChartEditor, +> extends React.Component<{ + editor: Editor onSuccess: (redirect: ChartRedirect) => void }> { static contextType = AdminAppContext diff --git a/adminSiteClient/EditorTextTab.tsx b/adminSiteClient/EditorTextTab.tsx index 0a5ba8b0220..43b657d55ad 100644 --- a/adminSiteClient/EditorTextTab.tsx +++ b/adminSiteClient/EditorTextTab.tsx @@ -15,7 +15,7 @@ import { observer } from "mobx-react" import React from "react" import Select from "react-select" import { TOPICS_CONTENT_GRAPH } from "../settings/clientSettings.js" -import { ChartEditor, ChartEditorManager } from "./ChartEditor.js" +import { isChartEditorInstance } from "./ChartEditor.js" import { AutoTextField, BindAutoString, @@ -27,9 +27,15 @@ import { TextField, Toggle, } from "./Forms.js" +import { ChartEditorContext } from "./ChartEditorContext.js" +import { AbstractChartEditor } from "./AbstractChartEditor.js" @observer -export class EditorTextTab extends React.Component<{ editor: ChartEditor }> { +export class EditorTextTab< + Editor extends AbstractChartEditor, +> extends React.Component<{ editor: Editor }> { + static contextType = ChartEditorContext + @action.bound onSlug(slug: string) { this.props.editor.grapher.slug = slugify(slug) } @@ -74,8 +80,8 @@ export class EditorTextTab extends React.Component<{ editor: ChartEditor }> { grapher.hideAnnotationFieldsInTitle.changeInPrefix = value || undefined } - @computed get errorMessages(): ChartEditorManager["errorMessages"] { - return this.props.editor.manager.errorMessages + @computed get errorMessages() { + return this.context.errorMessages } @computed get showAnyAnnotationFieldInTitleToggle() { @@ -88,7 +94,8 @@ export class EditorTextTab extends React.Component<{ editor: ChartEditor }> { } render() { - const { grapher, references, features } = this.props.editor + const { editor } = this.props + const { grapher, features } = editor const { relatedQuestions } = grapher return ( @@ -186,15 +193,16 @@ export class EditorTextTab extends React.Component<{ editor: ChartEditor }> { placeholder={grapher.originUrlWithProtocol} helpText="The page containing this chart where more context can be found" /> - {references && - (references.postsWordpress.length > 0 || - references.postsGdocs.length > 0) && ( + {isChartEditorInstance(editor) && + editor.references && + (editor.references.postsWordpress.length > 0 || + editor.references.postsGdocs.length > 0) && (

Origin url suggestions

    {[ - ...references.postsWordpress, - ...references.postsGdocs, + ...editor.references.postsWordpress, + ...editor.references.postsGdocs, ].map((post) => (
  • {post.url}
    • ))} @@ -213,10 +221,12 @@ export class EditorTextTab extends React.Component<{ editor: ChartEditor }> { /> - + {isChartEditorInstance(editor) && ( + + )}
    {relatedQuestions.map( diff --git a/adminSiteClient/GrapherConfigGridEditor.tsx b/adminSiteClient/GrapherConfigGridEditor.tsx index 06ebd7ab5c0..979101f95cc 100644 --- a/adminSiteClient/GrapherConfigGridEditor.tsx +++ b/adminSiteClient/GrapherConfigGridEditor.tsx @@ -7,6 +7,7 @@ import { getWindowUrl, setWindowUrl, excludeNull, + mergeGrapherConfigs, } from "@ourworldindata/utils" import { GrapherConfigPatch } from "../adminShared/AdminSessionTypes.js" import { @@ -348,8 +349,10 @@ export class GrapherConfigGridEditor extends React.Component { + @computed get availableTabs(): EditorTab[] { + const tabs: EditorTab[] = ["basic", "data", "text", "customize"] + if (this.grapher.hasMapTab) tabs.push("map") + if (this.grapher.isScatter) tabs.push("scatter") + if (this.grapher.isMarimekko) tabs.push("marimekko") + tabs.push("export") + return tabs + } + + @computed get isNewGrapher() { + return this.manager.isNewGrapher + } + + async saveGrapher({ + onError, + }: { onError?: () => void } = {}): Promise { + // TODO(inheritance) + console.log("save indicator chart") + } +} + +export function isIndicatorChartEditorInstance( + editor: AbstractChartEditor +): editor is IndicatorChartEditor { + return editor instanceof IndicatorChartEditor +} diff --git a/adminSiteClient/IndicatorChartEditorPage.tsx b/adminSiteClient/IndicatorChartEditorPage.tsx new file mode 100644 index 00000000000..119fab2d100 --- /dev/null +++ b/adminSiteClient/IndicatorChartEditorPage.tsx @@ -0,0 +1,72 @@ +import React from "react" +import { observer } from "mobx-react" +import { observable, computed, action } from "mobx" +import { isEmpty } from "@ourworldindata/utils" +import { GrapherInterface, DimensionProperty } from "@ourworldindata/types" +import { Grapher } from "@ourworldindata/grapher" +import { Admin } from "./Admin.js" +import { AdminAppContext, AdminAppContextType } from "./AdminAppContext.js" +import { ChartEditorView, ChartEditorViewManager } from "./ChartEditorView.js" +import { + IndicatorChartEditor, + IndicatorChartEditorManager, +} from "./IndicatorChartEditor.js" + +@observer +export class IndicatorChartEditorPage + extends React.Component<{ + variableId: number + }> + implements + IndicatorChartEditorManager, + ChartEditorViewManager +{ + @observable.ref grapher = new Grapher() + + static contextType = AdminAppContext + context!: AdminAppContextType + + patchConfig: GrapherInterface = {} + + isNewGrapher = false + + async fetchGrapher(): Promise { + const { variableId } = this.props + const config = await this.context.admin.getJSON( + `/api/variables/grapherConfigAdmin/${variableId}.patchConfig.json` + ) + if (isEmpty(config)) { + this.isNewGrapher = true + this.patchConfig = { + dimensions: [{ variableId, property: DimensionProperty.y }], + } + } + } + + @computed get admin(): Admin { + return this.context.admin + } + + @computed get editor(): IndicatorChartEditor { + return new IndicatorChartEditor({ manager: this }) + } + + @action.bound refresh(): void { + void this.fetchGrapher() + } + + componentDidMount(): void { + this.refresh() + } + + // TODO(inheritance) + // This funny construction allows the "new chart" link to work by forcing an update + // even if the props don't change + // UNSAFE_componentWillReceiveProps(): void { + // setTimeout(() => this.refresh(), 0) + // } + + render(): React.ReactElement { + return + } +} diff --git a/adminSiteClient/SaveButtons.tsx b/adminSiteClient/SaveButtons.tsx index 1ed6ecc5c0c..3bedd822669 100644 --- a/adminSiteClient/SaveButtons.tsx +++ b/adminSiteClient/SaveButtons.tsx @@ -3,13 +3,22 @@ import { ChartEditor } from "./ChartEditor.js" import { action, computed } from "mobx" import { observer } from "mobx-react" import { isEmpty } from "@ourworldindata/utils" +import { ChartEditorContext } from "./ChartEditorContext.js" +import { IndicatorChartEditor } from "./IndicatorChartEditor.js" @observer -export class SaveButtons extends React.Component<{ editor: ChartEditor }> { +export class SaveButtonsForChart extends React.Component<{ + editor: ChartEditor +}> { + // TODO + static contextType = ChartEditorContext + @action.bound onSaveChart() { void this.props.editor.saveGrapher() } + // TODO + @action.bound onSaveAsNew() { void this.props.editor.saveAsNewGrapher() } @@ -21,8 +30,7 @@ export class SaveButtons extends React.Component<{ editor: ChartEditor }> { } @computed get hasEditingErrors(): boolean { - const { editor } = this.props - const { errorMessages, errorMessagesForDimensions } = editor.manager + const { errorMessages, errorMessagesForDimensions } = this.context if (!isEmpty(errorMessages)) return true @@ -58,7 +66,7 @@ export class SaveButtons extends React.Component<{ editor: ChartEditor }> { disabled={isSavingDisabled} > Save as new - {" "} +
) + } +} + +@observer +export class SaveButtonsForIndicatorChart extends React.Component<{ + editor: IndicatorChartEditor +}> { + static contextType = ChartEditorContext + + @action.bound onSaveChart() { + void this.props.editor.saveGrapher() + } + + @computed get hasEditingErrors(): boolean { + const { errorMessages, errorMessagesForDimensions } = this.context + + if (!isEmpty(errorMessages)) return true + + const allErrorMessagesForDimensions = Object.values( + errorMessagesForDimensions + ).flat() + return allErrorMessagesForDimensions.some((error) => error) + } + + render() { + const { hasEditingErrors } = this + const { editor } = this.props + const { grapher } = editor + + const isSavingDisabled = grapher.hasFatalErrors || hasEditingErrors - /*return
- - {" "} - {" "} -
*/ + return ( +
+ +
+ ) } } diff --git a/adminSiteClient/VariableSelector.tsx b/adminSiteClient/VariableSelector.tsx index b146dd28e15..5728944d6d9 100644 --- a/adminSiteClient/VariableSelector.tsx +++ b/adminSiteClient/VariableSelector.tsx @@ -22,17 +22,18 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" import { faArchive } from "@fortawesome/free-solid-svg-icons" import { - ChartEditor, Dataset, EditorDatabase, Namespace, NamespaceData, -} from "./ChartEditor.js" +} from "./ChartEditorView.js" import { TextField, Toggle, Modal } from "./Forms.js" import { DimensionSlot } from "@ourworldindata/grapher" +import { AbstractChartEditor } from "./AbstractChartEditor.js" -interface VariableSelectorProps { - editor: ChartEditor +interface VariableSelectorProps { + database: EditorDatabase + editor: Editor slot: DimensionSlot onDismiss: () => void onComplete: (variableIds: OwidVariableId[]) => void @@ -49,7 +50,9 @@ interface Variable { } @observer -export class VariableSelector extends React.Component { +export class VariableSelector< + Editor extends AbstractChartEditor, +> extends React.Component> { @observable.ref chosenNamespaces: Namespace[] = [] @observable.ref searchInput?: string @observable.ref isProjection?: boolean @@ -63,7 +66,7 @@ export class VariableSelector extends React.Component { @observable rowHeight: number = 32 @computed get database(): EditorDatabase { - return this.props.editor.database + return this.props.database } @computed get searchWords(): SearchWord[] { @@ -200,7 +203,7 @@ export class VariableSelector extends React.Component { render() { const { slot } = this.props - const { database } = this.props.editor + const { database } = this.props const { searchInput, chosenVariables, @@ -498,7 +501,7 @@ export class VariableSelector extends React.Component { dispose!: IReactionDisposer base: React.RefObject = React.createRef() componentDidMount() { - void this.props.editor.loadVariableUsageCounts() + // void this.props.editor.loadVariableUsageCounts() // TODO??? this.initChosenVariablesAndNamespaces() } diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 91d4cebfb1f..72bf934caf2 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -13,12 +13,15 @@ import { expectInt, isValidSlug } from "../serverUtils/serverUtil.js" import { OldChartFieldList, assignTagsForCharts, + getParentConfigForChart, getChartConfigById, getChartSlugById, getGptTopicSuggestions, getRedirectsByChartId, oldChartFieldList, setChartTags, + getParentConfigForChartFromConfig, + getPatchConfigByChartId, } from "../db/model/Chart.js" import { Request } from "./authentication.js" import { @@ -26,7 +29,13 @@ import { fetchS3MetadataByPath, fetchS3DataValuesByPath, searchVariables, + getGrapherConfigsForVariable, + updateGrapherConfigAdminOfVariable, + updateGrapherConfigETLOfVariable, + updateAllChartsThatInheritFromIndicator, + getParentConfigForIndicatorChartAdmin, } from "../db/model/Variable.js" +import { updateExistingFullConfig } from "../db/model/ChartConfigs.js" import { getCanonicalUrl } from "@ourworldindata/components" import { camelCaseProperties, @@ -37,8 +46,6 @@ import { parseIntOrUndefined, DbRawPostWithGdocPublishStatus, OwidVariableWithSource, - OwidChartDimensionInterface, - DimensionProperty, TaggableType, DbChartTagJoin, pick, @@ -85,6 +92,7 @@ import { DbInsertUser, FlatTagGraph, DbRawChartConfig, + DimensionProperty, } from "@ourworldindata/types" import { defaultGrapherConfig, @@ -281,7 +289,7 @@ const saveNewChart = async ( } // compute patch and full configs - const parentConfig = defaultGrapherConfig + const parentConfig = await getParentConfigForChartFromConfig(knex, config) const patchConfig = diffGrapherConfigs(config, parentConfig) const fullConfig = mergeGrapherConfigs(parentConfig, patchConfig) @@ -343,7 +351,7 @@ const updateExistingChart = async ( } // compute patch and full configs - const parentConfig = defaultGrapherConfig + const parentConfig = await getParentConfigForChart(knex, chartId) const patchConfig = diffGrapherConfigs(config, parentConfig) const fullConfig = mergeGrapherConfigs(parentConfig, patchConfig) @@ -386,6 +394,8 @@ const saveGrapher = async ( referencedVariablesMightChange = true // if the variables a chart uses can change then we need // to update the latest country data which takes quite a long time (hundreds of ms) ) => { + console.log("saving grapher...") + // Slugs need some special logic to ensure public urls remain consistent whenever possible async function isSlugUsedInRedirect() { const rows = await db.knexRaw( @@ -635,6 +645,28 @@ getRouteWithROTransaction( async (req, res, trx) => expectChartById(trx, req.params.chartId) ) +getRouteWithROTransaction( + apiRouter, + "/charts/:chartId.parentConfig.json", + async (req, res, trx) => { + const chartId = expectInt(req.params.chartId) + return getParentConfigForChart(trx, chartId) ?? {} + } +) + +getRouteWithROTransaction( + apiRouter, + "/charts/:chartId.patchConfig.json", + async (req, res, trx) => { + const chartId = expectInt(req.params.chartId) + const config = await getPatchConfigByChartId(trx, chartId) + if (!config) { + throw new JsonError(`Chart with id ${chartId} not found`, 500) + } + return config + } +) + getRouteWithROTransaction( apiRouter, "/editorData/namespaces.json", @@ -1143,21 +1175,25 @@ getRouteWithROTransaction( const whereClause = filterSExpr?.toSql() ?? "true" const resultsWithStringGrapherConfigs = await db.knexRaw( trx, - `SELECT variables.id as id, - variables.name as name, - variables.grapherConfigAdmin as config, - d.name as datasetname, - namespaces.name as namespacename, - variables.createdAt as createdAt, - variables.updatedAt as updatedAt, - variables.description as description -FROM variables -LEFT JOIN active_datasets as d on variables.datasetId = d.id -LEFT JOIN namespaces on d.namespace = namespaces.name -WHERE ${whereClause} -ORDER BY variables.id DESC -LIMIT 50 -OFFSET ${offset.toString()}` + `-- sql + SELECT + variables.id as id, + variables.name as name, + chart_configs.patch as config, + d.name as datasetname, + namespaces.name as namespacename, + variables.createdAt as createdAt, + variables.updatedAt as updatedAt, + variables.description as description + FROM variables + LEFT JOIN active_datasets as d on variables.datasetId = d.id + LEFT JOIN namespaces on d.namespace = namespaces.name + LEFT JOIN chart_configs on variables.grapherConfigIdAdmin = chart_configs.id + WHERE ${whereClause} + ORDER BY variables.id DESC + LIMIT 50 + OFFSET ${offset.toString()} + ` ) const results = resultsWithStringGrapherConfigs.map((row: any) => ({ @@ -1166,11 +1202,14 @@ OFFSET ${offset.toString()}` })) const resultCount = await db.knexRaw<{ count: number }>( trx, - `SELECT count(*) as count -FROM variables -LEFT JOIN active_datasets as d on variables.datasetId = d.id -LEFT JOIN namespaces on d.namespace = namespaces.name -WHERE ${whereClause}` + `-- sql + SELECT count(*) as count + FROM variables + LEFT JOIN active_datasets as d on variables.datasetId = d.id + LEFT JOIN namespaces on d.namespace = namespaces.name + LEFT JOIN chart_configs on variables.grapherConfigIdAdmin = chart_configs.id + WHERE ${whereClause} + ` ) return { rows: results, numTotalRows: resultCount[0].count } } @@ -1184,14 +1223,25 @@ patchRouteWithRWTransaction( const variableIds = new Set(patchesList.map((patch) => patch.id)) const configsAndIds = await db.knexRaw< - Pick - >(trx, `SELECT id, grapherConfigAdmin FROM variables where id IN (?)`, [ - [...variableIds.values()], - ]) + Pick & { + grapherConfigAdmin: DbRawChartConfig["patch"] + } + >( + trx, + `-- sql + SELECT v.id, cc.patch AS grapherConfigAdmin + FROM variables v + LEFT JOIN chart_configs cc ON v.grapherConfigIdAdmin = cc.id + WHERE v.id IN (?) + `, + [[...variableIds.values()]] + ) const configMap = new Map( configsAndIds.map((item: any) => [ item.id, - item.grapherConfigAdmin ? JSON.parse(item.grapherConfig) : {}, + item.grapherConfigAdmin + ? JSON.parse(item.grapherConfigAdmin) + : {}, ]) ) // console.log("ids", configsAndIds.map((item : any) => item.id)) @@ -1201,11 +1251,9 @@ patchRouteWithRWTransaction( } for (const [variableId, newConfig] of configMap.entries()) { - await db.knexRaw( - trx, - `UPDATE variables SET grapherConfigAdmin = ? where id = ?`, - [JSON.stringify(newConfig), variableId] - ) + const variable = await getGrapherConfigsForVariable(trx, variableId) + if (!variable) continue + await updateGrapherConfigAdminOfVariable(trx, variable, newConfig) } return { success: true } @@ -1233,6 +1281,43 @@ getRouteWithROTransaction( } ) +getRouteWithROTransaction( + apiRouter, + "/variables/grapherConfigAdmin/:variableId.patchConfig.json", + async (req, res, trx) => { + const variableId = expectInt(req.params.variableId) + console.log("here", variableId) + const variable = await getGrapherConfigsForVariable(trx, variableId) + if (!variable) { + throw new JsonError(`Variable with id ${variableId} not found`, 500) + } + return variable.admin?.patchConfig ?? {} + } +) + +getRouteWithROTransaction( + apiRouter, + "/variables/grapherConfigAdmin/:variableId.parentConfig.json", + async (req, res, trx) => { + const variableId = expectInt(req.params.variableId) + const parentConfig = await getParentConfigForIndicatorChartAdmin( + trx, + variableId + ) + return parentConfig ?? {} + } +) + +getRouteWithROTransaction( + apiRouter, + "/variables/mergedGrapherConfig/:variableId.json", + async (req, res, trx) => { + const variableId = expectInt(req.params.variableId) + const config = await getMergedGrapherConfigForVariable(trx, variableId) + return config ?? {} + } +) + // Used in VariableEditPage getRouteWithROTransaction( apiRouter, @@ -1269,22 +1354,9 @@ getRouteWithROTransaction( await assignTagsForCharts(trx, charts) const grapherConfig = await getMergedGrapherConfigForVariable( - variableId, - trx + trx, + variableId ) - if ( - grapherConfig && - (!grapherConfig.dimensions || grapherConfig.dimensions.length === 0) - ) { - const dimensions: OwidChartDimensionInterface[] = [ - { - variableId: variableId, - property: DimensionProperty.y, - display: variable.display, - }, - ] - grapherConfig.dimensions = dimensions - } const variablesWithCharts: OwidVariableWithSource & { charts: Record @@ -1301,6 +1373,77 @@ getRouteWithROTransaction( } ) +// inserts a new config or updates an existing one +postRouteWithRWTransaction( + apiRouter, + "/variables/:variableId/grapherConfigETL/update", + async (req, res, trx) => { + const variableId = expectInt(req.params.variableId) + const variable = await getGrapherConfigsForVariable(trx, variableId) + if (!variable) { + throw new JsonError(`Variable with id ${variableId} not found`, 500) + } + await updateGrapherConfigETLOfVariable(trx, variable, req.body) + return { success: true } + } +) + +deleteRouteWithRWTransaction( + apiRouter, + "/variables/:variableId/grapherConfigETL/delete", + async (req, res, trx) => { + const variableId = expectInt(req.params.variableId) + + const variable = await getGrapherConfigsForVariable(trx, variableId) + if (!variable) { + throw new JsonError(`Variable with id ${variableId} not found`, 500) + } + + if (!variable.etl) { + throw new JsonError( + `Variable with id ${variableId} doesn't have an ETL config`, + 500 + ) + } + + // remove reference in the variables table + await db.knexRaw( + trx, + `-- sql + UPDATE variables + SET grapherConfigIdETL = NULL + WHERE id = ? + `, + [variableId] + ) + + // delete row in the chart_configs table + await db.knexRaw( + trx, + `-- sql + DELETE FROM chart_configs + WHERE id = ? + `, + [variable.etl.configId] + ) + + // update admin config if there is one + if (variable.admin) { + await updateExistingFullConfig(trx, { + configId: variable.admin.configId, + config: variable.admin.patchConfig, + }) + } + + // update all charts that inherit from the indicator + await updateAllChartsThatInheritFromIndicator(trx, variableId, { + patchConfigAdmin: variable.admin?.patchConfig, + }) + + return { success: true } + } +) + getRouteWithROTransaction( apiRouter, "/datasets.json", diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index 9046c9cc82c..8c1f65da6c0 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -9,9 +9,9 @@ import { deserializeJSONFromHTML, uniq, keyBy, - mergePartialGrapherConfigs, compact, partition, + mergeGrapherConfigs, } from "@ourworldindata/utils" import fs from "fs-extra" import * as lodash from "lodash" @@ -150,16 +150,15 @@ export async function renderDataPageV2( knex: db.KnexReadWriteTransaction ) { const grapherConfigForVariable = await getMergedGrapherConfigForVariable( - variableId, - knex + knex, + variableId ) // Only merge the grapher config on the indicator if the caller tells us to do so - // this is true for preview pages for datapages on the indicator level but false // 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) + ? mergeGrapherConfigs(grapherConfigForVariable ?? {}, pageGrapher ?? {}) : pageGrapher ?? {} const faqDocs = compact( diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index b69285f95d6..c541a326bf8 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -44,11 +44,11 @@ import { JsonError, Url, IndexPost, - mergePartialGrapherConfigs, OwidGdocType, OwidGdoc, OwidGdocDataInsightInterface, DbRawPost, + mergeGrapherConfigs, } from "@ourworldindata/utils" import { extractFormattingOptions } from "../serverUtils/wordpressUtils.js" import { @@ -800,7 +800,16 @@ export const renderExplorerPage = async ( if (requiredVariableIds.length) { partialGrapherConfigRows = await knexRaw( knex, - `SELECT id, grapherConfigETL, grapherConfigAdmin FROM variables WHERE id IN (?)`, + `-- sql + SELECT + v.id, + cc_etl.patch AS grapherConfigETL, + cc_admin.patch AS grapherConfigAdmin + FROM variables v + LEFT JOIN chart_configs cc_admin ON cc_admin.id=v.grapherConfigIdAdmin + LEFT JOIN chart_configs cc_etl ON cc_etl.id=v.grapherConfigIdETL + WHERE v.id IN (?) + `, [requiredVariableIds] ) @@ -840,8 +849,7 @@ export const renderExplorerPage = async ( config: row.grapherConfigETL as string, }) : {} - // TODO(inheritance): use mergeGrapherConfigs instead - return mergePartialGrapherConfigs(etlConfig, adminConfig) + return mergeGrapherConfigs(etlConfig, adminConfig) }) const wpContent = transformedProgram.wpBlockId diff --git a/db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts b/db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts new file mode 100644 index 00000000000..ffcefd330e6 --- /dev/null +++ b/db/migration/1721296631522-MoveIndicatorChartsToTheChartConfigsTable.ts @@ -0,0 +1,184 @@ +import { defaultGrapherConfig } from "@ourworldindata/grapher" +import { DimensionProperty, GrapherInterface } from "@ourworldindata/types" +import { omit } from "@ourworldindata/utils" +import { MigrationInterface, QueryRunner } from "typeorm" + +export class MoveIndicatorChartsToTheChartConfigsTable1721296631522 + implements MigrationInterface +{ + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(`-- sql + ALTER TABLE variables + ADD COLUMN grapherConfigIdAdmin binary(16) UNIQUE AFTER sort, + ADD COLUMN grapherConfigIdETL binary(16) UNIQUE AFTER grapherConfigIdAdmin, + ADD CONSTRAINT fk_variables_grapherConfigIdAdmin + FOREIGN KEY (grapherConfigIdAdmin) + REFERENCES chart_configs (id) + ON DELETE RESTRICT + ON UPDATE RESTRICT, + ADD CONSTRAINT fk_variables_grapherConfigIdETL + FOREIGN KEY (grapherConfigIdETL) + REFERENCES chart_configs (id) + ON DELETE RESTRICT + ON UPDATE RESTRICT + `) + + // note that we copy the ETL-authored configs to the chart_configs table, + // but drop the admin-authored configs + + const variables = await queryRunner.query(`-- sql + SELECT id, grapherConfigETL + FROM variables + WHERE grapherConfigETL IS NOT NULL + `) + + for (const { id: variableId, grapherConfigETL } of variables) { + let config: GrapherInterface = JSON.parse(grapherConfigETL) + + // if the config has no schema, assume it's the default version + if (!config.$schema) { + config.$schema = defaultGrapherConfig.$schema + } + + // check if the given dimensions are correct + if (config.dimensions && config.dimensions.length >= 1) { + // make sure there is only a single entry + config.dimensions = config.dimensions.slice(0, 1) + // make sure the variable id matches + config.dimensions[0].variableId = variableId + } + + // fill dimensions if not given to make the config plottable + if (!config.dimensions || config.dimensions.length === 0) { + config.dimensions = [ + { property: DimensionProperty.y, variableId }, + ] + } + + // we have v3 configs in the database (the current version is v4); + // turn these into v4 configs by removing the `data` property + // which was the breaking change that lead to v4 + // (we don't have v2 or v1 configs in the database, so we don't need to handle those) + if ( + config.$schema === + "https://files.ourworldindata.org/schemas/grapher-schema.003.json" + ) { + config = omit(config, "data") + config.$schema = defaultGrapherConfig.$schema + } + + // insert config into the chart_configs table + const configId = await getBinaryUUID(queryRunner) + await queryRunner.query( + `-- sql + INSERT INTO chart_configs (id, patch, full) + VALUES (?, ?, ?) + `, + [configId, JSON.stringify(config), JSON.stringify(config)] + ) + + // update reference in the variables table + await queryRunner.query( + `-- sql + UPDATE variables + SET grapherConfigIdETL = ? + WHERE id = ? + `, + [configId, variableId] + ) + } + + // drop `grapherConfigAdmin` and `grapherConfigETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + DROP COLUMN grapherConfigAdmin, + DROP COLUMN grapherConfigETL + `) + + // add a view that lists all charts that inherit from an indicator + await queryRunner.query(`-- sql + CREATE VIEW inheriting_charts AS ( + WITH y_dimensions AS ( + SELECT + * + FROM + chart_dimensions + WHERE + property = 'y' + ), + single_y_indicator_charts As ( + SELECT + c.id as chartId, + cc.patch as patchConfig, + max(yd.variableId) as variableId + FROM + charts c + JOIN chart_configs cc ON cc.id = c.configId + JOIN y_dimensions yd ON c.id = yd.chartId + WHERE + cc.full ->> '$.type' != 'ScatterPlot' + GROUP BY + c.id + HAVING + COUNT(distinct yd.variableId) = 1 + ) + SELECT + variableId, + chartId + FROM + single_y_indicator_charts + ORDER BY + variableId + ) + `) + } + + public async down(queryRunner: QueryRunner): Promise { + // drop view + await queryRunner.query(`-- sql + DROP VIEW inheriting_charts + `) + + // add back the `grapherConfigAdmin` and `grapherConfigETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + ADD COLUMN grapherConfigAdmin json AFTER sort, + ADD COLUMN grapherConfigETL json AFTER grapherConfigAdmin + `) + + // copy configs from the chart_configs table to the variables table + await queryRunner.query(`-- sql + UPDATE variables v + JOIN chart_configs cc ON v.grapherConfigIdETL = cc.id + SET v.grapherConfigETL = cc.patch + `) + + // remove constraints on the `grapherConfigIdAdmin` and `grapherConfigIdETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + DROP CONSTRAINT fk_variables_grapherConfigIdAdmin, + DROP CONSTRAINT fk_variables_grapherConfigIdETL + `) + + // drop rows from the chart_configs table + await queryRunner.query(`-- sql + DELETE FROM chart_configs + WHERE id IN ( + SELECT grapherConfigIdETL FROM variables + WHERE grapherConfigIdETL IS NOT NULL + ) + `) + + // remove the `grapherConfigIdAdmin` and `grapherConfigIdETL` columns + await queryRunner.query(`-- sql + ALTER TABLE variables + DROP COLUMN grapherConfigIdAdmin, + DROP COLUMN grapherConfigIdETL + `) + } +} + +const getBinaryUUID = async (queryRunner: QueryRunner): Promise => { + const rows = await queryRunner.query(`SELECT UUID_TO_BIN(UUID(), 1) AS id`) + return rows[0].id +} diff --git a/db/model/Chart.ts b/db/model/Chart.ts index 86ece2b0648..92150f74330 100644 --- a/db/model/Chart.ts +++ b/db/model/Chart.ts @@ -1,11 +1,16 @@ import * as lodash from "lodash" import * as db from "../db.js" -import { getDataForMultipleVariables } from "./Variable.js" +import { + getDataForMultipleVariables, + getGrapherConfigsForVariable, +} from "./Variable.js" import { JsonError, KeyChartLevel, MultipleOwidVariableDataDimensionsMap, DbChartTagJoin, + mergeGrapherConfigs, + getParentIndicatorIdFromChartConfig, } from "@ourworldindata/utils" import { GrapherInterface, @@ -18,7 +23,9 @@ import { DbPlainTag, DbRawChartConfig, DbEnrichedChartConfig, + DimensionProperty, } from "@ourworldindata/types" +import { defaultGrapherConfig } from "@ourworldindata/grapher" import { OpenAI } from "openai" import { BAKED_BASE_URL, @@ -154,6 +161,24 @@ export async function getRawChartById( return chart } +export async function getPatchConfigByChartId( + knex: db.KnexReadonlyTransaction, + id: number +): Promise { + const chart = await db.knexRawFirst>( + knex, + `-- sql + SELECT patch + FROM chart_configs cc + JOIN charts c ON c.configId = cc.id + WHERE c.id = ? + `, + [id] + ) + if (!chart) return undefined + return parseChartConfig(chart.patch) +} + export async function getEnrichedChartById( knex: db.KnexReadonlyTransaction, id: number @@ -232,6 +257,46 @@ export async function getChartConfigBySlug( return { id: row.id, config: parseChartConfig(row.config) } } +export async function getParentConfigForChart( + trx: db.KnexReadonlyTransaction, + chartId: number +): Promise { + // check if the chart inherits settings from an indicator + const parentVariable = await db.knexRawFirst<{ id: number }>( + trx, + `-- sql + SELECT variableId AS id + FROM inheriting_charts + WHERE chartId = ? + `, + [chartId] + ) + + // all charts inherit from the default config + if (!parentVariable) return defaultGrapherConfig + + const variable = await getGrapherConfigsForVariable(trx, parentVariable.id) + return mergeGrapherConfigs( + defaultGrapherConfig, + variable?.etl?.patchConfig ?? {}, + variable?.admin?.patchConfig ?? {} + ) +} + +export async function getParentConfigForChartFromConfig( + trx: db.KnexReadonlyTransaction, + config: GrapherInterface +): Promise { + const parentVariableId = getParentIndicatorIdFromChartConfig(config) + if (!parentVariableId) return defaultGrapherConfig + const variable = await getGrapherConfigsForVariable(trx, parentVariableId) + return mergeGrapherConfigs( + defaultGrapherConfig, + variable?.etl?.patchConfig ?? {}, + variable?.admin?.patchConfig ?? {} + ) +} + export async function setChartTags( knex: db.KnexReadWriteTransaction, chartId: number, diff --git a/db/model/ChartConfigs.ts b/db/model/ChartConfigs.ts new file mode 100644 index 00000000000..f66d3ff7eef --- /dev/null +++ b/db/model/ChartConfigs.ts @@ -0,0 +1,68 @@ +import { DbInsertChartConfig, GrapherInterface } from "@ourworldindata/types" + +import * as db from "../db.js" + +interface ConfigWithId { + configId: DbInsertChartConfig["id"] + config: GrapherInterface +} + +export async function updateExistingConfigPair( + knex: db.KnexReadWriteTransaction, + { + configId, + patchConfig, + fullConfig, + }: { + configId: DbInsertChartConfig["id"] + patchConfig: GrapherInterface + fullConfig: GrapherInterface + } +): Promise { + await db.knexRaw( + knex, + `-- sql + UPDATE chart_configs + SET + patch = ?, + full = ? + WHERE id = ? + `, + [JSON.stringify(patchConfig), JSON.stringify(fullConfig), configId] + ) +} + +export async function updateExistingPatchConfig( + knex: db.KnexReadWriteTransaction, + params: ConfigWithId +): Promise { + await updateExistingConfig(knex, { ...params, column: "patch" }) +} + +export async function updateExistingFullConfig( + knex: db.KnexReadWriteTransaction, + params: ConfigWithId +): Promise { + await updateExistingConfig(knex, { ...params, column: "full" }) +} + +export async function updateExistingConfig( + knex: db.KnexReadWriteTransaction, + { + column, + configId, + config, + }: ConfigWithId & { + column: "patch" | "full" + } +): Promise { + await db.knexRaw( + knex, + `-- sql + UPDATE chart_configs + SET ?? = ? + WHERE id = ? + `, + [column, JSON.stringify(config), configId] + ) +} diff --git a/db/model/Variable.ts b/db/model/Variable.ts index 8bcd74abe0e..aa319f29a14 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -1,10 +1,18 @@ import _ from "lodash" import { Writable } from "stream" import * as db from "../db.js" -import { retryPromise, isEmpty } from "@ourworldindata/utils" +import { + retryPromise, + isEmpty, + omitUndefinedValues, + mergeGrapherConfigs, + omitNullableValues, +} from "@ourworldindata/utils" import { getVariableDataRoute, getVariableMetadataRoute, + defaultGrapherConfig, + DEFAULT_GRAPHER_CONFIG_SCHEMA, } from "@ourworldindata/grapher" import pl from "nodejs-polars" import { DATA_API_URL } from "../../settings/serverSettings.js" @@ -20,33 +28,319 @@ import { GrapherInterface, DbRawVariable, VariablesTableName, + DbRawChartConfig, + parseChartConfig, + DbEnrichedChartConfig, + DbEnrichedVariable, } from "@ourworldindata/types" -import { knexRaw } from "../db.js" +import { knexRaw, knexRawFirst } from "../db.js" +import { + updateExistingConfigPair, + updateExistingFullConfig, + updateExistingPatchConfig, +} from "./ChartConfigs.js" + +interface ChartConfigPair { + configId: DbEnrichedChartConfig["id"] + patchConfig: DbEnrichedChartConfig["patch"] + fullConfig: DbEnrichedChartConfig["full"] +} -//export type Field = keyof VariableRow +interface VariableWithGrapherConfigs { + variableId: DbEnrichedVariable["id"] + admin?: ChartConfigPair + etl?: ChartConfigPair +} -export async function getMergedGrapherConfigForVariable( - variableId: number, - knex: db.KnexReadonlyTransaction -): Promise { - const rows: Pick< - DbRawVariable, - "grapherConfigAdmin" | "grapherConfigETL" - >[] = await knexRaw( +export async function getGrapherConfigsForVariable( + knex: db.KnexReadonlyTransaction, + variableId: number +): Promise { + const variable = await knexRawFirst< + Pick< + DbRawVariable, + "id" | "grapherConfigIdAdmin" | "grapherConfigIdETL" + > & { + patchConfigAdmin?: DbRawChartConfig["patch"] + patchConfigETL?: DbRawChartConfig["patch"] + fullConfigAdmin?: DbRawChartConfig["full"] + fullConfigETL?: DbRawChartConfig["full"] + } + >( knex, - `SELECT grapherConfigAdmin, grapherConfigETL FROM variables WHERE id = ?`, + `-- sql + SELECT + v.id, + v.grapherConfigIdAdmin, + v.grapherConfigIdETL, + cc_admin.patch AS patchConfigAdmin, + cc_admin.full AS fullConfigAdmin, + cc_etl.patch AS patchConfigETL, + cc_etl.full AS fullConfigETL + FROM variables v + LEFT JOIN chart_configs cc_admin ON v.grapherConfigIdAdmin = cc_admin.id + LEFT JOIN chart_configs cc_etl ON v.grapherConfigIdETL = cc_etl.id + WHERE v.id = ? + `, [variableId] ) - if (!rows.length) return - const row = rows[0] - const grapherConfigAdmin = row.grapherConfigAdmin - ? JSON.parse(row.grapherConfigAdmin) + + if (!variable) return + + const maybeParseChartConfig = (config: string | undefined) => + config ? parseChartConfig(config) : {} + + const admin = variable.grapherConfigIdAdmin + ? { + configId: variable.grapherConfigIdAdmin, + patchConfig: maybeParseChartConfig(variable.patchConfigAdmin), + fullConfig: maybeParseChartConfig(variable.fullConfigAdmin), + } : undefined - const grapherConfigETL = row.grapherConfigETL - ? JSON.parse(row.grapherConfigETL) + + const etl = variable.grapherConfigIdETL + ? { + configId: variable.grapherConfigIdETL, + patchConfig: maybeParseChartConfig(variable.patchConfigETL), + fullConfig: maybeParseChartConfig(variable.fullConfigETL), + } : undefined - // TODO(inheritance): use mergeGrapherConfigs instead - return _.merge({}, grapherConfigAdmin, grapherConfigETL) + + return omitUndefinedValues({ + variableId: variable.id, + admin, + etl, + }) +} + +export async function getMergedGrapherConfigForVariable( + knex: db.KnexReadonlyTransaction, + variableId: number +): Promise { + const variable = await getGrapherConfigsForVariable(knex, variableId) + return variable?.admin?.fullConfig ?? variable?.etl?.fullConfig +} + +export async function insertNewGrapherConfigForVariable( + knex: db.KnexReadonlyTransaction, + { + type, + variableId, + patchConfig, + fullConfig, + }: { + type: "admin" | "etl" + variableId: number + patchConfig: GrapherInterface + fullConfig: GrapherInterface + } +): Promise { + // insert chart config into the database + const configId = await db.getBinaryUUID(knex) + await db.knexRaw( + knex, + `-- sql + INSERT INTO chart_configs (id, patch, full) + VALUES (?, ?, ?) + `, + [configId, JSON.stringify(patchConfig), JSON.stringify(fullConfig)] + ) + + // make a reference to the config from the variables table + const column = + type === "admin" ? "grapherConfigIdAdmin" : "grapherConfigIdETL" + await db.knexRaw( + knex, + `-- sql + UPDATE variables + SET ?? = ? + WHERE id = ? + `, + [column, configId, variableId] + ) +} + +function makeConfigValidForIndicator({ + config, + variableId, +}: { + config: GrapherInterface + variableId: number +}): GrapherInterface { + const updatedConfig = { ...config } + + // if no schema is given, assume it's the latest + if (!updatedConfig.$schema) { + updatedConfig.$schema = DEFAULT_GRAPHER_CONFIG_SCHEMA + } + + // check if the given dimensions are correct + if (updatedConfig.dimensions && updatedConfig.dimensions.length >= 1) { + // make sure there is only a single entry + updatedConfig.dimensions = updatedConfig.dimensions.slice(0, 1) + // make sure the variable id matches + updatedConfig.dimensions[0].variableId = variableId + } + + // fill dimensions if not given to make the updatedConfig plottable + if (!updatedConfig.dimensions || updatedConfig.dimensions.length === 0) { + updatedConfig.dimensions = [ + { property: DimensionProperty.y, variableId }, + ] + } + + return updatedConfig +} + +async function findAllChartsThatInheritFromIndicator( + trx: db.KnexReadonlyTransaction, + variableId: number +): Promise<{ chartId: number; patchConfig: GrapherInterface }[]> { + const charts = await db.knexRaw<{ + chartId: number + patchConfig: string + }>( + trx, + `-- sql + SELECT c.id as chartId, cc.patch as patchConfig + FROM inheriting_charts ic + JOIN charts c ON c.id = ic.chartId + JOIN chart_configs cc ON cc.id = c.configId + WHERE ic.variableId = ? + `, + [variableId] + ) + return charts.map((chart) => ({ + ...chart, + patchConfig: parseChartConfig(chart.patchConfig), + })) +} + +export async function updateAllChartsThatInheritFromIndicator( + trx: db.KnexReadWriteTransaction, + variableId: number, + { + patchConfigETL, + patchConfigAdmin, + }: { + patchConfigETL?: GrapherInterface + patchConfigAdmin?: GrapherInterface + } +): Promise { + const inheritingCharts = await findAllChartsThatInheritFromIndicator( + trx, + variableId + ) + for (const chart of inheritingCharts) { + const fullConfig = mergeGrapherConfigs( + defaultGrapherConfig, + patchConfigETL ?? {}, + patchConfigAdmin ?? {}, + chart.patchConfig + ) + await db.knexRaw( + trx, + `-- sql + UPDATE chart_configs cc + JOIN charts c ON c.configId = cc.id + SET cc.full = ? + WHERE c.id = ? + `, + [JSON.stringify(fullConfig), chart.chartId] + ) + } +} + +export async function updateGrapherConfigETLOfVariable( + trx: db.KnexReadWriteTransaction, + variable: VariableWithGrapherConfigs, + config: GrapherInterface +): Promise { + const { variableId } = variable + + const configETL = makeConfigValidForIndicator({ + config, + variableId, + }) + + if (variable.etl) { + await updateExistingConfigPair(trx, { + configId: variable.etl.configId, + patchConfig: configETL, + fullConfig: configETL, + }) + } else { + await insertNewGrapherConfigForVariable(trx, { + type: "etl", + variableId, + patchConfig: configETL, + fullConfig: configETL, + }) + } + + // update admin-authored full config it is exists + if (variable.admin) { + const fullConfig = mergeGrapherConfigs( + configETL, + variable.admin.patchConfig + ) + await updateExistingFullConfig(trx, { + configId: variable.admin.configId, + config: fullConfig, + }) + } + + await updateAllChartsThatInheritFromIndicator(trx, variableId, { + patchConfigETL: configETL, + patchConfigAdmin: variable.admin?.patchConfig, + }) +} + +export async function updateGrapherConfigAdminOfVariable( + trx: db.KnexReadWriteTransaction, + variable: VariableWithGrapherConfigs, + config: GrapherInterface +): Promise { + const { variableId } = variable + + const patchConfigAdmin = makeConfigValidForIndicator({ + config, + variableId, + }) + + const fullConfigAdmin = mergeGrapherConfigs( + variable.etl?.patchConfig ?? {}, + patchConfigAdmin + ) + + if (variable.admin) { + await updateExistingConfigPair(trx, { + configId: variable.admin.configId, + patchConfig: patchConfigAdmin, + fullConfig: fullConfigAdmin, + }) + } else { + await insertNewGrapherConfigForVariable(trx, { + type: "admin", + variableId, + patchConfig: patchConfigAdmin, + fullConfig: fullConfigAdmin, + }) + } + + await updateAllChartsThatInheritFromIndicator(trx, variableId, { + patchConfigETL: variable.etl?.patchConfig ?? {}, + patchConfigAdmin: patchConfigAdmin, + }) +} + +export async function getParentConfigForIndicatorChartAdmin( + trx: db.KnexReadonlyTransaction, + variableId: number +): Promise { + // check if there is an ETL-authored indicator chart + const variable = await getGrapherConfigsForVariable(trx, variableId) + return variable?.etl?.fullConfig } // TODO: these are domain functions and should live somewhere else diff --git a/packages/@ourworldindata/grapher/src/index.ts b/packages/@ourworldindata/grapher/src/index.ts index 9e9d08b9902..ed27bd627f5 100644 --- a/packages/@ourworldindata/grapher/src/index.ts +++ b/packages/@ourworldindata/grapher/src/index.ts @@ -26,6 +26,7 @@ export { grapherInterfaceWithHiddenTabsOnly, CONTINENTS_INDICATOR_ID, POPULATION_INDICATOR_ID_USED_IN_ADMIN, + DEFAULT_GRAPHER_CONFIG_SCHEMA, } from "./core/GrapherConstants" export { getVariableDataRoute, diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml index a48db22efcb..5a895dea054 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.004.yaml @@ -185,8 +185,7 @@ $defs: description: The minimum bracket of the first bin additionalProperties: false required: - - title - - version + - $schema - dimensions type: object description: | diff --git a/packages/@ourworldindata/types/src/dbTypes/InheritingCharts.ts b/packages/@ourworldindata/types/src/dbTypes/InheritingCharts.ts new file mode 100644 index 00000000000..3a956f68cc7 --- /dev/null +++ b/packages/@ourworldindata/types/src/dbTypes/InheritingCharts.ts @@ -0,0 +1,8 @@ +export const InheritingChartsTableName = "inheriting_charts" + +export interface DbInsertInheritingChart { + variableId: number + chartId: number +} + +export type DbPlainInheritingChart = Required diff --git a/packages/@ourworldindata/types/src/dbTypes/Variables.ts b/packages/@ourworldindata/types/src/dbTypes/Variables.ts index ce241a997c8..3c2d40c5a3f 100644 --- a/packages/@ourworldindata/types/src/dbTypes/Variables.ts +++ b/packages/@ourworldindata/types/src/dbTypes/Variables.ts @@ -1,7 +1,6 @@ import { OwidVariableType } from "../OwidVariable.js" import { OwidVariableDisplayConfigInterface } from "../OwidVariableDisplayConfigInterface.js" import { JsonString } from "../domainTypes/Various.js" -import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" export const VariablesTableName = "variables" export interface DbInsertVariable { @@ -20,8 +19,8 @@ export interface DbInsertVariable { descriptionShort?: string | null dimensions?: JsonString | null display: JsonString - grapherConfigAdmin?: JsonString | null - grapherConfigETL?: JsonString | null + grapherConfigIdAdmin?: Buffer | null + grapherConfigIdETL?: Buffer | null id?: number license?: JsonString | null licenses?: JsonString | null @@ -66,8 +65,6 @@ export type DbEnrichedVariable = Omit< | "dimensions" | "descriptionKey" | "originalMetadata" - | "grapherConfigAdmin" - | "grapherConfigETL" | "processingLog" | "sort" > & { @@ -77,8 +74,6 @@ export type DbEnrichedVariable = Omit< dimensions: VariableDisplayDimension | null descriptionKey: string[] | null originalMetadata: unknown | null - grapherConfigAdmin: GrapherInterface | null - grapherConfigETL: GrapherInterface | null processingLog: unknown | null sort: string[] | null } @@ -149,30 +144,6 @@ export function serializeVariableOriginalMetadata( return originalMetadata ? JSON.stringify(originalMetadata) : null } -export function parseVariableGrapherConfigAdmin( - grapherConfigAdmin: JsonString | null -): GrapherInterface { - return grapherConfigAdmin ? JSON.parse(grapherConfigAdmin) : null -} - -export function serializeVariableGrapherConfigAdmin( - grapherConfigAdmin: GrapherInterface | null -): JsonString | null { - return grapherConfigAdmin ? JSON.stringify(grapherConfigAdmin) : null -} - -export function parseVariableGrapherConfigETL( - grapherConfigETL: JsonString | null -): GrapherInterface { - return grapherConfigETL ? JSON.parse(grapherConfigETL) : null -} - -export function serializeVariableGrapherConfigETL( - grapherConfigETL: GrapherInterface | null -): JsonString | null { - return grapherConfigETL ? JSON.stringify(grapherConfigETL) : null -} - export function parseVariableProcessingLog( processingLog: JsonString | null ): any { @@ -204,10 +175,6 @@ export function parseVariablesRow(row: DbRawVariable): DbEnrichedVariable { dimensions: parseVariableDimensions(row.dimensions), descriptionKey: parseVariableDescriptionKey(row.descriptionKey), originalMetadata: parseVariableOriginalMetadata(row.originalMetadata), - grapherConfigAdmin: parseVariableGrapherConfigAdmin( - row.grapherConfigAdmin - ), - grapherConfigETL: parseVariableGrapherConfigETL(row.grapherConfigETL), processingLog: parseVariableProcessingLog(row.processingLog), sort: parseVariableSort(row.sort), } @@ -224,12 +191,6 @@ export function serializeVariablesRow(row: DbEnrichedVariable): DbRawVariable { originalMetadata: serializeVariableOriginalMetadata( row.originalMetadata ), - grapherConfigAdmin: serializeVariableGrapherConfigAdmin( - row.grapherConfigAdmin - ), - grapherConfigETL: serializeVariableGrapherConfigETL( - row.grapherConfigETL - ), processingLog: serializeVariableProcessingLog(row.processingLog), sort: serializeVariableSort(row.sort), } diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index f888ef464bf..fca5baf4db7 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -634,10 +634,6 @@ export { serializeVariableOriginalMetadata, parseVariableLicenses, serializeVariableLicenses, - parseVariableGrapherConfigAdmin, - serializeVariableGrapherConfigAdmin, - parseVariableGrapherConfigETL, - serializeVariableGrapherConfigETL, parseVariableProcessingLog, serializeVariableProcessingLog, type License, diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts index a95acc7721b..0dca9ae008c 100644 --- a/packages/@ourworldindata/utils/src/Util.ts +++ b/packages/@ourworldindata/utils/src/Util.ts @@ -182,6 +182,9 @@ import { TagGraphRoot, TagGraphRootName, TagGraphNode, + GrapherInterface, + ChartTypeName, + DimensionProperty, } from "@ourworldindata/types" import { PointVector } from "./PointVector.js" import React from "react" @@ -1776,13 +1779,6 @@ export function filterValidStringValues( return filteredValues } -// TODO(inheritance): remove in favour of mergeGrapherConfigs -export function mergePartialGrapherConfigs>( - ...grapherConfigs: (T | undefined)[] -): T { - return merge({}, ...grapherConfigs) -} - /** Works for: * #dod:text * #dod:text-hyphenated @@ -1970,3 +1966,20 @@ export function traverseObjects>( } return result } + +export function getParentIndicatorIdFromChartConfig( + config: GrapherInterface +): number | undefined { + const { type, dimensions } = config + + if (type === ChartTypeName.ScatterPlot) return undefined + if (!dimensions) return undefined + + const yVariableIds = dimensions + .filter((d) => d.property === DimensionProperty.y) + .map((d) => d.variableId) + + if (yVariableIds.length !== 1) return undefined + + return yVariableIds[0] +} diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts index e18511ec46d..cf0886ae99a 100644 --- a/packages/@ourworldindata/utils/src/grapherConfigUtils.ts +++ b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts @@ -7,8 +7,11 @@ import { excludeUndefined, omitUndefinedValuesRecursive, traverseObjects, + isEmpty, } from "./Util" +const REQUIRED_KEYS = ["$schema", "dimensions"] + const KEYS_EXCLUDED_FROM_INHERITANCE = [ "$schema", "id", @@ -22,12 +25,11 @@ export function mergeGrapherConfigs( ): GrapherInterface { // warn if one of the configs is missing a schema version const configsWithoutSchema = grapherConfigs.filter( - (c) => c["$schema"] === undefined + (c) => !isEmpty(c) && c["$schema"] === undefined ) if (configsWithoutSchema.length > 0) { - const ids = configsWithoutSchema.map((c) => c.id) console.warn( - `About to merge Grapher configs with missing schema information. Charts with missing schema information: ${ids}` + `About to merge Grapher configs with missing schema information: ${configsWithoutSchema}` ) } @@ -68,9 +70,10 @@ export function diffGrapherConfigs( return omitUndefinedValuesRecursive( traverseObjects(config, reference, (value, refValue, key) => { if (KEYS_EXCLUDED_FROM_INHERITANCE.includes(key)) return value + if (REQUIRED_KEYS.includes(key)) return value if (refValue === undefined) return value if (!isEqual(value, refValue)) return value - else return undefined + return undefined }) ) } diff --git a/packages/@ourworldindata/utils/src/index.ts b/packages/@ourworldindata/utils/src/index.ts index 7dd328dd516..f66126f98ec 100644 --- a/packages/@ourworldindata/utils/src/index.ts +++ b/packages/@ourworldindata/utils/src/index.ts @@ -109,7 +109,6 @@ export { type NodeWithUrl, filterValidStringValues, traverseEnrichedSpan, - mergePartialGrapherConfigs, copyToClipboard, checkIsGdocPost, checkIsGdocPostExcludingFragments, @@ -124,6 +123,7 @@ export { createTagGraph, formatInlineList, lazy, + getParentIndicatorIdFromChartConfig, } from "./Util.js" export { diff --git a/site/DataPageV2.tsx b/site/DataPageV2.tsx index 259f2044379..6d31df799d7 100644 --- a/site/DataPageV2.tsx +++ b/site/DataPageV2.tsx @@ -9,7 +9,7 @@ import { SiteFooterContext, DataPageDataV2, serializeJSONForHTML, - mergePartialGrapherConfigs, + mergeGrapherConfigs, compact, FaqEntryData, pick, @@ -87,10 +87,9 @@ export const DataPageV2 = (props: { compact(grapher?.dimensions?.map((d) => d.variableId)) ) - // TODO(inheritance): use mergeGrapherConfigs instead - const mergedGrapherConfig = mergePartialGrapherConfigs( + const mergedGrapherConfig = mergeGrapherConfigs( datapageData.chartConfig as GrapherInterface, - grapher + grapher ?? {} ) // Note that we cannot set `bindUrlToWindow` and `isEmbeddedInADataPage` here,