Skip to content

Commit

Permalink
Merge pull request #2766 from owid/fix-unpublishing
Browse files Browse the repository at this point in the history 
Remove Gdoc Preview autoupdating, fix lightning publishing
  • Loading branch information
@ikesau
ikesau authored Oct 24, 2023
2 parents 42552f7 + c7f4f26 commit 847e883
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 148 deletions.
8 changes: 0 additions & 8 deletions adminSiteClient/Admin.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -132,7 +126,6 @@ export class Admin {
abortController
)
this.addRequest(request)
this.currentRequestAbortControllers.set(request, abortController)

response = await request
text = await response.text()
Expand All @@ -154,7 +147,6 @@ export class Admin {
} finally {
if (request) {
this.removeRequest(request)
this.currentRequestAbortControllers.delete(request)
}
}

Expand Down
86 changes: 1 addition & 85 deletions adminSiteClient/GdocsPreviewPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, {
useCallback,
useContext,
useEffect,
useMemo,
useRef,
useState,
} from "react"
Expand All @@ -19,16 +18,10 @@ 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"
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"
Expand All @@ -40,7 +33,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
Expand All @@ -62,23 +54,13 @@ 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
>()
const [isDiffOpen, setDiffOpen] = useState(false)
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)

Expand All @@ -99,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)
}
}, [])

Expand All @@ -124,42 +103,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,
Expand All @@ -178,7 +121,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")
Expand All @@ -190,7 +132,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}`)
Expand All @@ -205,7 +146,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")
Expand Down Expand Up @@ -281,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>
Expand Down Expand Up @@ -389,7 +306,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
Expand Down
10 changes: 9 additions & 1 deletion adminSiteClient/GdocsStore.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useContext, createContext, useState } from "react"
import { action, observable } from "mobx"
import {
getOwidGdocFromJSON,
omit,
OwidGdocInterface,
OwidGdocJSON,
Tag,
Expand Down Expand Up @@ -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)
}

Expand Down
118 changes: 87 additions & 31 deletions adminSiteClient/gdocsDeploy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,43 +26,87 @@ export const checkIsLightningUpdate = (
nextGdoc: OwidGdocInterface,
hasChanges: boolean
) => {
const lightningArticleProps: Array<keyof OwidGdocInterface> = [
"updatedAt",
"linkedDocuments",
"imageMetadata",
"linkedCharts",
"errors",
]
// 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,
// "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
}

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
// 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.
const prevOmitted = omit({ ...prevGdoc }, keysToOmit)
const nextOmitted = omit({ ...nextGdoc }, keysToOmit)

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)
)
}

Expand All @@ -71,6 +115,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
)
)
7 changes: 6 additions & 1 deletion adminSiteServer/apiRouter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 847e883

Please sign in to comment.