diff --git a/adminSiteClient/AbstractChartEditor.ts b/adminSiteClient/AbstractChartEditor.ts index 8a1b08f8bb3..e25ddb3cac8 100644 --- a/adminSiteClient/AbstractChartEditor.ts +++ b/adminSiteClient/AbstractChartEditor.ts @@ -6,7 +6,7 @@ import { mergeGrapherConfigs, merge, } from "@ourworldindata/utils" -import { computed, observable, when } from "mobx" +import { action, computed, observable, when } from "mobx" import { EditorFeatures } from "./EditorFeatures.js" import { Admin } from "./Admin.js" import { defaultGrapherConfig, Grapher } from "@ourworldindata/grapher" @@ -28,6 +28,7 @@ export interface AbstractChartEditorManager { admin: Admin patchConfig: GrapherInterface parentConfig?: GrapherInterface + isInheritanceEnabled?: boolean } export abstract class AbstractChartEditor< @@ -42,7 +43,11 @@ export abstract class AbstractChartEditor< @observable.ref previewMode: "mobile" | "desktop" @observable.ref showStaticPreview = false @observable.ref savedPatchConfig: GrapherInterface = {} + + // parent config derived from the current chart config (not necessarily in use) @observable.ref parentConfig: GrapherInterface | undefined = undefined + // if inheritance is enabled, the parent config is applied to grapher + @observable.ref isInheritanceEnabled: boolean | undefined = undefined constructor(props: { manager: Manager }) { this.manager = props.manager @@ -56,6 +61,12 @@ export abstract class AbstractChartEditor< () => (this.parentConfig = this.manager.parentConfig) ) + when( + () => this.manager.isInheritanceEnabled !== undefined, + () => + (this.isInheritanceEnabled = this.manager.isInheritanceEnabled) + ) + when( () => this.grapher.hasData && this.grapher.isReady, () => (this.savedPatchConfig = this.patchConfig) @@ -63,7 +74,7 @@ export abstract class AbstractChartEditor< } /** default object with all possible keys */ - private fullDefaultObject = merge( + fullDefaultObject = merge( {}, Grapher.defaultObject(), // contains all keys defaultGrapherConfig // contains a subset of keys with the right defaults @@ -81,16 +92,27 @@ export abstract class AbstractChartEditor< return mergeGrapherConfigs(this.fullDefaultObject, this.grapher.object) } + /** patch config of the chart that is written to the db on save */ @computed get patchConfig(): GrapherInterface { return diffGrapherConfigs( this.fullConfig, - this.parentConfigWithDefaults ?? this.fullDefaultObject + this.activeParentConfigWithDefaults ?? this.fullDefaultObject ) } - @computed get parentConfigWithDefaults(): GrapherInterface | undefined { - if (!this.parentConfig) return undefined - return mergeGrapherConfigs(this.fullDefaultObject, this.parentConfig) + /** parent config currently applied to grapher */ + @computed get activeParentConfig(): GrapherInterface | undefined { + return this.isInheritanceEnabled ? this.parentConfig : undefined + } + + @computed get activeParentConfigWithDefaults(): + | GrapherInterface + | undefined { + if (!this.activeParentConfig) return undefined + return mergeGrapherConfigs( + this.fullDefaultObject, + this.activeParentConfig + ) } @computed get isModified(): boolean { @@ -104,12 +126,18 @@ export abstract class AbstractChartEditor< return new EditorFeatures(this) } + @action.bound updateLiveGrapher(config: GrapherInterface): void { + this.grapher.reset() + this.grapher.updateFromObject(config) + this.grapher.updateAuthoredVersion(config) + } + // TODO: only works for first level at the moment isPropertyInherited(property: keyof GrapherInterface): boolean { - if (!this.parentConfig) return false + if (!this.activeParentConfig) return false return ( !Object.hasOwn(this.patchConfig, property) && - Object.hasOwn(this.parentConfig, property) + Object.hasOwn(this.activeParentConfig, property) ) } diff --git a/adminSiteClient/ChartEditor.ts b/adminSiteClient/ChartEditor.ts index 01f3c9d92db..c821828b9b6 100644 --- a/adminSiteClient/ChartEditor.ts +++ b/adminSiteClient/ChartEditor.ts @@ -12,11 +12,11 @@ import { ChartRedirect, Json, GrapherInterface, - getParentIndicatorIdFromChartConfig, + getParentVariableIdFromChartConfig, mergeGrapherConfigs, isEmpty, } from "@ourworldindata/utils" -import { action, computed, observable, runInAction } from "mobx" +import { action, computed, observable, runInAction, when } from "mobx" import { BAKED_GRAPHER_URL } from "../settings/clientSettings.js" import { AbstractChartEditor, @@ -98,7 +98,7 @@ export class ChartEditor extends AbstractChartEditor { @action.bound async updateParentConfig() { const currentParentIndicatorId = this.parentConfig?.dimensions?.[0].variableId - const newParentIndicatorId = getParentIndicatorIdFromChartConfig( + const newParentIndicatorId = getParentVariableIdFromChartConfig( this.grapher.object ) @@ -109,22 +109,25 @@ export class ChartEditor extends AbstractChartEditor { (currentParentIndicatorId === undefined || newParentIndicatorId !== currentParentIndicatorId) ) { - newParentConfig = await fetchParentConfigForChart( + newParentConfig = await fetchMergedGrapherConfigByVariableId( this.manager.admin, newParentIndicatorId ) } + // update the live grapher object const newConfig = mergeGrapherConfigs( newParentConfig ?? {}, this.patchConfig ) - - this.grapher.reset() - this.grapher.updateFromObject(newConfig) - this.grapher.updateAuthoredVersion(newConfig) + this.updateLiveGrapher(newConfig) this.parentConfig = newParentConfig + + // disable inheritance if there is no parent config + if (!this.parentConfig) { + this.isInheritanceEnabled = false + } } async saveGrapher({ @@ -137,9 +140,12 @@ export class ChartEditor extends AbstractChartEditor { if (!patchConfig.title) patchConfig.title = grapher.displayTitle if (!patchConfig.slug) patchConfig.slug = grapher.displaySlug + const query = new URLSearchParams({ + inheritance: this.isInheritanceEnabled ? "1" : "0", + }) const targetUrl = isNewGrapher - ? "/api/charts" - : `/api/charts/${grapher.id}` + ? `/api/charts?${query}` + : `/api/charts/${grapher.id}?${query}` const json = await this.manager.admin.requestJSON( targetUrl, @@ -172,8 +178,13 @@ export class ChartEditor extends AbstractChartEditor { // Need to open intermediary tab before AJAX to avoid popup blockers const w = window.open("/", "_blank") as Window + const query = new URLSearchParams({ + inheritance: this.isInheritanceEnabled ? "1" : "0", + }) + const targetUrl = `/api/charts?${query}` + const json = await this.manager.admin.requestJSON( - "/api/charts", + targetUrl, chartJson, "POST" ) @@ -208,7 +219,7 @@ export class ChartEditor extends AbstractChartEditor { } } -export async function fetchParentConfigForChart( +export async function fetchMergedGrapherConfigByVariableId( admin: Admin, indicatorId: number ): Promise { diff --git a/adminSiteClient/ChartEditorPage.tsx b/adminSiteClient/ChartEditorPage.tsx index 8e842a3a14d..829ba9f1078 100644 --- a/adminSiteClient/ChartEditorPage.tsx +++ b/adminSiteClient/ChartEditorPage.tsx @@ -2,7 +2,7 @@ import React from "react" import { observer } from "mobx-react" import { observable, computed, runInAction, action } from "mobx" import { - getParentIndicatorIdFromChartConfig, + getParentVariableIdFromChartConfig, RawPageview, isEmpty, } from "@ourworldindata/utils" @@ -13,7 +13,7 @@ import { Log, References, ChartEditorManager, - fetchParentConfigForChart, + fetchMergedGrapherConfigByVariableId, } from "./ChartEditor.js" import { AdminAppContext, AdminAppContextType } from "./AdminAppContext.js" import { ChartEditorView, ChartEditorViewManager } from "./ChartEditorView.js" @@ -33,6 +33,8 @@ export class ChartEditorPage patchConfig: GrapherInterface = {} parentConfig: GrapherInterface | undefined = undefined + isInheritanceEnabled: boolean | undefined = undefined + async fetchGrapherConfig(): Promise { const { grapherId, grapherConfig } = this.props if (grapherId !== undefined) { @@ -47,19 +49,21 @@ export class ChartEditorPage async fetchParentConfig(): Promise { const { grapherId, grapherConfig } = this.props if (grapherId !== undefined) { - const parentConfig = await this.context.admin.getJSON( - `/api/charts/${grapherId}.parentConfig.json` + const parent = await this.context.admin.getJSON( + `/api/charts/${grapherId}.parent.json` ) - this.parentConfig = isEmpty(parentConfig) ? undefined : parentConfig + this.parentConfig = parent?.config + this.isInheritanceEnabled = parent?.isActive ?? false } else if (grapherConfig) { const parentIndicatorId = - getParentIndicatorIdFromChartConfig(grapherConfig) + getParentVariableIdFromChartConfig(grapherConfig) if (parentIndicatorId) { - this.parentConfig = await fetchParentConfigForChart( + this.parentConfig = await fetchMergedGrapherConfigByVariableId( this.context.admin, parentIndicatorId ) } + this.isInheritanceEnabled = false } } diff --git a/adminSiteClient/EditorBasicTab.tsx b/adminSiteClient/EditorBasicTab.tsx index 06f0133658f..eb3c5bf1b9f 100644 --- a/adminSiteClient/EditorBasicTab.tsx +++ b/adminSiteClient/EditorBasicTab.tsx @@ -181,8 +181,9 @@ class DimensionSlotView< } @action.bound private updateParentConfig() { - if (isChartEditorInstance(this.props.editor)) { - void this.props.editor.updateParentConfig() + const { editor } = this.props + if (isChartEditorInstance(editor)) { + void editor.updateParentConfig() } } diff --git a/adminSiteClient/EditorInheritanceTab.tsx b/adminSiteClient/EditorInheritanceTab.tsx index 8c3e0e6a107..d3801abcac6 100644 --- a/adminSiteClient/EditorInheritanceTab.tsx +++ b/adminSiteClient/EditorInheritanceTab.tsx @@ -1,54 +1,84 @@ import React from "react" import { observer } from "mobx-react" -import { AbstractChartEditor } from "./AbstractChartEditor.js" -import { Section } from "./Forms.js" +import { Section, Toggle } from "./Forms.js" +import { ChartEditor } from "./ChartEditor.js" +import { action } from "mobx" +import { mergeGrapherConfigs } from "@ourworldindata/utils" @observer -export class EditorInheritanceTab< - Editor extends AbstractChartEditor, -> extends React.Component<{ editor: Editor }> { - render() { - const { parentConfig, fullConfig, grapher } = this.props.editor +export class EditorInheritanceTab extends React.Component<{ + editor: ChartEditor +}> { + @action.bound onToggleInheritance(newValue: boolean) { + const { patchConfig, parentConfig } = this.props.editor + + // update live grapher + const newParentConfig = newValue ? parentConfig : undefined + const newConfig = mergeGrapherConfigs( + newParentConfig ?? {}, + patchConfig + ) + this.props.editor.updateLiveGrapher(newConfig) - if (!parentConfig) - return ( -
- Doesn't inherit settings from any indicator -
- ) + this.props.editor.isInheritanceEnabled = newValue + } - const parentIndicatorId = parentConfig.dimensions?.[0].variableId + render() { + const { + parentConfig, + isInheritanceEnabled = false, + fullConfig, + grapher, + } = this.props.editor - if (!parentIndicatorId) - return ( -
- Does inherit settings from any indicator but can't find the - indicator's id (shouldn't happen, please report this bug!) -
- ) + const parentIndicatorId = parentConfig?.dimensions?.[0].variableId + const column = parentIndicatorId + ? grapher.inputTable.get(parentIndicatorId.toString()) + : undefined - const column = grapher.inputTable.get(parentIndicatorId.toString()) + const parentVariableEditLink = ( + + {column?.name ?? parentIndicatorId} + + ) return (
- Inherits settings from indicator{" "} - - {column.name}. - -
-
-