🔨 (grapher) turn related questions into an object

owid · Nov 4, 2024 · 4597cf7 · 4597cf7
1 parent 66f141f
commit 4597cf7
Show file tree

Hide file tree

Showing 12 changed files with 199 additions and 124 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/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
     }
 

diff --git a/explorer/GrapherGrammar.ts b/explorer/GrapherGrammar.ts
@@ -284,13 +284,13 @@ export const GrapherGrammar: Grammar<GrapherCellDef> = {
         keyword: "relatedQuestionText",
         description:
             "The text used for the related question (at the very bottom of the chart)",
-        toGrapherObject: () => ({}), // handled in code (can be done properly once the relatedQuestion field is refactored)
+        toGrapherObject: (value) => ({ relatedQuestion: { text: value } }),
     },
     relatedQuestionUrl: {
         ...UrlCellDef,
         keyword: "relatedQuestionUrl",
         description: "The link of the related question text",
-        toGrapherObject: () => ({}), // handled in code (can be done properly once the relatedQuestion field is refactored)
+        toGrapherObject: (value) => ({ relatedQuestion: { url: value } }),
     },
     mapTargetTime: {
         ...IntegerCellDef,

diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx
@@ -94,7 +94,7 @@ export interface CaptionedChartManager
     detailRenderers: MarkdownTextWrap[]
 
     // related question
-    relatedQuestions?: RelatedQuestionsConfig[]
+    relatedQuestion?: RelatedQuestionsConfig
     showRelatedQuestion?: boolean
 }
 
@@ -271,7 +271,7 @@ export class CaptionedChart extends React.Component<CaptionedChartProps> {
     }
 
     private renderRelatedQuestion(): React.ReactElement {
-        const { relatedQuestions } = this.manager
+        const { relatedQuestion } = this.manager
         return (
             <div
                 className="relatedQuestion"
@@ -283,12 +283,12 @@ export class CaptionedChart extends React.Component<CaptionedChartProps> {
             >
                 Related:&nbsp;
                 <a
-                    href={relatedQuestions![0].url}
+                    href={relatedQuestion!.url}
                     target="_blank"
                     rel="noopener"
                     data-track-note="chart_click_related"
                 >
-                    {relatedQuestions![0].text}
+                    {relatedQuestion!.text}
                 </a>
                 <FontAwesomeIcon icon={faExternalLinkAlt} />
             </div>

diff --git a/packages/@ourworldindata/grapher/src/core/Grapher.tsx b/packages/@ourworldindata/grapher/src/core/Grapher.tsx
@@ -400,6 +400,7 @@ export class Grapher
     @observable.ref hideTotalValueLabel?: boolean = undefined
     @observable.ref missingDataStrategy?: MissingDataStrategy = undefined
     @observable.ref showSelectionOnlyInDataTable?: boolean = undefined
+    @observable relatedQuestion?: RelatedQuestionsConfig = undefined // todo: Persistables?
 
     @observable.ref xAxis = new AxisConfig(undefined, this)
     @observable.ref yAxis = new AxisConfig(undefined, this)
@@ -426,7 +427,6 @@ export class Grapher
      */
     @observable includedEntities?: number[] = undefined
     @observable comparisonLines?: ComparisonLineConfig[] = undefined // todo: Persistables?
-    @observable relatedQuestions?: RelatedQuestionsConfig[] = undefined // todo: Persistables?
 
     @observable.ref annotation?: Annotation = undefined
 
@@ -1249,10 +1249,8 @@ export class Grapher
 
     // todo: did this name get botched in a merge?
     @computed get hasFatalErrors(): boolean {
-        const { relatedQuestions = [] } = this
-        return relatedQuestions.some(
-            (question) => !!getErrorMessageRelatedQuestionUrl(question)
-        )
+        if (!this.relatedQuestion) return false
+        return !!getErrorMessageRelatedQuestionUrl(this.relatedQuestion)
     }
 
     disposers: (() => void)[] = []
@@ -3055,14 +3053,8 @@ export class Grapher
     @observable isShareMenuActive = false
 
     @computed get hasRelatedQuestion(): boolean {
-        if (
-            this.hideRelatedQuestion ||
-            !this.relatedQuestions ||
-            !this.relatedQuestions.length
-        )
-            return false
-        const question = this.relatedQuestions[0]
-        return !!question && !!question.text && !!question.url
+        if (this.hideRelatedQuestion) return false
+        return !!(this.relatedQuestion?.url && this.relatedQuestion?.text)
     }
 
     @computed get isRelatedQuestionTargetDifferentFromCurrentPage(): boolean {
@@ -3073,19 +3065,18 @@ export class Grapher
         // "ourworldindata.org" and yet should still yield a match.
         // - Note that this won't work on production previews (where the
         //   path is /admin/posts/preview/ID)
-        const { hasRelatedQuestion, relatedQuestions = [] } = this
-        const relatedQuestion = relatedQuestions[0]
-        return (
+        const { hasRelatedQuestion, relatedQuestion } = this
+        return !!(
             hasRelatedQuestion &&
-            !!relatedQuestion &&
+            relatedQuestion?.url &&
             getWindowUrl().pathname !==
                 Url.fromURL(relatedQuestion.url).pathname
         )
     }
 
     @computed get showRelatedQuestion(): boolean {
         return (
-            !!this.relatedQuestions &&
+            !!this.relatedQuestion &&
             !!this.hasRelatedQuestion &&
             !!this.isRelatedQuestionTargetDifferentFromCurrentPage
         )
@@ -3477,10 +3468,9 @@ const defaultObject = objectWithPersistablesToObject(
 export const getErrorMessageRelatedQuestionUrl = (
     question: RelatedQuestionsConfig
 ): string | undefined => {
-    return question.text
-        ? (!question.url && "Missing URL") ||
-              (!question.url.match(/^https?:\/\//) &&
-                  "URL should start with http(s)://") ||
-              undefined
-        : undefined
+    if (!question.text) return undefined
+    if (!question.url) return "Missing URL"
+    if (!question.url.match(/^https?:\/\//))
+        return "URL should start with http(s)://"
+    return undefined
 }
diff --git a/packages/@ourworldindata/grapher/src/schema/README.md b/packages/@ourworldindata/grapher/src/schema/README.md
@@ -18,7 +18,6 @@ Checklist for breaking changes:
 -   Rename the authorative url inside the schema file to point to the new version number json
 -   Write the migrations from the last to the current version of the schema, including the migration of pointing to the URL of the new schema version
 -   Regenerate the default object from the schema and save it to `defaultGrapherConfig.ts` (see below)
--   Write a migration to update the `chart_configs.full` column in the database for all stand-alone charts
 -   Write a migration to update configs in code (see `migrations/migrations.ts`)
 -   Update the hardcoded default schema version in ETL