✨ (grapher) support multiple chart types

owid · Nov 14, 2024 · 603bca2 · 603bca2
1 parent 1e07f96
commit 603bca2
Show file tree

Hide file tree

Showing 45 changed files with 830 additions and 310 deletions.
diff --git a/adminSiteClient/BulkGrapherConfigEditor.tsx b/adminSiteClient/BulkGrapherConfigEditor.tsx
@@ -79,7 +79,7 @@ const bulkChartEditorColumnSets: ColumnSet[] = [
         label: "Common",
         kind: "specificColumns",
         columns: [
-            "/type",
+            "/chartTypes",
             "/hasMapTab",
             "/title",
             "/subtitle",
@@ -110,9 +110,8 @@ const bulkChartEditorColumnSets: ColumnSet[] = [
             "/baseColorScheme",
             "/map/colorScale",
             "/colorScale",
-            "/hasChartTab",
             "/hasMapTab",
-            "/type",
+            "/chartTypes",
         ],
     },
 ]

diff --git a/adminSiteClient/ChartList.tsx b/adminSiteClient/ChartList.tsx
@@ -3,7 +3,11 @@ import { observer } from "mobx-react"
 import { runInAction, observable } from "mobx"
 import { bind } from "decko"
 import { AdminAppContext, AdminAppContextType } from "./AdminAppContext.js"
-import { ChartTypeName, GrapherInterface } from "@ourworldindata/types"
+import {
+    ChartTypeName,
+    GrapherInterface,
+    GrapherTabOption,
+} from "@ourworldindata/types"
 import { startCase, DbChartTagJoin } from "@ourworldindata/utils"
 import { References, getFullReferencesCount } from "./ChartEditor.js"
 import { ChartRow } from "./ChartRow.js"
@@ -14,14 +18,15 @@ export interface ChartListItem {
     id: GrapherInterface["id"]
     title: GrapherInterface["title"]
     slug: GrapherInterface["slug"]
-    type: GrapherInterface["type"]
     internalNotes: GrapherInterface["internalNotes"]
     variantName: GrapherInterface["variantName"]
     isPublished: GrapherInterface["isPublished"]
     tab: GrapherInterface["tab"]
-    hasChartTab: GrapherInterface["hasChartTab"]
     hasMapTab: GrapherInterface["hasMapTab"]
 
+    type?: ChartTypeName
+    hasChartTab: boolean
+
     lastEditedAt: string
     lastEditedBy: string
     publishedAt: string
@@ -142,13 +147,16 @@ export class ChartList extends React.Component<{
     }
 }
 
-export function showChartType(chart: ChartListItem) {
-    const chartType = chart.type ?? ChartTypeName.LineChart
+export function showChartType(chart: ChartListItem): string {
+    const chartType = chart.type
+
+    if (!chartType) return "Map"
+
     const displayType = ChartTypeName[chartType]
         ? startCase(ChartTypeName[chartType])
         : "Unknown"
 
-    if (chart.tab === "map") {
+    if (chart.tab === GrapherTabOption.map) {
         if (chart.hasChartTab) return `Map + ${displayType}`
         else return "Map"
     } else {

diff --git a/adminSiteClient/EditorBasicTab.tsx b/adminSiteClient/EditorBasicTab.tsx
@@ -147,7 +147,10 @@ class DimensionSlotView<
             () => this.grapher.isReady,
             () => {
                 this.disposers.push(
-                    reaction(() => this.grapher.type, this.updateDefaults),
+                    reaction(
+                        () => this.grapher.validChartTypes,
+                        this.updateDefaults
+                    ),
                     reaction(
                         () => this.grapher.yColumnsFromDimensions.length,
                         this.updateDefaults
@@ -355,7 +358,9 @@ export class EditorBasicTab<
 
     @action.bound onChartTypeChange(value: string) {
         const { grapher } = this.props.editor
-        grapher.type = value as ChartTypeName
+
+        const newChartType = value as ChartTypeName
+        grapher.chartTypes = [newChartType]
 
         if (grapher.isMarimekko) {
             grapher.hideRelativeToggle = false
@@ -395,7 +400,7 @@ export class EditorBasicTab<
     render() {
         const { editor } = this.props
         const { grapher } = editor
-        const chartTypes = Object.keys(ChartTypeName).filter(
+        const allChartTypes = Object.keys(ChartTypeName).filter(
             (chartType) => chartType !== ChartTypeName.WorldMap
         )
 
@@ -407,9 +412,9 @@ export class EditorBasicTab<
 
                 <Section name="Type of chart">
                     <SelectField
-                        value={grapher.type}
+                        value={grapher.chartType}
                         onValue={this.onChartTypeChange}
-                        options={chartTypes.map((key) => ({
+                        options={allChartTypes.map((key) => ({
                             value: key,
                             label: startCase(key),
                         }))}
@@ -418,12 +423,18 @@ export class EditorBasicTab<
                         <Toggle
                             label="Chart tab"
                             value={grapher.hasChartTab}
-                            onValue={(value) => (grapher.hasChartTab = value)}
+                            onValue={(shouldHaveChartTab) =>
+                                (grapher.chartTypes = shouldHaveChartTab
+                                    ? [ChartTypeName.LineChart]
+                                    : [])
+                            }
                         />
                         <Toggle
                             label="Map tab"
                             value={grapher.hasMapTab}
-                            onValue={(value) => (grapher.hasMapTab = value)}
+                            onValue={(shouldHaveMapTab) =>
+                                (grapher.hasMapTab = shouldHaveMapTab)
+                            }
                         />
                     </FieldsRow>
                 </Section>

diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx
@@ -6,6 +6,7 @@ import {
     ColorSchemeName,
     FacetAxisDomain,
     FacetStrategy,
+    ChartTypeName,
 } from "@ourworldindata/types"
 import { Grapher } from "@ourworldindata/grapher"
 import {
@@ -158,7 +159,10 @@ export class ColorSchemeSelector extends React.Component<{
                             value={grapher.baseColorScheme}
                             onChange={this.onChange}
                             onBlur={this.onBlur}
-                            chartType={this.props.grapher.type}
+                            chartType={
+                                this.props.grapher.chartType ??
+                                ChartTypeName.LineChart
+                            }
                             invertedColorScheme={!!grapher.invertColorScheme}
                             additionalOptions={[
                                 {
@@ -751,7 +755,7 @@ export class EditorCustomizeTab<
                 {grapher.chartInstanceExceptMap.colorScale && (
                     <EditorColorScaleSection
                         scale={grapher.chartInstanceExceptMap.colorScale}
-                        chartType={grapher.type}
+                        chartType={grapher.chartType ?? ChartTypeName.LineChart}
                         showLineChartColors={grapher.isLineChart}
                         features={{
                             visualScaling: true,

diff --git a/adminSiteClient/GrapherConfigGridEditor.tsx b/adminSiteClient/GrapherConfigGridEditor.tsx
@@ -512,7 +512,9 @@ export class GrapherConfigGridEditor extends React.Component<GrapherConfigGridEd
                         return (
                             <EditorColorScaleSection
                                 scale={colorScale}
-                                chartType={grapher.type}
+                                chartType={
+                                    grapher.chartType ?? ChartTypeName.LineChart
+                                }
                                 features={{
                                     visualScaling: true,
                                     legendDescription: false,

diff --git a/adminSiteClient/VariablesAnnotationPage.tsx b/adminSiteClient/VariablesAnnotationPage.tsx
@@ -93,7 +93,7 @@ const variableAnnotationsColumnSets: ColumnSet[] = [
         columns: [
             "name",
             "datasetname",
-            "/type",
+            "/chartTypes",
             "/hasMapTab",
             "/title",
             "/subtitle",
@@ -126,9 +126,8 @@ const variableAnnotationsColumnSets: ColumnSet[] = [
             "/baseColorScheme",
             "/map/colorScale",
             "/colorScale",
-            "/hasChartTab",
             "/hasMapTab",
-            "/type",
+            "/chartTypes",
         ],
     },
 ]

diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts
@@ -785,7 +785,7 @@ getRouteWithROTransaction(apiRouter, "/charts.csv", async (req, res, trx) => {
                 chart_configs.full->>"$.variantName" AS variantName,
                 chart_configs.full->>"$.isPublished" AS isPublished,
                 chart_configs.full->>"$.tab" AS tab,
-                JSON_EXTRACT(chart_configs.full, "$.hasChartTab") = true AS hasChartTab,
+                chart_configs.chartType IS NOT NULL AS hasChartTab,
                 JSON_EXTRACT(chart_configs.full, "$.hasMapTab") = true AS hasMapTab,
                 chart_configs.full->>"$.originUrl" AS originUrl,
                 charts.lastEditedAt,

diff --git a/adminSiteServer/app.test.tsx b/adminSiteServer/app.test.tsx
@@ -58,12 +58,15 @@ import {
 import path from "path"
 import fs from "fs"
 import { omitUndefinedValues } from "@ourworldindata/utils"
+import { defaultGrapherConfig } from "@ourworldindata/grapher"
 
 const ADMIN_SERVER_HOST = "localhost"
 const ADMIN_SERVER_PORT = 8765
 
 const ADMIN_URL = `http://${ADMIN_SERVER_HOST}:${ADMIN_SERVER_PORT}/admin/api`
 
+const currentSchema = defaultGrapherConfig.$schema
+
 jest.setTimeout(10000) // wait for up to 10s for the app server to start
 let testKnexInstance: Knex<any, unknown[]> | undefined = undefined
 let serverKnexInstance: Knex<any, unknown[]> | undefined = undefined
@@ -192,8 +195,7 @@ async function makeRequestAgainstAdminApi(
 
 describe("OwidAdminApp", () => {
     const testChartConfig = {
-        $schema:
-            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+        $schema: currentSchema,
         slug: "test-chart",
         title: "Test chart",
         type: "LineChart",
@@ -325,16 +327,14 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         display: '{ "unit": "kg", "shortUnit": "kg" }',
     }
     const testVariableConfigETL = {
-        $schema:
-            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+        $schema: currentSchema,
         hasMapTab: true,
         note: "Indicator note",
         selectedEntityNames: ["France", "Italy", "Spain"],
         hideRelativeToggle: false,
     }
     const testVariableConfigAdmin = {
-        $schema:
-            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+        $schema: currentSchema,
         title: "Admin title",
         subtitle: "Admin subtitle",
     }
@@ -345,14 +345,12 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         id: otherVariableId,
     }
     const otherTestVariableConfig = {
-        $schema:
-            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+        $schema: currentSchema,
         note: "Other indicator note",
     }
 
     const testChartConfig = {
-        $schema:
-            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+        $schema: currentSchema,
         slug: "test-chart",
         title: "Test chart",
         type: "Marimekko",
@@ -366,8 +364,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         ],
     }
     const testMultiDimConfig = {
-        grapherConfigSchema:
-            "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+        grapherConfigSchema: currentSchema,
         title: {
             title: "Energy use",
             titleVariant: "by energy source",
@@ -621,7 +618,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`
         )
@@ -640,8 +637,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
             `/charts/${chartId}.patchConfig.json`
         )
         expect(patchConfig).toEqual({
-            $schema:
-                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+            $schema: currentSchema,
             id: chartId,
             version: 1,
             isPublished: false,
@@ -696,8 +692,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
             `/charts/${chartId}.patchConfig.json`
         )
         expect(patchConfigAfterDelete).toEqual({
-            $schema:
-                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+            $schema: currentSchema,
             id: chartId,
             version: 1,
             isPublished: false,

diff --git a/adminSiteServer/testPageRouter.tsx b/adminSiteServer/testPageRouter.tsx
@@ -153,17 +153,11 @@ async function propsFromQueryParams(
             tab = tab || GrapherTabOption.map
         } else {
             if (params.type === "LineChart") {
-                query = query.andWhereRaw(
-                    `(
-                        cc.full->"$.type" = "LineChart"
-                        OR cc.full->"$.type" IS NULL
-                    ) AND COALESCE(cc.full->>"$.hasChartTab", "true") = "true"`
-                )
+                query = query.andWhereRaw(`cc.chartType = "LineChart"`)
             } else {
-                query = query.andWhereRaw(
-                    `cc.full->"$.type" = :type AND COALESCE(cc.full->>"$.hasChartTab", "true") = "true"`,
-                    { type: params.type }
-                )
+                query = query.andWhereRaw(`cc.chartType = :type`, {
+                    type: params.type,
+                })
             }
             tab = tab || GrapherTabOption.chart
         }
@@ -245,9 +239,7 @@ async function propsFromQueryParams(
     if (tab === GrapherTabOption.map) {
         query = query.andWhereRaw(`cc.full->>"$.hasMapTab" = "true"`)
     } else if (tab === GrapherTabOption.chart) {
-        query = query.andWhereRaw(
-            `COALESCE(cc.full->>"$.hasChartTab", "true") = "true"`
-        )
+        query = query.andWhereRaw(`cc.chartType IS NOT NULL`)
     }
 
     if (datasetIds.length > 0) {

diff --git a/baker/countryProfiles.tsx b/baker/countryProfiles.tsx
@@ -36,9 +36,12 @@ function bakeCache<T>(cacheKey: any, retriever: () => T): T {
     return result
 }
 
+const hasChartTab = (grapher: GrapherInterface): boolean =>
+    !grapher.chartTypes || grapher.chartTypes.length > 0
+
 const checkShouldShowIndicator = (grapher: GrapherInterface) =>
-    (grapher.hasChartTab ?? true) &&
-    (grapher.type ?? "LineChart") === "LineChart" &&
+    hasChartTab(grapher) &&
+    (grapher.chartTypes?.[0] ?? "LineChart") === "LineChart" &&
     grapher.dimensions?.length === 1
 
 // Find the charts that will be shown on the country profile page (if they have that country)

diff --git a/db/migration/1661264304751-MigrateSelectedData.ts b/db/migration/1661264304751-MigrateSelectedData.ts
@@ -3,7 +3,9 @@ import { MigrationInterface, QueryRunner } from "typeorm"
 
 import { entityNameById } from "./data/entityNameById.js"
 
-import { ChartTypeName, GrapherInterface } from "@ourworldindata/types"
+import { ChartTypeName } from "@ourworldindata/types"
+
+type GrapherInterface = Record<string, any>
 
 /**
  * Migrate the legacy `selectedData` and get rid of it.