🔨 migrate slope charts / TAS-721

adminSiteClient/EditorBasicTab.tsx

@@ -13,6 +13,7 @@ import {
     StackMode,
     ALL_GRAPHER_CHART_TYPES,
     GrapherChartType,
+    GRAPHER_CHART_TYPES,
 } from "@ourworldindata/types"
 import {
     DimensionSlot,
@@ -109,7 +110,7 @@ class DimensionSlotView<
         const { selection } = grapher
         const { availableEntityNames, availableEntityNameSet } = selection
 
-        if (grapher.isScatter || grapher.isSlopeChart || grapher.isMarimekko) {
+        if (grapher.isScatter || grapher.isMarimekko) {
             // chart types that display all entities by default shouldn't select any by default
             selection.clearSelection()
         } else if (
@@ -367,13 +368,17 @@ export class EditorBasicTab<
                 ? []
                 : [value as GrapherChartType]
 
+        if (grapher.isLineChart) {
+            this.addSlopeChart()
+        }
+
         if (grapher.isMarimekko) {
             grapher.hideRelativeToggle = false
             grapher.stackMode = StackMode.relative
         }
 
-        // Give scatterplots and slope charts a default color dimension if they don't have one
-        if (grapher.isScatter || grapher.isSlopeChart) {
+        // Give scatterplots a default color and size dimensions
+        if (grapher.isScatter) {
             const hasColor = grapher.dimensions.find(
                 (d) => d.property === DimensionProperty.color
             )
@@ -382,10 +387,7 @@ export class EditorBasicTab<
                     variableId: CONTINENTS_INDICATOR_ID,
                     property: DimensionProperty.color,
                 })
-        }
 
-        // Give scatterplots a default size dimension if they don't have one
-        if (grapher.isScatter) {
             const hasSize = grapher.dimensions.find(
                 (d) => d.property === DimensionProperty.size
             )
@@ -417,6 +419,32 @@ export class EditorBasicTab<
         ]
     }
 
+    private addSlopeChart(): void {
+        const { grapher } = this.props.editor
+        if (grapher.hasSlopeChart) return
+        grapher.chartTypes = [
+            ...grapher.chartTypes,
+            GRAPHER_CHART_TYPES.SlopeChart,
+        ]
+    }
+
+    private removeSlopeChart(): void {
+        const { grapher } = this.props.editor
+        grapher.chartTypes = grapher.chartTypes.filter(
+            (type) => type !== GRAPHER_CHART_TYPES.SlopeChart
+        )
+    }
+
+    @action.bound toggleSecondarySlopeChart(
+        shouldHaveSlopeChart: boolean
+    ): void {
+        if (shouldHaveSlopeChart) {
+            this.addSlopeChart()
+        } else {
+            this.removeSlopeChart()
+        }
+    }
+
     render() {
         const { editor } = this.props
         const { grapher } = editor
@@ -441,6 +469,13 @@ export class EditorBasicTab<
                                 (grapher.hasMapTab = shouldHaveMapTab)
                             }
                         />
+                        {grapher.isLineChart && (
+                            <Toggle
+                                label="Slope chart"
+                                value={grapher.hasSlopeChart}
+                                onValue={this.toggleSecondarySlopeChart}
+                            />
+                        )}
                     </FieldsRow>
                 </Section>
                 {!isIndicatorChart && (

adminSiteClient/EditorFeatures.tsx

@@ -62,6 +62,7 @@ export class EditorFeatures {
     @computed get hideLegend() {
         return (
             this.grapher.isLineChart ||
+            this.grapher.isSlopeChart ||
             this.grapher.isStackedArea ||
             this.grapher.isStackedDiscreteBar
         )
@@ -118,9 +119,9 @@ export class EditorFeatures {
             return true
         }
 
-        // for line charts, specifying a missing data strategy only makes sense
+        // for line and slope charts, specifying a missing data strategy only makes sense
         // if there are multiple entities
-        if (this.grapher.isLineChart) {
+        if (this.grapher.isLineChart || this.grapher.isSlopeChart) {
             return (
                 this.grapher.canChangeEntity ||
                 this.grapher.canSelectMultipleEntities

baker/updateChartEntities.ts

@@ -106,7 +106,7 @@ const obtainAvailableEntitiesForGrapherConfig = async (
 
         // In these chart types, an unselected entity is still shown
         const chartTypeShowsUnselectedEntities =
-            grapher.isScatter || grapher.isSlopeChart || grapher.isMarimekko
+            grapher.isScatter || grapher.isMarimekko
 
         if (canChangeEntities || chartTypeShowsUnselectedEntities)
             return grapher.tableForSelection.availableEntityNames as string[]

db/migration/1732195571407-RemoveColorDimensionFromSlopeCharts.ts

@@ -0,0 +1,32 @@
+import { MigrationInterface, QueryRunner } from "typeorm"
+
+export class RemoveColorDimensionFromSlopeCharts1732195571407
+    implements MigrationInterface
+{
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        // remove color dimension for all slope charts
+        // the y-dimension always comes first and the color dimension second,
+        // so it's safe to keep the first dimension only
+        await queryRunner.query(`
+            -- sql
+            UPDATE chart_configs
+            SET
+                patch = JSON_REPLACE(patch, '$.dimensions', JSON_ARRAY(patch -> '$.dimensions[0]')),
+                full = JSON_REPLACE(full, '$.dimensions', JSON_ARRAY(full -> '$.dimensions[0]'))
+            WHERE
+                chartType = 'SlopeChart'
+        `)
+
+        // remove the color dimension for slope charts from the chart_dimensions table
+        await queryRunner.query(`
+            -- sql
+            DELETE cd FROM chart_dimensions cd
+            JOIN charts c ON c.id = cd.chartId
+            JOIN chart_configs cc ON c.configId = cc.id
+            WHERE cc.chartType = 'SlopeChart' AND cd.property = 'color'
+        `)
+    }
+
+    // eslint-disable-next-line @typescript-eslint/no-empty-function
+    public async down(): Promise<void> {}
+}

db/migration/1732291572062-MigrateSlopeCharts.ts

@@ -0,0 +1,156 @@
+import { MigrationInterface, QueryRunner } from "typeorm"
+
+export class MigrateSlopeCharts1732291572062 implements MigrationInterface {
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        // create a temporary table that lists all slope charts and their
+        // corresponding line charts (there might be multiple)
+        await queryRunner.query(`
+            -- sql
+            CREATE TABLE slope_line_charts (
+                variableId integer NOT NULL,
+                slopeChartId integer NOT NULL,
+                slopeChartConfigId varchar(255) NOT NULL,
+                slopeChartSelectedEntityNames JSON,
+                lineChartId integer,
+                lineChartConfigId varchar(255)
+            )
+        `)
+        await queryRunner.query(`
+            INSERT INTO slope_line_charts (
+                variableId,
+                slopeChartId,
+                slopeChartConfigId,
+                slopeChartSelectedEntityNames,
+                lineChartId,
+                lineChartConfigId
+            )
+            SELECT * FROM (
+                WITH line_charts AS (
+                    SELECT
+                        c.id,
+                        c.configId,
+                        cc.full ->> '$.dimensions[0].variableId' as variableId
+                    FROM charts c
+                    JOIN chart_configs cc ON c.configId = cc.id
+                    WHERE
+                        cc.chartType = 'LineChart'
+                        AND JSON_LENGTH(cc.full, '$.dimensions') = 1
+                        AND cc.full ->> '$.isPublished' = 'true'
+                ), slope_charts AS (
+                    SELECT
+                        c.id,
+                        c.configId,
+                        cc.full ->> '$.dimensions[0].variableId' as variableId,
+                        cc.full -> '$.selectedEntityNames' as selectedEntityNames
+                    FROM charts c
+                    JOIN chart_configs cc ON c.configId = cc.id
+                    WHERE
+                        cc.chartType = 'SlopeChart'
+                        AND cc.full ->> '$.isPublished' = 'true'
+                )
+                SELECT
+                    sc.variableId AS variableId,
+                    sc.id AS slopeChartId,
+                    sc.configId AS slopeChartConfigId,
+                    sc.selectedEntityNames AS slopeChartSelectedEntityNames,
+                    lc.id AS lineChartId,
+                    lc.configId AS lineChartConfigId
+                FROM slope_charts sc
+                LEFT JOIN line_charts lc ON lc.variableId = sc.variableId
+            ) AS derived_table;
+        `)
+
+        // STAND-ALONE SLOPE CHARTS
+
+        // make sure entity selection is not disabled
+        await queryRunner.query(`
+            -- sql
+            UPDATE chart_configs cc
+            JOIN slope_line_charts slc ON slc.slopeChartConfigId = cc.id
+            SET
+                cc.patch = JSON_SET(cc.patch, '$.addCountryMode', 'add-country'),
+                cc.full = JSON_SET(cc.full, '$.addCountryMode', 'add-country')
+            WHERE
+                slc.lineChartId IS NULL
+                AND (
+                    cc.full ->> '$.addCountryMode' = 'disabled'
+                    OR cc.full ->> '$.addCountryMode' = 'change-country'
+                )
+        `)
+
+        // make sure the line legend isn't hidden
+        await queryRunner.query(`
+            -- sql
+            UPDATE chart_configs cc
+            JOIN slope_line_charts slc ON slc.slopeChartConfigId = cc.id
+            SET
+                cc.patch = JSON_SET(cc.patch, '$.hideLegend', false),
+                cc.full = JSON_SET(cc.full, '$.hideLegend', false)
+            WHERE
+                slc.lineChartId IS NULL
+                AND cc.full ->> '$.hideLegend' = 'true'
+        `)
+
+        // for stand-alone slope charts that don't currently have any selected
+        // entities, just pick a random set of five entities.
+        // it's possible to end up with entities that don't have data for the
+        // selected years. after running the migration, I'll go through each
+        // slope chart and correct the selected entities manually if necessary.
+        await queryRunner.query(`
+            -- sql
+            WITH selected_entities AS (
+                WITH ranked_entities AS (
+                    SELECT
+                        slc.slopeChartId AS chartId,
+                        slc.slopeChartConfigId AS configId,
+                        cxe.entityId,
+                        e.name AS entityName,
+                        ROW_NUMBER() OVER (PARTITION BY chartId ORDER BY RAND()) AS randomIndex
+                    FROM slope_line_charts slc
+                    JOIN charts_x_entities cxe ON cxe.chartId = slc.slopeChartId
+                    JOIN entities e ON e.id = cxe.entityId
+                    WHERE
+                        slc.lineChartId IS NULL
+                        AND (
+                            slc.slopeChartSelectedEntityNames IS NULL
+                            OR JSON_LENGTH(slc.slopeChartSelectedEntityNames) = 0
+                        )
+                )
+                SELECT chartId, configId, JSON_ARRAYAGG(entityName) as selectedEntityNames
+                FROM ranked_entities
+                WHERE randomIndex <= 4
+                GROUP BY chartId, configId
+            )
+            UPDATE chart_configs cc
+            JOIN selected_entities se ON se.configId = cc.id
+            SET
+                cc.patch = JSON_SET(cc.patch, '$.selectedEntityNames', se.selectedEntityNames),
+                cc.full = JSON_SET(cc.full, '$.selectedEntityNames', se.selectedEntityNames)
+        `)
+
+        // LINE+SLOPE CHARTS
+
+        // add a slope tab to all line charts that have a corresponding slope
+        // chart (excluded are slope charts that have been matched with more
+        // than one line chart)
+        await queryRunner.query(`
+            WITH deduped_slope_line_charts AS (
+                SELECT slopeChartId, COUNT(*) count
+                FROM slope_line_charts
+                GROUP BY slopeChartId
+                HAVING count = 1
+            )
+            UPDATE chart_configs cc
+            JOIN slope_line_charts slc ON slc.lineChartConfigId = cc.id
+            JOIN deduped_slope_line_charts dslc ON dslc.slopeChartId = slc.slopeChartId
+            SET
+                cc.patch = JSON_SET(cc.patch, '$.chartTypes', JSON_ARRAY('LineChart', 'SlopeChart')),
+                cc.full = JSON_SET(cc.full, '$.chartTypes', JSON_ARRAY('LineChart', 'SlopeChart'))
+        `)
+
+        await queryRunner.query(`DROP TABLE slope_line_charts`)
+    }
+
+    // eslint-disable-next-line @typescript-eslint/no-empty-function
+    public async down(): Promise<void> {}
+}

packages/@ourworldindata/core-table/src/CoreTableColumns.ts

@@ -305,12 +305,22 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
     @imemo get displayName(): string {
         return (
             this.display?.name ??
-            this.def.presentation?.titlePublic ?? // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
+            // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
+            this.def.presentation?.titlePublic ??
             this.name ??
             ""
         )
     }
 
+    @imemo get nonEmptyDisplayName(): string {
+        return (
+            this.display?.name ||
+            // this is a bit of an unusual fallback - if display.name is not given, titlePublic is the next best thing before name
+            this.def.presentation?.titlePublic ||
+            this.nonEmptyName
+        )
+    }
+
     @imemo get titlePublicOrDisplayName(): IndicatorTitleWithFragments {
         return this.def.presentation?.titlePublic
             ? {
@@ -526,14 +536,16 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
     // assumes table is sorted by time
     @imemo get owidRows(): OwidVariableRow<JS_TYPE>[] {
         const entities = this.allEntityNames
-        const times = this.originalTimes
+        const times = this.allTimes
         const values = this.values
+        const originalTimes = this.originalTimes
         const originalValues = this.originalValues
-        return range(0, times.length).map((index) => {
+        return range(0, originalTimes.length).map((index) => {
             return omitUndefinedValues({
                 entityName: entities[index],
                 time: times[index],
                 value: values[index],
+                originalTime: originalTimes[index],
                 originalValue: originalValues[index],
             })
         })
@@ -552,6 +564,23 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
         return map
     }
 
+    // todo: remove? Should not be on CoreTable
+    @imemo get owidRowByEntityNameAndTime(): Map<
+        EntityName,
+        Map<Time, OwidVariableRow<JS_TYPE>>
+    > {
+        const valueByEntityNameAndTime = new Map<
+            EntityName,
+            Map<Time, OwidVariableRow<JS_TYPE>>
+        >()
+        this.owidRows.forEach((row) => {
+            if (!valueByEntityNameAndTime.has(row.entityName))
+                valueByEntityNameAndTime.set(row.entityName, new Map())
+            valueByEntityNameAndTime.get(row.entityName)!.set(row.time, row)
+        })
+        return valueByEntityNameAndTime
+    }
+
     // todo: remove? Should not be on CoreTable
     // NOTE: this uses the original times, so any tolerance is effectively unapplied.
     @imemo get valueByEntityNameAndOriginalTime(): Map<
@@ -567,7 +596,7 @@ export abstract class AbstractCoreColumn<JS_TYPE extends PrimitiveType> {
                 valueByEntityNameAndTime.set(row.entityName, new Map())
             valueByEntityNameAndTime
                 .get(row.entityName)!
-                .set(row.time, row.value)
+                .set(row.originalTime, row.value)
         })
         return valueByEntityNameAndTime
     }