🐛 fix misguided tag-saving logic

owid · Oct 18, 2023 · c7f4f26 · c7f4f26
1 parent e7a0609
commit c7f4f26
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 45 deletions.
diff --git 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
@@ -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
@@ -40,14 +40,16 @@ 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
         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 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 &&