Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Gdoc Preview autoupdating, fix lightning publishing #2766

Merged
merged 4 commits into from
Oct 24, 2023
Merged

Remove Gdoc Preview autoupdating, fix lightning publishing #2766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9280f7d
✨ remove autosave from gdocs preview page
ikesau Oct 12, 2023
e68b7d3
🐛 fix lightning updates
ikesau Oct 12, 2023
e7a0609
✨ make lightning prop declaration more explicit
ikesau Oct 13, 2023
c7f4f26
🐛 fix misguided tag-saving logic
ikesau Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions adminSiteClient/Admin.tsx
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading