🔨 (grapher) turn related questions into an object

owid · Nov 6, 2024 · ed6ea01 · ed6ea01
1 parent c9de5c6
commit ed6ea01
Show file tree

Hide file tree

Showing 14 changed files with 217 additions and 147 deletions.
diff --git a/adminSiteClient/EditorTextTab.tsx b/adminSiteClient/EditorTextTab.tsx
@@ -1,6 +1,6 @@
-import { faMinus, faPlus } from "@fortawesome/free-solid-svg-icons"
+import { faMinus } from "@fortawesome/free-solid-svg-icons"
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js"
-import { LogoOption, RelatedQuestionsConfig } from "@ourworldindata/types"
+import { LogoOption } from "@ourworldindata/types"
 import { getErrorMessageRelatedQuestionUrl } from "@ourworldindata/grapher"
 import { slugify } from "@ourworldindata/utils"
 import { action, computed } from "mobx"
@@ -37,21 +37,6 @@ export class EditorTextTab<
         }
     }
 
-    @action.bound onAddRelatedQuestion() {
-        const { grapher } = this.props.editor
-        if (!grapher.relatedQuestions) grapher.relatedQuestions = []
-        grapher.relatedQuestions.push({
-            text: "",
-            url: "",
-        })
-    }
-
-    @action.bound onRemoveRelatedQuestion(idx: number) {
-        const { grapher } = this.props.editor
-        if (!grapher.relatedQuestions) grapher.relatedQuestions = []
-        grapher.relatedQuestions.splice(idx, 1)
-    }
-
     @action.bound onToggleTitleAnnotationEntity(value: boolean) {
         const { grapher } = this.props.editor
         grapher.hideAnnotationFieldsInTitle ??= {}
@@ -86,7 +71,6 @@ export class EditorTextTab<
     render() {
         const { editor } = this.props
         const { grapher, features } = editor
-        const { relatedQuestions = [] } = grapher
 
         return (
             <div className="EditorTextTab">
@@ -250,50 +234,49 @@ export class EditorTextTab<
                 </Section>
 
                 <Section name="Related">
-                    {relatedQuestions.map(
-                        (question: RelatedQuestionsConfig, idx: number) => (
-                            <div key={idx}>
-                                <TextField
-                                    label="Related question"
-                                    value={question.text}
-                                    onValue={action((value: string) => {
-                                        question.text = value
-                                    })}
-                                    placeholder="e.g. How did countries respond to the pandemic?"
-                                    helpText="Short question promoting exploration of related content"
-                                    softCharacterLimit={50}
-                                />
-                                {question.text && (
-                                    <TextField
-                                        label="URL"
-                                        value={question.url}
-                                        onValue={action((value: string) => {
-                                            question.url = value
-                                        })}
-                                        placeholder="e.g. https://ourworldindata.org/coronavirus"
-                                        helpText="Page or section of a page where the answer to the previous question can be found."
-                                        errorMessage={getErrorMessageRelatedQuestionUrl(
-                                            question
-                                        )}
-                                    />
-                                )}
-                                <Button
-                                    onClick={() =>
-                                        this.onRemoveRelatedQuestion(idx)
+                    <div>
+                        <TextField
+                            label="Related question"
+                            value={grapher.relatedQuestion?.text}
+                            onValue={action((value: string) => {
+                                if (grapher.relatedQuestion) {
+                                    grapher.relatedQuestion.text = value
+                                } else {
+                                    grapher.relatedQuestion = {
+                                        text: value,
+                                        url: "",
                                     }
-                                >
-                                    <FontAwesomeIcon icon={faMinus} /> Remove
-                                    related question
-                                </Button>
-                            </div>
-                        )
-                    )}
-                    {!relatedQuestions.length && (
-                        <Button onClick={this.onAddRelatedQuestion}>
-                            <FontAwesomeIcon icon={faPlus} /> Add related
-                            question
-                        </Button>
-                    )}
+                                }
+                            })}
+                            placeholder="e.g. How did countries respond to the pandemic?"
+                            helpText="Short question promoting exploration of related content"
+                            softCharacterLimit={50}
+                        />
+                        {grapher.relatedQuestion?.text && (
+                            <TextField
+                                label="URL"
+                                value={grapher.relatedQuestion.url}
+                                onValue={action((value: string) => {
+                                    grapher.relatedQuestion!.url = value
+                                })}
+                                placeholder="e.g. https://ourworldindata.org/coronavirus"
+                                helpText="Page or section of a page where the answer to the previous question can be found."
+                                errorMessage={getErrorMessageRelatedQuestionUrl(
+                                    grapher.relatedQuestion
+                                )}
+                            />
+                        )}
+                        {grapher.relatedQuestion?.text && (
+                            <Button
+                                onClick={() =>
+                                    (grapher.relatedQuestion = undefined)
+                                }
+                            >
+                                <FontAwesomeIcon icon={faMinus} /> Remove
+                                related question
+                            </Button>
+                        )}
+                    </div>
                 </Section>
                 <Section name="Misc">
                     <BindString

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",
@@ -311,16 +313,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",
     }
@@ -331,14 +331,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",
@@ -352,8 +350,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",
@@ -612,8 +609,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
         )
 
         expect(fullConfig).toEqual({
-            $schema:
-                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+            $schema: currentSchema,
             id: chartId,
             isPublished: false,
             version: 1,
@@ -633,8 +629,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,
@@ -670,8 +665,7 @@ describe("OwidAdminApp: indicator-level chart configs", () => {
             `/charts/${chartId}.config.json`
         )
         expect(fullConfigAfterDelete).toEqual({
-            $schema:
-                "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+            $schema: currentSchema,
             id: chartId,
             version: 1,
             isPublished: false,
@@ -687,8 +681,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/multiDim.ts b/adminSiteServer/multiDim.ts
@@ -1,7 +1,10 @@
 import { uniq } from "lodash"
 import { uuidv7 } from "uuidv7"
 
-import { migrateGrapherConfigToLatestVersion } from "@ourworldindata/grapher"
+import {
+    defaultGrapherConfig,
+    migrateGrapherConfigToLatestVersion,
+} from "@ourworldindata/grapher"
 import {
     Base64String,
     ChartConfigsTableName,
@@ -285,8 +288,7 @@ export async function createMultiDimConfig(
             const variableId = view.indicators.y[0]
             // Main config for each view.
             const mainGrapherConfig = {
-                $schema:
-                    "https://files.ourworldindata.org/schemas/grapher-schema.005.json",
+                $schema: defaultGrapherConfig.$schema,
                 dimensions: MultiDimDataPageConfig.viewToDimensionsConfig(view),
                 selectedEntityNames: config.defaultSelection ?? [],
             }

diff --git a/db/migration/1730722779919-TurnRelatedQuestionIntoObject.ts b/db/migration/1730722779919-TurnRelatedQuestionIntoObject.ts
@@ -0,0 +1,91 @@
+import { MigrationInterface, QueryRunner } from "typeorm"
+
+export class TurnRelatedQuestionIntoObject1730722779919
+    implements MigrationInterface
+{
+    private async updateSchema(
+        queryRunner: QueryRunner,
+        newSchema: string
+    ): Promise<void> {
+        await queryRunner.query(
+            `
+            -- sql
+            UPDATE chart_configs cc
+            SET cc.patch = JSON_SET(cc.patch, '$.$schema', ?),
+                cc.full = JSON_SET(cc.full, '$.$schema', ?)
+        `,
+            [newSchema, newSchema]
+        )
+    }
+
+    private async turnRelatedQuestionIntoObject(
+        queryRunner: QueryRunner,
+        config: "patch" | "full"
+    ): Promise<void> {
+        await queryRunner.query(
+            `
+            -- sql
+            UPDATE chart_configs
+            SET
+                ?? = JSON_REMOVE(
+                    JSON_SET(
+                        ??,
+                        '$.relatedQuestion',
+                        ??->'$.relatedQuestions[0]'
+                    ),
+                    '$.relatedQuestions'
+                )
+            WHERE ?? ->> '$.relatedQuestions' is not null
+            `,
+            [config, config, config, config]
+        )
+    }
+
+    private async turnRelatedQuestionIntoArray(
+        queryRunner: QueryRunner,
+        config: "patch" | "full"
+    ): Promise<void> {
+        await queryRunner.query(
+            `
+            -- sql
+            UPDATE chart_configs
+            SET
+                ?? = JSON_REMOVE(
+                    JSON_SET(
+                        ??,
+                        '$.relatedQuestions',
+                        JSON_ARRAY(
+                            JSON_OBJECT(
+                                'url', ?? ->> '$.relatedQuestion.url',
+                                'text', ?? ->> '$.relatedQuestion.text'
+                            )
+                        )
+                    ),
+                    '$.relatedQuestion'
+                )
+            WHERE ?? ->> '$.relatedQuestion' IS NOT NULL
+            `,
+            [config, config, config, config, config]
+        )
+    }
+
+    public async up(queryRunner: QueryRunner): Promise<void> {
+        await this.turnRelatedQuestionIntoObject(queryRunner, "patch")
+        await this.turnRelatedQuestionIntoObject(queryRunner, "full")
+
+        await this.updateSchema(
+            queryRunner,
+            "https://files.ourworldindata.org/schemas/grapher-schema.006.json"
+        )
+    }
+
+    public async down(queryRunner: QueryRunner): Promise<void> {
+        await this.turnRelatedQuestionIntoArray(queryRunner, "patch")
+        await this.turnRelatedQuestionIntoArray(queryRunner, "full")
+
+        await this.updateSchema(
+            queryRunner,
+            "https://files.ourworldindata.org/schemas/grapher-schema.005.json"
+        )
+    }
+}
diff --git a/devTools/schemaProcessor/columns.json b/devTools/schemaProcessor/columns.json
@@ -617,12 +617,12 @@
     },
     {
         "type": "string",
-        "pointer": "/relatedQuestions/0/url",
+        "pointer": "/relatedQuestion/url",
         "editor": "textfield"
     },
     {
         "type": "string",
-        "pointer": "/relatedQuestions/0/text",
+        "pointer": "/relatedQuestion/text",
         "editor": "textfield"
     },
     {

diff --git a/explorer/ExplorerProgram.ts b/explorer/ExplorerProgram.ts
@@ -400,15 +400,6 @@ export class ExplorerProgram extends GridProgram {
         // assume config is valid against the latest schema
         mergedConfig.$schema = defaultGrapherConfig.$schema
 
-        // TODO: can be removed once relatedQuestions is refactored
-        const { relatedQuestionUrl, relatedQuestionText } =
-            this.explorerGrapherConfig
-        if (relatedQuestionUrl && relatedQuestionText) {
-            mergedConfig.relatedQuestions = [
-                { url: relatedQuestionUrl, text: relatedQuestionText },
-            ]
-        }
-
         return mergedConfig
     }