From 9280f7d16b9596dd6b5589f986f444e84ed50fed Mon Sep 17 00:00:00 2001
From: Ike Saunders <ikesau@protonmail.com>
Date: Thu, 12 Oct 2023 15:58:52 -0400
Subject: [PATCH 1/4] =?UTF-8?q?=E2=9C=A8=20remove=20autosave=20from=20gdoc?=
 =?UTF-8?q?s=20preview=20page?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 adminSiteClient/Admin.tsx            |  8 -----
 adminSiteClient/GdocsPreviewPage.tsx | 51 ----------------------------
 site/hooks.ts                        | 22 ------------
 3 files changed, 81 deletions(-)

diff --git a/adminSiteClient/Admin.tsx b/adminSiteClient/Admin.tsx
index 273fd935ea3..6185b6107a0 100644
--- a/adminSiteClient/Admin.tsx
+++ b/adminSiteClient/Admin.tsx
@@ -46,12 +46,6 @@ export class Admin {
     }
 
     @observable currentRequests: Promise<Response>[] = []
-    // A way to cancel fetch requests
-    // e.g. currentRequestAbortControllers.get(request).abort()
-    @observable currentRequestAbortControllers: Map<
-        Promise<Response>,
-        AbortController
-    > = new Map()
 
     @computed get showLoadingIndicator(): boolean {
         return this.loadingIndicatorSetting === "default"
@@ -132,7 +126,6 @@ export class Admin {
                 abortController
             )
             this.addRequest(request)
-            this.currentRequestAbortControllers.set(request, abortController)
 
             response = await request
             text = await response.text()
@@ -154,7 +147,6 @@ export class Admin {
         } finally {
             if (request) {
                 this.removeRequest(request)
-                this.currentRequestAbortControllers.delete(request)
             }
         }
 
diff --git a/adminSiteClient/GdocsPreviewPage.tsx b/adminSiteClient/GdocsPreviewPage.tsx
index 109ba390625..97548780671 100644
--- a/adminSiteClient/GdocsPreviewPage.tsx
+++ b/adminSiteClient/GdocsPreviewPage.tsx
@@ -19,7 +19,6 @@ import {
     OwidGdocErrorMessage,
     OwidGdocErrorMessageType,
     slugify,
-    pick,
 } from "@ourworldindata/utils"
 import { Button, Col, Drawer, Row, Space, Tag, Typography } from "antd"
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js"
@@ -40,7 +39,6 @@ import { GdocsEditLink } from "./GdocsEditLink.js"
 import { openSuccessNotification } from "./gdocsNotifications.js"
 import { GdocsDiffButton } from "./GdocsDiffButton.js"
 import { GdocsDiff } from "./GdocsDiff.js"
-import { useInterval } from "../site/hooks.js"
 
 export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
     const { id } = match.params
@@ -70,15 +68,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
     const [errors, setErrors] = React.useState<OwidGdocErrorMessage[]>()
     const { admin } = useContext(AdminAppContext)
     const store = useGdocsStore()
-    const [iframeScrollY, setIframeScrollY] = useState<number | undefined>()
-    // Cancel all other requests in progress (most likely just the automatic fetch)
-    const cancelAllRequests = useMemo(
-        () => () =>
-            admin.currentRequestAbortControllers.forEach((abortController) => {
-                abortController.abort()
-            }),
-        [admin]
-    )
 
     const iframeRef = useRef<HTMLIFrameElement>(null)
 
@@ -124,42 +113,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
         }
     }, [originalGdoc, fetchGdoc, handleError, admin])
 
-    // synchronise content every 5 seconds
-    useInterval(async () => {
-        if (currentGdoc) {
-            const latestGdoc = await fetchGdoc(GdocsContentSource.Gdocs)
-
-            // Save the scroll position of the iframe to restore it after the
-            // refresh. The condition is here to prevent firefox from
-            // calculating the scroll position on page load, which somehow results in a
-            // wrong value (likely at the bottom).
-            if (latestGdoc.revisionId !== currentGdoc.revisionId) {
-                setIframeScrollY(iframeRef.current?.contentWindow?.scrollY)
-            }
-
-            setCurrentGdoc((current) => ({
-                ...latestGdoc,
-
-                // keep values that might've been updated in the admin (e.g. slug) as they are locally
-                ...pick(current, [
-                    "slug",
-                    "published",
-                    "publishedAt",
-                    "publicationContext",
-                    "breadcrumbs",
-                ]),
-            }))
-        }
-    }, 5000)
-
-    const onIframeLoad = () => {
-        if (!iframeScrollY) return
-        // scroll the iframe to the position it was at before the refresh
-        iframeRef.current?.contentWindow?.scrollTo({
-            top: iframeScrollY,
-        })
-    }
-
     const isLightningUpdate = useLightningUpdate(
         originalGdoc,
         currentGdoc,
@@ -178,7 +131,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
 
     const saveDraft = async () => {
         if (!currentGdoc) return
-        cancelAllRequests()
 
         if (currentGdoc.published)
             throw new Error("Cannot save a published doc as a draft")
@@ -190,7 +142,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
 
     const doPublish = async () => {
         if (!currentGdoc) return
-        cancelAllRequests()
         // set to today if not specified
         const publishedAt = currentGdoc.publishedAt ?? new Date()
         const slug = currentGdoc.slug || slugify(`${currentGdoc.content.title}`)
@@ -205,7 +156,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
 
     const doUnpublish = async () => {
         if (!currentGdoc) return
-        cancelAllRequests()
         const unpublishedGdoc = await store.unpublish(currentGdoc)
         setGdoc({ original: unpublishedGdoc, current: unpublishedGdoc })
         openSuccessNotification("unpublished")
@@ -389,7 +339,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
                 */}
                 <iframe
                     ref={iframeRef}
-                    onLoad={onIframeLoad}
                     src={`/gdocs/${currentGdoc.id}/preview#owid-document-root`}
                     style={{ width: "100%", border: "none" }}
                     // use `updatedAt` as a proxy for when database-level settings such as breadcrumbs have changed
diff --git a/site/hooks.ts b/site/hooks.ts
index 53cd5ce7560..275e8af2bcb 100644
--- a/site/hooks.ts
+++ b/site/hooks.ts
@@ -67,28 +67,6 @@ export const useEmbedChart = (
     }, [activeChartIdx, refChartContainer])
 }
 
-// Adapted from https://overreacted.io/making-setinterval-declarative-with-react-hooks/
-export const useInterval = (callback: VoidFunction, delay: number | null) => {
-    const savedCallback = useRef(callback)
-
-    // Remember the latest callback.
-    useEffect(() => {
-        savedCallback.current = callback
-    }, [callback])
-
-    // Set up the interval.
-    useEffect(() => {
-        function tick() {
-            savedCallback.current()
-        }
-        if (delay !== null) {
-            const id = setInterval(tick, delay)
-            return () => clearInterval(id)
-        }
-        return
-    }, [delay])
-}
-
 export const useDebounceCallback = (callback: any, delay: number) => {
     return useRef(debounce(callback, delay)).current
 }

From e68b7d3a40e6dc56b927499be983bed2e0a332a1 Mon Sep 17 00:00:00 2001
From: Ike Saunders <ikesau@protonmail.com>
Date: Thu, 12 Oct 2023 15:59:37 -0400
Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=90=9B=20fix=20lightning=20updates?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 adminSiteClient/gdocsDeploy.ts | 42 +++++++++++++++++++++++++---------
 adminSiteServer/apiRouter.ts   |  7 +++++-
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/adminSiteClient/gdocsDeploy.ts b/adminSiteClient/gdocsDeploy.ts
index e8e6a1b900c..5183672c1d7 100644
--- a/adminSiteClient/gdocsDeploy.ts
+++ b/adminSiteClient/gdocsDeploy.ts
@@ -32,6 +32,7 @@ export const checkIsLightningUpdate = (
         "imageMetadata",
         "linkedCharts",
         "errors",
+        "revisionId",
     ]
 
     const lightningContentProps: Array<keyof OwidGdocContent> = [
@@ -50,19 +51,26 @@ export const checkIsLightningUpdate = (
         ...lightningContentProps.map((prop) => `content.${prop}`),
     ]
 
+    // When this function is called from server-side code and a Gdoc object
+    // is passed in, the omit() call will surface Gdoc class members. The
+    // comparison will then fail if the other operand in the comparison is
+    // an OwidGdocInterface object. To avoid this, we spread into new objects
+    // in order to compare the same types. Tags are stringified to avoid a similar
+    // issue with Dates and date strings.
+    const prevOmitted = omit(
+        { ...prevGdoc, tags: nextGdoc.tags?.map((tag) => JSON.stringify(tag)) },
+        lightningProps
+    )
+    const nextOmitted = omit(
+        { ...nextGdoc, tags: prevGdoc.tags?.map((tag) => JSON.stringify(tag)) },
+        lightningProps
+    )
+
     return (
         hasChanges &&
         prevGdoc.published &&
         nextGdoc.published &&
-        // When this function is called from server-side code and a Gdoc object
-        // is passed in, the omit() call will surface Gdoc class members. The
-        // comparison will then fail if the other operand in the comparison is
-        // an OwidGdocInterface object. To avoid this, we spread into new objects
-        // in order to compare the same types.
-        isEqual(
-            omit({ ...prevGdoc }, lightningProps),
-            omit({ ...nextGdoc }, lightningProps)
-        )
+        isEqual(prevOmitted, nextOmitted)
     )
 }
 
@@ -71,6 +79,18 @@ export const checkHasChanges = (
     nextGdoc: OwidGdocInterface
 ) =>
     !isEqual(
-        omit(prevGdoc, GDOC_DIFF_OMITTABLE_PROPERTIES),
-        omit(nextGdoc, GDOC_DIFF_OMITTABLE_PROPERTIES)
+        omit(
+            {
+                ...prevGdoc,
+                tags: prevGdoc.tags?.map((tag) => JSON.stringify(tag)),
+            },
+            GDOC_DIFF_OMITTABLE_PROPERTIES
+        ),
+        omit(
+            {
+                ...nextGdoc,
+                tags: nextGdoc.tags?.map((tag) => JSON.stringify(tag)),
+            },
+            GDOC_DIFF_OMITTABLE_PROPERTIES
+        )
     )
diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts
index 45c4a14afac..d1eb2632fc2 100644
--- a/adminSiteServer/apiRouter.ts
+++ b/adminSiteServer/apiRouter.ts
@@ -2490,7 +2490,12 @@ apiRouter.put("/gdocs/:id", async (req, res) => {
         return updated
     }
 
-    const prevGdoc = await Gdoc.findOneBy({ id })
+    const prevGdoc = await Gdoc.findOne({
+        where: {
+            id: id,
+        },
+        relations: ["tags"],
+    })
     if (!prevGdoc) throw new JsonError(`No Google Doc with id ${id} found`)
 
     const nextGdoc = dataSource

From e7a0609ab098018ddf22bd4d86d24f6ce1829811 Mon Sep 17 00:00:00 2001
From: Ike Saunders <ikesau@protonmail.com>
Date: Fri, 13 Oct 2023 10:29:43 -0400
Subject: [PATCH 3/4] =?UTF-8?q?=E2=9C=A8=20make=20lightning=20prop=20decla?=
 =?UTF-8?q?ration=20more=20explicit?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 adminSiteClient/GdocsPreviewPage.tsx |  1 -
 adminSiteClient/gdocsDeploy.ts       | 87 ++++++++++++++++++++--------
 2 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/adminSiteClient/GdocsPreviewPage.tsx b/adminSiteClient/GdocsPreviewPage.tsx
index 97548780671..52721054387 100644
--- a/adminSiteClient/GdocsPreviewPage.tsx
+++ b/adminSiteClient/GdocsPreviewPage.tsx
@@ -2,7 +2,6 @@ import React, {
     useCallback,
     useContext,
     useEffect,
-    useMemo,
     useRef,
     useState,
 } from "react"
diff --git a/adminSiteClient/gdocsDeploy.ts b/adminSiteClient/gdocsDeploy.ts
index 5183672c1d7..414a7cc0c0b 100644
--- a/adminSiteClient/gdocsDeploy.ts
+++ b/adminSiteClient/gdocsDeploy.ts
@@ -26,29 +26,70 @@ export const checkIsLightningUpdate = (
     nextGdoc: OwidGdocInterface,
     hasChanges: boolean
 ) => {
-    const lightningArticleProps: Array<keyof OwidGdocInterface> = [
-        "updatedAt",
-        "linkedDocuments",
-        "imageMetadata",
-        "linkedCharts",
-        "errors",
-        "revisionId",
-    ]
+    // lightning props are props that do not require a full rebake of the site if changed, because
+    // their impact is limited to just this article. They are marked as "true" in these two config maps.
+    // The props that *do* require a full rebake if changed are marked "false".
+    const lightningPropConfigMap: Record<
+        Exclude<keyof OwidGdocInterface, "content">,
+        boolean
+    > = {
+        breadcrumbs: true,
+        errors: true,
+        imageMetadata: true,
+        linkedCharts: true,
+        linkedDocuments: true,
+        relatedCharts: true,
+        revisionId: true,
+        updatedAt: true,
+        createdAt: false,
+        id: false, // weird case - can't be updated
+        publicationContext: false, // requires an update of the blog roll
+        published: false, // requires an update of the blog roll
+        publishedAt: false, // could require an update of the blog roll
+        slug: false, // requires updating any articles that link to it
+        tags: false, // requires updating related articles on grapher pages
+    }
 
-    const lightningContentProps: Array<keyof OwidGdocContent> = [
-        "body",
-        "subtitle",
-        "toc",
-        "supertitle",
-        "refs",
-        "summary",
-        "cover-image",
-        "cover-color",
-    ]
+    const lightningPropContentConfigMap: Record<
+        keyof OwidGdocContent,
+        boolean
+    > = {
+        "cover-color": true,
+        "cover-image": true,
+        "hide-citation": true,
+        body: true,
+        dateline: true,
+        details: true,
+        refs: true,
+        subtitle: true,
+        summary: true,
+        "sticky-nav": true,
+        supertitle: true,
+        toc: true,
+        "atom-excerpt": false, // requires updating the atom feed / blog roll
+        "atom-title": false, // requires updating the atom feed / blog roll
+        "featured-image": false, // requires updating references to this article
+        authors: false, // requires updating references to this article
+        excerpt: false, // requires updating references to this article
+        faqs: false, // requires updating datapages
+        parsedFaqs: false, // requires updating datapages
+        title: false, // requires updating references to this article
+        type: false, // could require updating other content if switching type to fragment
+    }
+
+    const getLightningPropKeys = (configMap: Record<string, boolean>) =>
+        Object.entries(configMap)
+            .filter(([_, isLightningProp]) => isLightningProp)
+            .map(([key]) => key)
+
+    const lightningPropKeys = getLightningPropKeys(lightningPropConfigMap)
+    const lightningPropContentKeys = getLightningPropKeys(
+        lightningPropContentConfigMap
+    )
 
-    const lightningProps = [
-        ...lightningArticleProps,
-        ...lightningContentProps.map((prop) => `content.${prop}`),
+    const keysToOmit = [
+        ...lightningPropKeys,
+        ...lightningPropContentKeys.map((key) => `content.${key}`),
     ]
 
     // When this function is called from server-side code and a Gdoc object
@@ -59,11 +100,11 @@ export const checkIsLightningUpdate = (
     // issue with Dates and date strings.
     const prevOmitted = omit(
         { ...prevGdoc, tags: nextGdoc.tags?.map((tag) => JSON.stringify(tag)) },
-        lightningProps
+        keysToOmit
     )
     const nextOmitted = omit(
         { ...nextGdoc, tags: prevGdoc.tags?.map((tag) => JSON.stringify(tag)) },
-        lightningProps
+        keysToOmit
     )
 
     return (

From c7f4f261ab5e349ac1669622e67b93c16b25e3b5 Mon Sep 17 00:00:00 2001
From: Ike Saunders <ikesau@protonmail.com>
Date: Wed, 18 Oct 2023 21:54:09 +0000
Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=90=9B=20fix=20misguided=20tag-saving?=
 =?UTF-8?q?=20logic?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 adminSiteClient/GdocsPreviewPage.tsx | 34 +---------------------------
 adminSiteClient/GdocsStore.tsx       | 10 +++++++-
 adminSiteClient/gdocsDeploy.ts       | 17 +++++---------
 3 files changed, 16 insertions(+), 45 deletions(-)

diff --git a/adminSiteClient/GdocsPreviewPage.tsx b/adminSiteClient/GdocsPreviewPage.tsx
index 52721054387..f5418353476 100644
--- a/adminSiteClient/GdocsPreviewPage.tsx
+++ b/adminSiteClient/GdocsPreviewPage.tsx
@@ -21,12 +21,7 @@ import {
 } from "@ourworldindata/utils"
 import { Button, Col, Drawer, Row, Space, Tag, Typography } from "antd"
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js"
-import {
-    faGear,
-    faArrowsRotate,
-    faExclamationTriangle,
-    faAngleLeft,
-} from "@fortawesome/free-solid-svg-icons"
+import { faGear, faAngleLeft } from "@fortawesome/free-solid-svg-icons"
 
 import { getErrors } from "./gdocsValidation.js"
 import { GdocsSaveButtons } from "./GdocsSaveButtons.js"
@@ -59,7 +54,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
     }
     const hasChanges = useGdocsChanged(originalGdoc, currentGdoc)
     const [isSettingsOpen, setSettingsOpen] = useState(false)
-    const [hasSyncingError, setHasSyncingError] = useState(false)
     const [criticalErrorMessage, setCriticalErrorMessage] = useState<
         undefined | string
     >()
@@ -87,9 +81,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
         if (checkIsPlainObjectWithGuard(error) && error.status === 500) {
             console.log("Critical error", error)
             setCriticalErrorMessage(error.message as string)
-        } else {
-            console.log("Syncing error", error)
-            setHasSyncingError(true)
         }
     }, [])
 
@@ -230,29 +221,6 @@ export const GdocsPreviewPage = ({ match, history }: GdocsMatchProps) => {
                                 {!currentGdoc.published && (
                                     <Tag color="default">Draft</Tag>
                                 )}
-                                {hasSyncingError ? (
-                                    <Tag
-                                        icon={
-                                            <FontAwesomeIcon
-                                                icon={faExclamationTriangle}
-                                            />
-                                        }
-                                        color="warning"
-                                    >
-                                        Syncing error, retrying...
-                                    </Tag>
-                                ) : (
-                                    <Tag
-                                        icon={
-                                            <FontAwesomeIcon
-                                                icon={faArrowsRotate}
-                                            />
-                                        }
-                                        color="success"
-                                    >
-                                        preview
-                                    </Tag>
-                                )}
                             </div>
                         </Space>
                     </Col>
diff --git a/adminSiteClient/GdocsStore.tsx b/adminSiteClient/GdocsStore.tsx
index 9d0a356dc6d..c808100cf82 100644
--- a/adminSiteClient/GdocsStore.tsx
+++ b/adminSiteClient/GdocsStore.tsx
@@ -2,6 +2,7 @@ import React, { useContext, createContext, useState } from "react"
 import { action, observable } from "mobx"
 import {
     getOwidGdocFromJSON,
+    omit,
     OwidGdocInterface,
     OwidGdocJSON,
     Tag,
@@ -33,7 +34,14 @@ export class GdocsStore {
     @action
     async update(gdoc: OwidGdocInterface): Promise<OwidGdocInterface> {
         return this.admin
-            .requestJSON<OwidGdocJSON>(`/api/gdocs/${gdoc.id}`, gdoc, "PUT")
+            .requestJSON<OwidGdocJSON>(
+                `/api/gdocs/${gdoc.id}`,
+                // omitting tags because they get saved via the /api/gdocs/:id/setTags route, not this /api/gdocs/:id route
+                // If we were to save them here, it could lead to updates from this request
+                // overwriting tags that had been set by someone else after the preview page was loaded
+                omit(gdoc, "tags"),
+                "PUT"
+            )
             .then(getOwidGdocFromJSON)
     }
 
diff --git a/adminSiteClient/gdocsDeploy.ts b/adminSiteClient/gdocsDeploy.ts
index 414a7cc0c0b..d37d61b2e10 100644
--- a/adminSiteClient/gdocsDeploy.ts
+++ b/adminSiteClient/gdocsDeploy.ts
@@ -40,6 +40,9 @@ export const checkIsLightningUpdate = (
         linkedDocuments: true,
         relatedCharts: true,
         revisionId: true,
+        // "tags" is not actually a lightning prop, as it requires rebaking other parts of the site,
+        // but they're set via the /setTags route and so should be ignored here
+        tags: true,
         updatedAt: true,
         createdAt: false,
         id: false, // weird case - can't be updated
@@ -47,7 +50,6 @@ export const checkIsLightningUpdate = (
         published: false, // requires an update of the blog roll
         publishedAt: false, // could require an update of the blog roll
         slug: false, // requires updating any articles that link to it
-        tags: false, // requires updating related articles on grapher pages
     }
 
     const lightningPropContentConfigMap: Record<
@@ -96,16 +98,9 @@ export const checkIsLightningUpdate = (
     // is passed in, the omit() call will surface Gdoc class members. The
     // comparison will then fail if the other operand in the comparison is
     // an OwidGdocInterface object. To avoid this, we spread into new objects
-    // in order to compare the same types. Tags are stringified to avoid a similar
-    // issue with Dates and date strings.
-    const prevOmitted = omit(
-        { ...prevGdoc, tags: nextGdoc.tags?.map((tag) => JSON.stringify(tag)) },
-        keysToOmit
-    )
-    const nextOmitted = omit(
-        { ...nextGdoc, tags: prevGdoc.tags?.map((tag) => JSON.stringify(tag)) },
-        keysToOmit
-    )
+    // in order to compare the same types.
+    const prevOmitted = omit({ ...prevGdoc }, keysToOmit)
+    const nextOmitted = omit({ ...nextGdoc }, keysToOmit)
 
     return (
         hasChanges &&