wip: remove defaultGrapherConfig from admin

owid · Nov 1, 2024 · a0bb2cf · a0bb2cf
1 parent 9bc51a9
commit a0bb2cf
Show file tree

Hide file tree

Showing 16 changed files with 299 additions and 128 deletions.
diff --git a/adminSiteClient/AbstractChartEditor.ts b/adminSiteClient/AbstractChartEditor.ts
@@ -8,7 +8,9 @@ import {
 import { action, computed, observable, when } from "mobx"
 import { EditorFeatures } from "./EditorFeatures.js"
 import { Admin } from "./Admin.js"
-import { defaultGrapherConfig, Grapher } from "@ourworldindata/grapher"
+import { Grapher } from "@ourworldindata/grapher"
+
+const defaultGrapherObject = Grapher.defaultObject()
 
 export type EditorTab =
     | "basic"
@@ -86,7 +88,7 @@ export abstract class AbstractChartEditor<
     }
 
     @computed get liveConfigWithDefaults(): GrapherInterface {
-        return mergeGrapherConfigs(defaultGrapherConfig, this.liveConfig)
+        return mergeGrapherConfigs(defaultGrapherObject, this.liveConfig)
     }
 
     /** full config: patch config merged with parent config */
@@ -105,7 +107,7 @@ export abstract class AbstractChartEditor<
         | undefined {
         if (!this.activeParentConfig) return undefined
         return mergeGrapherConfigs(
-            defaultGrapherConfig,
+            defaultGrapherObject,
             this.activeParentConfig
         )
     }
@@ -114,7 +116,7 @@ export abstract class AbstractChartEditor<
     @computed get patchConfig(): GrapherInterface {
         return diffGrapherConfigs(
             this.liveConfigWithDefaults,
-            this.activeParentConfigWithDefaults ?? defaultGrapherConfig
+            this.activeParentConfigWithDefaults ?? defaultGrapherObject
         )
     }
 

diff --git a/adminSiteClient/ChartEditorPage.tsx b/adminSiteClient/ChartEditorPage.tsx
@@ -57,6 +57,7 @@ export class ChartEditorPage
             )
             this.parentConfig = parent?.config
             this.isInheritanceEnabled = parent?.isActive ?? true
+            if (this.parentConfig) this.parentConfig.title = "parent title"
         } else if (grapherConfig) {
             const parentIndicatorId =
                 getParentVariableIdFromChartConfig(grapherConfig)

diff --git a/adminSiteServer/app.test.tsx b/adminSiteServer/app.test.tsx
@@ -249,36 +249,22 @@ describe("OwidAdminApp", () => {
         )?.config
         expect(parentConfig).toBeUndefined()
 
-        // fetch the full config and verify it's merged with the default
+        // fetch the full config and verify that id, version and isPublished are added
         const fullConfig = await fetchJsonFromAdminApi(
             `/charts/${chartId}.config.json`
         )
-        expect(fullConfig).toHaveProperty(
-            "$schema",
-            "https://files.ourworldindata.org/schemas/grapher-schema.005.json"
-        )
-        expect(fullConfig).toHaveProperty("id", chartId) // must match the db id
-        expect(fullConfig).toHaveProperty("version", 1) // automatically added
-        expect(fullConfig).toHaveProperty("isPublished", false) // automatically added
-        expect(fullConfig).toHaveProperty("slug", "test-chart")
-        expect(fullConfig).toHaveProperty("title", "Test chart")
-        expect(fullConfig).toHaveProperty("type", "LineChart") // default property
-        expect(fullConfig).toHaveProperty("tab", "chart") // default property
+        expect(fullConfig).toEqual({
+            ...testChartConfig,
+            id: chartId, // must match the db id
+            version: 1, // automatically added
+            isPublished: false, // automatically added
+        })
 
-        // fetch the patch config and verify it's diffed correctly
+        // fetch the patch config and verify it's identical to the full config
         const patchConfig = await fetchJsonFromAdminApi(
             `/charts/${chartId}.patchConfig.json`
         )
-        expect(patchConfig).toEqual({
-            $schema:
-                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
-            id: chartId,
-            version: 1,
-            isPublished: false,
-            slug: "test-chart",
-            title: "Test chart",
-            // note that the type is not included
-        })
+        expect(patchConfig).toEqual(fullConfig)
     })
 
     it("should be able to create a GDoc article", async () => {
@@ -620,7 +606,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         expect(parentConfig).toEqual(mergedGrapherConfig)
 
         // fetch the full config of the chart and verify that it's been merged
-        // with the indicator config and the default config
+        // with the indicator config
         const fullConfig = await fetchJsonFromAdminApi(
             `/charts/${chartId}.config.json`
         )
@@ -632,7 +618,6 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         expect(fullConfig).toHaveProperty("note", "Indicator note") // inherited from variable
         expect(fullConfig).toHaveProperty("hasMapTab", true) // inherited from variable
         expect(fullConfig).toHaveProperty("subtitle", "Admin subtitle") // inherited from variable
-        expect(fullConfig).toHaveProperty("tab", "chart") // default value
 
         // fetch the patch config and verify it's diffed correctly
         const patchConfig = await fetchJsonFromAdminApi(

diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts
@@ -2,7 +2,8 @@ import { BASE_FONT_SIZE } from "../core/GrapherConstants"
 import {
     extend,
     trimObject,
-    deleteRuntimeAndUnchangedProps,
+    deleteRuntimeProps,
+    deleteUnchangedProps,
     Persistable,
     AxisAlign,
     Position,
@@ -94,7 +95,39 @@ export class AxisConfig
             domainValues: this.domainValues,
         }) as AxisConfigInterface
 
-        deleteRuntimeAndUnchangedProps(obj, new AxisConfigDefaults())
+        deleteRuntimeProps(obj, new AxisConfigDefaults())
+        deleteUnchangedProps(obj, new AxisConfigDefaults())
+
+        if (obj.min === undefined) obj.min = AxisMinMaxValueStr.auto
+        if (obj.max === undefined) obj.max = AxisMinMaxValueStr.auto
+
+        return obj
+    }
+
+    toDefaultObject(): AxisConfigInterface {
+        const obj = trimObject({
+            orient: this.orient,
+            min: this.min,
+            max: this.max,
+            canChangeScaleType: this.canChangeScaleType,
+            removePointsOutsideDomain: this.removePointsOutsideDomain,
+            minSize: this.minSize,
+            hideAxis: this.hideAxis,
+            hideGridlines: this.hideGridlines,
+            hideTickLabels: this.hideTickLabels,
+            labelPadding: this.labelPadding,
+            nice: this.nice,
+            maxTicks: this.maxTicks,
+            tickFormattingOptions: this.tickFormattingOptions,
+            scaleType: this.scaleType,
+            label: this.label ? this.label : undefined,
+            facetDomain: this.facetDomain,
+            ticks: this.ticks,
+            singleValueAxisPointAlign: this.singleValueAxisPointAlign,
+            domainValues: this.domainValues,
+        }) as AxisConfigInterface
+
+        deleteRuntimeProps(obj, new AxisConfigDefaults())
 
         if (obj.min === undefined) obj.min = AxisMinMaxValueStr.auto
         if (obj.max === undefined) obj.max = AxisMinMaxValueStr.auto

diff --git a/packages/@ourworldindata/grapher/src/chart/ChartDimension.ts b/packages/@ourworldindata/grapher/src/chart/ChartDimension.ts
@@ -8,7 +8,7 @@ import {
     DimensionProperty,
     OwidVariableId,
     Persistable,
-    deleteRuntimeAndUnchangedProps,
+    deleteUnchangedProps,
     updatePersistables,
     OwidVariableDisplayConfig,
     OwidChartDimensionInterface,
@@ -65,7 +65,7 @@ export class ChartDimension
 
     toObject(): OwidChartDimensionInterface {
         return trimObject(
-            deleteRuntimeAndUnchangedProps(
+            deleteUnchangedProps(
                 {
                     property: this.property,
                     variableId: this.variableId,
@@ -77,6 +77,15 @@ export class ChartDimension
         )
     }
 
+    toDefaultObject(): OwidChartDimensionInterface {
+        return trimObject({
+            property: this.property,
+            variableId: this.variableId,
+            display: this.display,
+            targetYear: this.targetYear,
+        })
+    }
+
     // Do not persist yet, until we migrate off VariableIds
     @observable slug?: ColumnSlug
 

diff --git a/packages/@ourworldindata/grapher/src/color/ColorScaleConfig.ts b/packages/@ourworldindata/grapher/src/color/ColorScaleConfig.ts
@@ -10,7 +10,8 @@ import {
     extend,
     isEmpty,
     trimObject,
-    deleteRuntimeAndUnchangedProps,
+    deleteUnchangedProps,
+    deleteRuntimeProps,
     objectWithPersistablesToObject,
     Persistable,
     updatePersistables,
@@ -95,7 +96,15 @@ export class ColorScaleConfig
     toObject(): ColorScaleConfigInterface {
         const obj: ColorScaleConfigInterface =
             objectWithPersistablesToObject(this)
-        deleteRuntimeAndUnchangedProps(obj, new ColorScaleConfigDefaults())
+        deleteRuntimeProps(obj, new ColorScaleConfigDefaults())
+        deleteUnchangedProps(obj, new ColorScaleConfigDefaults())
+        return trimObject(obj)
+    }
+
+    toDefaultObject(): ColorScaleConfigInterface {
+        const obj: ColorScaleConfigInterface =
+            objectWithPersistablesToObject(this)
+        deleteRuntimeProps(obj, new ColorScaleConfigDefaults())
         return trimObject(obj)
     }
 

diff --git a/packages/@ourworldindata/grapher/src/controls/settings/TableFilterToggle.tsx b/packages/@ourworldindata/grapher/src/controls/settings/TableFilterToggle.tsx
@@ -32,7 +32,7 @@ export class TableFilterToggle extends React.Component<{
     @action.bound onToggle(): void {
         const { manager } = this.props
         manager.showSelectionOnlyInDataTable =
-            manager.showSelectionOnlyInDataTable ? undefined : true
+            !manager.showSelectionOnlyInDataTable
     }
 
     render(): React.ReactElement {

diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts b/packages/@ourworldindata/grapher/src/core/Grapher.jsdom.test.ts
@@ -78,9 +78,6 @@ it("can get dimension slots", () => {
 it("an empty Grapher serializes to an object that includes only the schema", () => {
     expect(new Grapher().toObject()).toEqual({
         $schema: defaultGrapherConfig.$schema,
-
-        // TODO: ideally, selectedEntityNames is not serialised for an empty object
-        selectedEntityNames: [],
     })
 })
 
@@ -91,18 +88,12 @@ it("a bad chart type does not crash grapher", () => {
     expect(new Grapher(input).toObject()).toEqual({
         ...input,
         $schema: defaultGrapherConfig.$schema,
-
-        // TODO: ideally, selectedEntityNames is not serialised for an empty object
-        selectedEntityNames: [],
     })
 })
 
 it("does not preserve defaults in the object (except for the schema)", () => {
     expect(new Grapher({ tab: GrapherTabOption.chart }).toObject()).toEqual({
         $schema: defaultGrapherConfig.$schema,
-
-        // TODO: ideally, selectedEntityNames is not serialised for an empty object
-        selectedEntityNames: [],
     })
 })