From 2be42750ab1e3871931dc272f23501ae33bf834d Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Thu, 24 Oct 2024 11:41:18 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20(line=20chart)=20set=20y-axis=20min?= =?UTF-8?q?=20default=20to=200?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/EditorCustomizeTab.tsx | 8 +- .../1729763649580-SetYAxisMinDefaultToZero.ts | 81 +++++++++++++++++++ .../grapher/src/axis/AxisConfig.ts | 19 +++-- .../grapher/src/lineCharts/LineChart.tsx | 2 + .../src/schema/defaultGrapherConfig.ts | 2 - .../src/schema/grapher-schema.005.yaml | 5 +- 6 files changed, 103 insertions(+), 14 deletions(-) create mode 100644 db/migration/1729763649580-SetYAxisMinDefaultToZero.ts diff --git a/adminSiteClient/EditorCustomizeTab.tsx b/adminSiteClient/EditorCustomizeTab.tsx index cdbc1cb260..ae3cdf5bed 100644 --- a/adminSiteClient/EditorCustomizeTab.tsx +++ b/adminSiteClient/EditorCustomizeTab.tsx @@ -577,9 +577,9 @@ export class EditorCustomizeTab< } resetButton={{ onClick: () => - (yAxisConfig.min = undefined), + (yAxisConfig.min = Infinity), disabled: - yAxisConfig.min === undefined, + yAxisConfig.min === Infinity, }} allowDecimal allowNegative @@ -592,9 +592,9 @@ export class EditorCustomizeTab< } resetButton={{ onClick: () => - (yAxisConfig.max = undefined), + (yAxisConfig.max = -Infinity), disabled: - yAxisConfig.max === undefined, + yAxisConfig.max === -Infinity, }} allowDecimal allowNegative diff --git a/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts b/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts new file mode 100644 index 0000000000..d121c4bc77 --- /dev/null +++ b/db/migration/1729763649580-SetYAxisMinDefaultToZero.ts @@ -0,0 +1,81 @@ +import { defaultGrapherConfig } from "@ourworldindata/grapher" +import { mergeGrapherConfigs } from "@ourworldindata/utils" +import { MigrationInterface, QueryRunner } from "typeorm" + +export class SetYAxisMinDefaultToZero1729763649580 + implements MigrationInterface +{ + public async up(queryRunner: QueryRunner): Promise { + // when inheritance is disabled, set yAxis.min explicitly to "auto" + // for charts that used to rely on "auto" being the default + await queryRunner.query(` + -- sql + UPDATE chart_configs cc + JOIN charts c ON cc.id = c.configId + SET + cc.patch = JSON_SET(cc.patch, '$.yAxis.min', 'auto'), + cc.full = JSON_SET(cc.full, '$.yAxis.min', 'auto') + WHERE + cc.full ->> '$.type' = 'LineChart' + AND cc.patch ->> '$.yAxis.min' IS NULL + AND c.isInheritanceEnabled IS FALSE + `) + + // ETL-authored indicator configs: + // set yAxis.min to "auto" for etl-authored configs for configs + // that used to rely on "auto" being the default + await queryRunner.query(` + -- sql + UPDATE chart_configs cc + JOIN variables v ON cc.id = v.grapherConfigIdETL + SET + cc.patch = JSON_SET(cc.patch, '$.yAxis.min', 'auto'), + cc.full = JSON_SET(cc.full, '$.yAxis.min', 'auto') + WHERE + COALESCE(cc.patch ->> '$.type', 'LineChart') = 'LineChart' + AND cc.patch ->> '$.yAxis.min' IS NULL + `) + + // Update the full configs of all charts that inherit from an indicator + const charts = await queryRunner.query(` + -- sql + SELECT + cc.id AS configId, + cc.patch AS patchConfig, + cc_etl.patch AS etlConfig, + cc_admin.patch AS adminConfig + FROM charts c + JOIN chart_configs cc ON cc.id = c.configId + JOIN charts_x_parents p ON p.chartId = c.id + JOIN variables v ON v.id = p.variableId + LEFT JOIN chart_configs cc_etl ON cc_etl.id = v.grapherConfigIdETL + LEFT JOIN chart_configs cc_admin ON cc_admin.id = v.grapherConfigIdAdmin + WHERE c.isInheritanceEnabled IS TRUE + `) + + for (const chart of charts) { + const fullConfig = mergeGrapherConfigs( + defaultGrapherConfig, + chart.etlConfig ?? {}, + chart.adminConfig ?? {}, + chart.patchConfig + ) + + await queryRunner.query( + ` + -- sql + UPDATE chart_configs cc + SET cc.full = ? + WHERE cc.id = ? + `, + [JSON.stringify(fullConfig), chart.configId] + ) + } + } + + public async down(): Promise { + throw new Error( + "Migration SetYAxisMinDefaultToZero1729763649580 can't be reverted" + ) + } +} diff --git a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts index 2226665d24..676da96add 100644 --- a/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts +++ b/packages/@ourworldindata/grapher/src/axis/AxisConfig.ts @@ -45,10 +45,17 @@ class AxisConfigDefaults implements AxisConfigInterface { @observable.ref domainValues?: number[] = undefined } -function parseAutoOrNumberFromJSON( +function parseMinFromJSON( value: AxisMinMaxValueStr.auto | number | undefined ): number | undefined { - if (value === AxisMinMaxValueStr.auto) return undefined + if (value === AxisMinMaxValueStr.auto) return Infinity + return value +} + +function parseMaxFromJSON( + value: AxisMinMaxValueStr.auto | number | undefined +): number | undefined { + if (value === AxisMinMaxValueStr.auto) return -Infinity return value } @@ -67,8 +74,8 @@ export class AxisConfig // todo: test/refactor updateFromObject(props?: AxisConfigInterface): void { if (props) extend(this, props) - if (props?.min) this.min = parseAutoOrNumberFromJSON(props?.min) - if (props?.max) this.max = parseAutoOrNumberFromJSON(props?.max) + if (props?.min) this.min = parseMinFromJSON(props?.min) + if (props?.max) this.max = parseMaxFromJSON(props?.max) } toObject(): AxisConfigInterface { @@ -96,8 +103,8 @@ export class AxisConfig deleteRuntimeAndUnchangedProps(obj, new AxisConfigDefaults()) - if (obj.min === undefined) obj.min = AxisMinMaxValueStr.auto - if (obj.max === undefined) obj.max = AxisMinMaxValueStr.auto + if (obj.min === Infinity) obj.min = AxisMinMaxValueStr.auto + if (obj.max === -Infinity) obj.max = AxisMinMaxValueStr.auto return obj } diff --git a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx index 6b6e2b64ce..37d5c4f16e 100644 --- a/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx +++ b/packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx @@ -1342,6 +1342,8 @@ export class LineChart // horizontal axis to be at the bottom of the chart. // see https://github.com/owid/owid-grapher/pull/975#issuecomment-890798547 singleValueAxisPointAlign: AxisAlign.start, + // default to 0 if not set + min: 0, ...this.manager.yAxisConfig, }, this diff --git a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts index 6497d08f13..dd0f68334e 100644 --- a/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts +++ b/packages/@ourworldindata/grapher/src/schema/defaultGrapherConfig.ts @@ -26,7 +26,6 @@ export const defaultGrapherConfig = { maxTime: "latest", yAxis: { removePointsOutsideDomain: false, - min: "auto", scaleType: "linear", max: "auto", canChangeScaleType: false, @@ -66,7 +65,6 @@ export const defaultGrapherConfig = { }, xAxis: { removePointsOutsideDomain: false, - min: "auto", scaleType: "linear", max: "auto", canChangeScaleType: false, diff --git a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml b/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml index 79ca82b412..4465a68f6c 100644 --- a/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml +++ b/packages/@ourworldindata/grapher/src/schema/grapher-schema.005.yaml @@ -514,8 +514,9 @@ $defs: type: string description: Axis label min: - description: Minimum domain value of the axis. Inferred from data if set to "auto". - default: auto + description: | + Minimum domain value of the axis. Inferred from data if set to "auto". + Usually defaults to "auto", but defaults to 0 for line charts on the y-axis. oneOf: - type: number - type: string