From a7d896eab80e8db509fa71b7d69c5b2da8f6fcc9 Mon Sep 17 00:00:00 2001
From: Marcel Gerber <mgerber59@gmail.com>
Date: Thu, 9 Jan 2025 11:37:41 +0100
Subject: [PATCH] feat: use nice modal for entering narrative chart name,
 gracefully handle duplicate names

---
 adminSiteClient/ChartEditor.ts  |  32 ++++----
 adminSiteClient/SaveButtons.tsx | 129 ++++++++++++++++++++++++++++----
 adminSiteServer/apiRouter.ts    |  12 +++
 3 files changed, 141 insertions(+), 32 deletions(-)

diff --git a/adminSiteClient/ChartEditor.ts b/adminSiteClient/ChartEditor.ts
index 25b10df424..c059e75ebc 100644
--- a/adminSiteClient/ChartEditor.ts
+++ b/adminSiteClient/ChartEditor.ts
@@ -26,6 +26,8 @@ import {
     References,
 } from "./AbstractChartEditor.js"
 import { Admin } from "./Admin.js"
+import { Form, Input, Modal } from "antd"
+import React, { useState } from "react"
 
 export interface Log {
     userId: number
@@ -200,22 +202,17 @@ export class ChartEditor extends AbstractChartEditor<ChartEditorManager> {
             )
     }
 
-    async saveAsChartView(): Promise<void> {
-        const { patchConfig, grapher } = this
-
-        const chartJson = omit(patchConfig, CHART_VIEW_PROPS_TO_OMIT)
-
+    openNarrativeChartNameModal(): void {
+        const { grapher } = this
         const suggestedName = grapher.title ? slugify(grapher.title) : undefined
+    }
 
-        const name = prompt(
-            "Please enter a programmatic name for the narrative chart. Note that this name cannot be changed later.",
-            suggestedName
-        )
-
-        if (name === null) return
+    async saveAsChartView(
+        name: string
+    ): Promise<{ success: boolean; errorMsg?: string }> {
+        const { patchConfig, grapher } = this
 
-        // Need to open intermediary tab before AJAX to avoid popup blockers
-        const w = window.open("/", "_blank") as Window
+        const chartJson = omit(patchConfig, CHART_VIEW_PROPS_TO_OMIT)
 
         const body = {
             name,
@@ -228,11 +225,14 @@ export class ChartEditor extends AbstractChartEditor<ChartEditorManager> {
             body,
             "POST"
         )
-
-        if (json.success)
-            w.location.assign(
+        if (json.success) {
+            window.open(
                 this.manager.admin.url(`chartViews/${json.chartViewId}/edit`)
             )
+            return { success: true }
+        } else {
+            return { success: false, errorMsg: json.errorMsg }
+        }
     }
 
     publishGrapher(): void {
diff --git a/adminSiteClient/SaveButtons.tsx b/adminSiteClient/SaveButtons.tsx
index 3166add379..bdae5d157f 100644
--- a/adminSiteClient/SaveButtons.tsx
+++ b/adminSiteClient/SaveButtons.tsx
@@ -1,8 +1,8 @@
-import { Component } from "react"
+import { Component, useEffect, useMemo, useRef, useState } from "react"
 import { ChartEditor, isChartEditorInstance } from "./ChartEditor.js"
-import { action, computed } from "mobx"
+import { action, computed, observable } from "mobx"
 import { observer } from "mobx-react"
-import { excludeUndefined, omit } from "@ourworldindata/utils"
+import { excludeUndefined, omit, slugify } from "@ourworldindata/utils"
 import {
     IndicatorChartEditor,
     isIndicatorChartEditorInstance,
@@ -17,6 +17,7 @@ import {
     chartViewsFeatureEnabled,
     isChartViewEditorInstance,
 } from "./ChartViewEditor.js"
+import { Form, Input, InputRef, Modal, Spin } from "antd"
 
 @observer
 export class SaveButtons<Editor extends AbstractChartEditor> extends Component<{
@@ -61,10 +62,6 @@ class SaveButtonsForChart extends Component<{
         void this.props.editor.saveAsNewGrapher()
     }
 
-    @action.bound onSaveAsChartView() {
-        void this.props.editor.saveAsChartView()
-    }
-
     @action.bound onPublishToggle() {
         if (this.props.editor.grapher.isPublished)
             this.props.editor.unpublishGrapher()
@@ -79,6 +76,29 @@ class SaveButtonsForChart extends Component<{
         ])
     }
 
+    @computed get initialNarrativeChartName(): string {
+        return slugify(this.props.editor.grapher.title ?? "")
+    }
+
+    @observable narrativeChartNameModalOpen:
+        | "open"
+        | "open-loading"
+        | "closed" = "closed"
+    @observable narrativeChartNameModalError: string | undefined = undefined
+
+    @action.bound async onSubmitNarrativeChartButton(name: string) {
+        const { editor } = this.props
+
+        this.narrativeChartNameModalOpen = "open-loading"
+        const res = await editor.saveAsChartView(name)
+        if (res.success) {
+            this.narrativeChartNameModalOpen = "closed"
+        } else {
+            this.narrativeChartNameModalOpen = "open"
+            this.narrativeChartNameModalError = res.errorMsg
+        }
+    }
+
     render() {
         const { editingErrors } = this
         const { editor } = this.props
@@ -117,15 +137,30 @@ class SaveButtonsForChart extends Component<{
                     </button>
                 </div>
                 {chartViewsFeatureEnabled && (
-                    <div className="mt-2">
-                        <button
-                            className="btn btn-primary"
-                            onClick={this.onSaveAsChartView}
-                            disabled={isSavingDisabled}
-                        >
-                            Save as narrative chart
-                        </button>
-                    </div>
+                    <>
+                        <div className="mt-2">
+                            <button
+                                className="btn btn-primary"
+                                onClick={() => {
+                                    this.narrativeChartNameModalOpen = "open"
+                                    this.narrativeChartNameModalError =
+                                        undefined
+                                }}
+                                disabled={isSavingDisabled}
+                            >
+                                Save as narrative chart
+                            </button>
+                        </div>
+                        <NarrativeChartNameModal
+                            open={this.narrativeChartNameModalOpen}
+                            initialName={this.initialNarrativeChartName}
+                            errorMsg={this.narrativeChartNameModalError}
+                            onSubmit={this.onSubmitNarrativeChartButton}
+                            onCancel={() =>
+                                (this.narrativeChartNameModalOpen = "closed")
+                            }
+                        />
+                    </>
                 )}
                 {editingErrors.map((error, i) => (
                     <div key={i} className="alert alert-danger mt-2">
@@ -238,3 +273,65 @@ class SaveButtonsForChartView extends Component<{
         )
     }
 }
+
+const NarrativeChartNameModal = (props: {
+    initialName: string
+    open: "open" | "open-loading" | "closed"
+    errorMsg?: string
+    onSubmit: (name: string) => void
+    onCancel?: () => void
+}) => {
+    const [name, setName] = useState<string>(props.initialName)
+    const inputField = useRef<InputRef>(null)
+    const isLoading = useMemo(() => props.open === "open-loading", [props.open])
+    const isOpen = useMemo(() => props.open !== "closed", [props.open])
+
+    useEffect(() => setName(props.initialName), [props.initialName])
+
+    useEffect(() => {
+        if (isOpen) {
+            inputField.current?.focus({ cursor: "all" })
+        }
+    }, [isOpen])
+
+    return (
+        <Modal
+            title="Save as narrative chart"
+            open={isOpen}
+            onOk={() => props.onSubmit(name)}
+            onCancel={props.onCancel}
+            onClose={props.onCancel}
+            okButtonProps={{ disabled: !name || isLoading }}
+            cancelButtonProps={{ disabled: isLoading }}
+        >
+            <div>
+                <p>
+                    This will create a new narrative chart that is linked to
+                    this chart. Any currently pending changes will be applied to
+                    the narrative chart.
+                </p>
+                <p>
+                    Please enter a programmatic name for the narrative chart.{" "}
+                    <i>Note that this name cannot be changed later.</i>
+                </p>
+                <Form.Item label="Name">
+                    <Input
+                        ref={inputField}
+                        onChange={(e) => setName(e.target.value)}
+                        value={name}
+                        disabled={isLoading}
+                    />
+                </Form.Item>
+                {isLoading && <Spin />}
+                {props.errorMsg && (
+                    <div
+                        className="alert alert-danger"
+                        style={{ whiteSpace: "pre-wrap" }}
+                    >
+                        {props.errorMsg}
+                    </div>
+                )}
+            </div>
+        </Modal>
+    )
+}
diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts
index 96278efa0f..3b001634de 100644
--- a/adminSiteServer/apiRouter.ts
+++ b/adminSiteServer/apiRouter.ts
@@ -3762,6 +3762,18 @@ postRouteWithRWTransaction(apiRouter, "/chartViews", async (req, res, trx) => {
         throw new JsonError("Invalid request", 400)
     }
 
+    const chartViewWithName = await trx
+        .table(ChartViewsTableName)
+        .where({ name })
+        .first()
+
+    if (chartViewWithName) {
+        return {
+            success: false,
+            errorMsg: `Narrative chart with name "${name}" already exists`,
+        }
+    }
+
     const { patchConfig, fullConfig, queryParams } =
         await createPatchConfigAndQueryParamsForChartView(
             trx,