From 6264e5840babfcff42993bfe4deb28f6348f8e44 Mon Sep 17 00:00:00 2001 From: Ike Saunders Date: Tue, 3 Dec 2024 15:58:54 -0500 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=90=9B=20fix=20image=20upload=20route?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- db/db.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/db.ts b/db/db.ts index bab8530fc3e..9f4e4c69528 100644 --- a/db/db.ts +++ b/db/db.ts @@ -737,7 +737,7 @@ export function getCloudflareImage( `-- sql SELECT * FROM images - WHERE i.filename = ?`, + WHERE filename = ?`, [filename] ) } From 5e67b9715a8f50306d9a9576c79ad9b3903f9a5c Mon Sep 17 00:00:00 2001 From: Ike Saunders Date: Tue, 3 Dec 2024 15:59:10 -0500 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=94=A8=20WIP=20image=20PUT?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/ImagesIndexPage.tsx | 73 +++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 20 deletions(-) diff --git a/adminSiteClient/ImagesIndexPage.tsx b/adminSiteClient/ImagesIndexPage.tsx index 31632fabbf5..b959885c916 100644 --- a/adminSiteClient/ImagesIndexPage.tsx +++ b/adminSiteClient/ImagesIndexPage.tsx @@ -37,6 +37,16 @@ type ImageEditorApi = { image: DbEnrichedImageWithUserId, patch: Partial ) => void + putImage: (payload: { + filename: string + content?: string + type: string + }) => void + postImage: (payload: { + filename: string + content?: string + type: string + }) => void deleteImage: (image: DbEnrichedImageWithUserId) => void getImages: () => void getUsers: () => void @@ -286,6 +296,9 @@ function createColumns({ width: 100, render: (_, image) => ( + api.putImage(payload)}> + + > - admin: Admin + children: React.ReactNode + onRequest: (payload: { + filename: string + content?: string + type: string + }) => void }) { function uploadImage({ file }: { file: string | Blob | RcFile }) { if (typeof file === "string") return @@ -323,23 +340,13 @@ function ImageUploadButton({ type: file.type, } - const { image } = await admin.requestJSON<{ - sucess: true - image: DbEnrichedImageWithUserId - }>("/api/image", payload, "POST") - - setImages((imagesMap) => ({ - ...imagesMap, - [image.id]: image, - })) + onRequest(payload) } reader.readAsDataURL(file) } return ( - + {children} ) } @@ -382,13 +389,33 @@ export function ImageIndexPage() { [image.id]: response.image, })) }, + postImage: async (image) => { + const response = await admin.requestJSON<{ + success: true + image: DbEnrichedImageWithUserId + }>(`/api/image`, image, "POST") + setImages((prevMap) => ({ + ...prevMap, + [response.image.id]: response.image, + })) + }, + putImage: async (image) => { + const response = await admin.requestJSON<{ + success: true + image: DbEnrichedImageWithUserId + }>(`/api/image/${image.id}`, image, "PUT") + setImages((prevMap) => ({ + ...prevMap, + [image.id]: response.image, + })) + }, postUserImage: async (user, image) => { - const result = await admin.requestJSON( + const response = await admin.requestJSON( `/api/users/${user.id}/image/${image.id}`, {}, "POST" ) - if (result.success) { + if (response.success) { setImages((prevMap) => ({ ...prevMap, [image.id]: { ...prevMap[image.id], userId: user.id }, @@ -439,7 +466,13 @@ export function ImageIndexPage() { onChange={(e) => setFilenameSearchValue(e.target.value)} style={{ width: 500, marginBottom: 20 }} /> - + api.postImage(payload)} + > + + From 5de66285ca5013b597510e65a58679bc5d0db00e Mon Sep 17 00:00:00 2001 From: Ike Saunders Date: Tue, 3 Dec 2024 18:02:15 -0500 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=8E=89=20add=20upload=20new=20version?= =?UTF-8?q?=20of=20image=20functionality?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/ImagesIndexPage.tsx | 202 ++++++++++++++++++---------- adminSiteClient/admin.scss | 5 + adminSiteServer/apiRouter.ts | 139 +++++++++---------- adminSiteServer/imagesHelpers.ts | 103 ++++++++++++++ 4 files changed, 304 insertions(+), 145 deletions(-) create mode 100644 adminSiteServer/imagesHelpers.ts diff --git a/adminSiteClient/ImagesIndexPage.tsx b/adminSiteClient/ImagesIndexPage.tsx index b959885c916..0eeff9194b2 100644 --- a/adminSiteClient/ImagesIndexPage.tsx +++ b/adminSiteClient/ImagesIndexPage.tsx @@ -3,18 +3,10 @@ import React, { useContext, useEffect, useMemo, + useRef, useState, } from "react" -import { - Button, - Flex, - Input, - Mentions, - Popconfirm, - Space, - Table, - Upload, -} from "antd" +import { Button, Flex, Input, Mentions, Popconfirm, Table, Upload } from "antd" import { AdminLayout } from "./AdminLayout.js" import { AdminAppContext } from "./AdminAppContext.js" import { DbEnrichedImageWithUserId, DbPlainUser } from "@ourworldindata/types" @@ -22,7 +14,6 @@ import { Timeago } from "./Forms.js" import { ColumnsType } from "antd/es/table/InternalTable.js" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome" import { faClose, faUpload } from "@fortawesome/free-solid-svg-icons" -import { Admin } from "./Admin.js" import { RcFile } from "antd/es/upload/interface.js" import TextArea from "antd/es/input/TextArea.js" import { CLOUDFLARE_IMAGES_URL } from "../settings/clientSettings.js" @@ -37,11 +28,14 @@ type ImageEditorApi = { image: DbEnrichedImageWithUserId, patch: Partial ) => void - putImage: (payload: { - filename: string - content?: string - type: string - }) => void + putImage: ( + id: number, + payload: { + filename: string + content?: string + type: string + } + ) => void postImage: (payload: { filename: string content?: string @@ -124,7 +118,11 @@ function UserSelect({ } const handleSelect = async (option: { value?: string; label?: string }) => { - const selectedUser = usersMap[option.value!] + // iterating because we only have the label when using the admin context + const selectedUser = Object.values(usersMap).find( + (user) => user.fullName === option.label + ) + if (selectedUser) { setValue(selectedUser.fullName) await onUserSelect(selectedUser) @@ -151,7 +149,7 @@ function UserSelect({
@@ -162,6 +160,33 @@ function UserSelect({ ) } +// when updatedAt changes, the image will reload the src +// but it looks like sometimes cloudflare doesn't update in time :( +function ImgWithRefresh({ + src, + updatedAt, +}: { + src: string + updatedAt: number | null +}) { + const ref = useRef(null) + useEffect(() => { + if (ref.current && updatedAt) { + ref.current.src = "" + fetch(src, { cache: "reload" }) + .then(() => { + if (ref.current) { + ref.current.src = src + } + }) + .catch(() => { + console.log("Something went wrong refreshing the image") + }) + } + }) + return +} + function createColumns({ api, users, @@ -175,7 +200,7 @@ function createColumns({ dataIndex: "cloudflareId", width: 100, key: "cloudflareId", - render: (cloudflareId, { originalWidth }) => { + render: (cloudflareId, { originalWidth, updatedAt }) => { const srcFor = (w: number) => `${CLOUDFLARE_IMAGES_URL}/${encodeURIComponent( cloudflareId @@ -187,9 +212,9 @@ function createColumns({ href={`${srcFor(originalWidth!)}`} rel="noopener" > -
@@ -295,10 +320,8 @@ function createColumns({ key: "action", width: 100, render: (_, image) => ( - - api.putImage(payload)}> - - + + - + ), }, ] } -function ImageUpload({ - children, - onRequest, -}: { - children: React.ReactNode - onRequest: (payload: { - filename: string - content?: string - type: string - }) => void -}) { - function uploadImage({ file }: { file: string | Blob | RcFile }) { - if (typeof file === "string") return +type File = string | Blob | RcFile - const reader = new FileReader() - reader.onload = async () => { - const base64Data = reader.result?.toString() +type FileToBase64Result = { + filename: string + content: string + type: string +} - const payload = { +/** + * Uploading as base64, because otherwise we'd need multipart/form-data parsing middleware in the server. + * This seems easier as a one-off. + **/ +function fileToBase64(file: File): Promise { + if (typeof file === "string") return Promise.resolve(null) + + return new Promise((resolve) => { + const reader = new FileReader() + reader.onload = () => { + resolve({ filename: file.name, - content: base64Data, + content: reader.result?.toString() ?? "", type: file.type, - } - - onRequest(payload) + }) } reader.readAsDataURL(file) + }) +} + +function PostImageButton({ + postImage, +}: { + postImage: ImageEditorApi["postImage"] +}) { + async function uploadImage({ file }: { file: File }) { + const result = await fileToBase64(file) + if (result) { + postImage(result) + } } return ( - {children} + + + ) +} + +function PutImageButton({ + putImage, + id, +}: { + putImage: ImageEditorApi["putImage"] + id: number +}) { + async function uploadImage({ file }: { file: File }) { + const result = await fileToBase64(file) + if (result) { + putImage(id, result) + } + } + return ( + + ) } @@ -384,34 +442,40 @@ export function ImageIndexPage() { success: true image: DbEnrichedImageWithUserId }>(`/api/images/${image.id}`, patch, "PATCH") - setImages((prevMap) => ({ - ...prevMap, - [image.id]: response.image, - })) + if (response.success) { + setImages((prevMap) => ({ + ...prevMap, + [image.id]: response.image, + })) + } }, postImage: async (image) => { const response = await admin.requestJSON<{ success: true image: DbEnrichedImageWithUserId - }>(`/api/image`, image, "POST") - setImages((prevMap) => ({ - ...prevMap, - [response.image.id]: response.image, - })) + }>(`/api/images`, image, "POST") + if (response.success) { + setImages((prevMap) => ({ + ...prevMap, + [response.image.id]: response.image, + })) + } }, - putImage: async (image) => { + putImage: async (id, payload) => { const response = await admin.requestJSON<{ success: true image: DbEnrichedImageWithUserId - }>(`/api/image/${image.id}`, image, "PUT") - setImages((prevMap) => ({ - ...prevMap, - [image.id]: response.image, - })) + }>(`/api/images/${id}`, payload, "PUT") + if (response.success) { + setImages((prevMap) => ({ + ...prevMap, + [id]: response.image, + })) + } }, postUserImage: async (user, image) => { const response = await admin.requestJSON( - `/api/users/${user.id}/image/${image.id}`, + `/api/users/${user.id}/images/${image.id}`, {}, "POST" ) @@ -424,7 +488,7 @@ export function ImageIndexPage() { }, deleteUserImage: async (user, image) => { const result = await admin.requestJSON( - `/api/users/${user.id}/image/${image.id}`, + `/api/users/${user.id}/images/${image.id}`, {}, "DELETE" ) @@ -466,13 +530,7 @@ export function ImageIndexPage() { onChange={(e) => setFilenameSearchValue(e.target.value)} style={{ width: 500, marginBottom: 20 }} /> - api.postImage(payload)} - > - - +
diff --git a/adminSiteClient/admin.scss b/adminSiteClient/admin.scss index b1d4b8865e6..a0a13798d6e 100644 --- a/adminSiteClient/admin.scss +++ b/adminSiteClient/admin.scss @@ -1219,4 +1219,9 @@ main:not(.ChartEditorPage):not(.GdocsEditPage) { width: 16px; vertical-align: -2px; } + + .ImageIndexPage__update-image-button { + color: #007bff; + margin-bottom: 8px; + } } diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 48484dbfbe4..d52e2a71d9a 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -1,7 +1,6 @@ /* eslint @typescript-eslint/no-unused-vars: [ "warn", { argsIgnorePattern: "^(res|req)$" } ] */ import * as lodash from "lodash" -import sharp from "sharp" import * as db from "../db/db.js" import { UNCATEGORIZED_TAG_ID, @@ -10,8 +9,6 @@ import { ADMIN_BASE_URL, DATA_API_URL, FEATURE_FLAGS, - CLOUDFLARE_IMAGES_ACCOUNT_ID, - CLOUDFLARE_IMAGES_API_KEY, } from "../settings/serverSettings.js" import { FeatureFlagFeature } from "../settings/clientSettings.js" import { expectInt, isValidSlug } from "../serverUtils/serverUtil.js" @@ -198,6 +195,12 @@ import { updateChartConfigInDbAndR2, } from "./chartConfigHelpers.js" import { CHART_VIEW_PROPS_TO_PERSIST } from "../db/model/ChartView.js" +import { + deleteFromCloudflare, + processImageContent, + uploadToCloudflare, + validateImagePayload, +} from "./imagesHelpers.js" const apiRouter = new FunctionalRouter() @@ -1223,7 +1226,7 @@ postRouteWithRWTransaction( postRouteWithRWTransaction( apiRouter, - "/users/:userId/image/:imageId", + "/users/:userId/images/:imageId", async (req, res, trx) => { const userId = expectInt(req.params.userId) const imageId = expectInt(req.params.imageId) @@ -1234,7 +1237,7 @@ postRouteWithRWTransaction( deleteRouteWithRWTransaction( apiRouter, - "/users/:userId/image/:imageId", + "/users/:userId/images/:imageId", async (req, res, trx) => { const userId = expectInt(req.params.userId) const imageId = expectInt(req.params.imageId) @@ -3103,18 +3106,8 @@ getRouteNonIdempotentWithRWTransaction( } ) -postRouteWithRWTransaction(apiRouter, "/image", async (req, res, trx) => { - const { filename, type, content } = req.body - if (!filename || !type || !content) { - throw new JsonError("Missing required fields", 400) - } - if ( - typeof filename !== "string" || - typeof type !== "string" || - typeof content !== "string" - ) { - throw new JsonError("Invalid field types", 400) - } +postRouteWithRWTransaction(apiRouter, "/images", async (req, res, trx) => { + const { filename, type, content } = validateImagePayload(req.body) const preexisting = await trx("images") .where("filename", "=", filename) @@ -3127,46 +3120,9 @@ postRouteWithRWTransaction(apiRouter, "/image", async (req, res, trx) => { } } - // Strip the data URL prefix - const stripped = content.slice(content.indexOf(",") + 1) - const asBuffer = Buffer.from(stripped, "base64") - const asBlob = new Blob([asBuffer], { type: type }) - const dimensions = await sharp(asBuffer) - .metadata() - .then(({ width, height }) => ({ width, height })) - - const body = new FormData() - body.append("file", asBlob, filename) - body.append("id", encodeURIComponent(filename)) - body.append("metadata", JSON.stringify({ filename })) - body.append("requireSignedURLs", "false") - - console.log("Uploading image:", filename) - const response = await fetch( - `https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_IMAGES_ACCOUNT_ID}/images/v1`, - { - method: "POST", - headers: { - Authorization: `Bearer ${CLOUDFLARE_IMAGES_API_KEY}`, - }, - body, - } - ).then((res) => res.json()) - - if (response.errors.length) { - if (response.errors.length === 1) { - if (response.errors[0].code === 5409) { - throw new JsonError( - "An image with this filename already exists in Cloudflare Images but isn't tracked in the database. Please contact a developer for help." - ) - } - } - - console.error("Error uploading image to Cloudflare:", response.errors) - throw new JsonError(JSON.stringify(response.errors)) - } + const { asBlob, dimensions } = await processImageContent(content, type) - const cloudflareId = response.result.id + const cloudflareId = await uploadToCloudflare(filename, asBlob) if (!cloudflareId) { return { @@ -3194,6 +3150,56 @@ postRouteWithRWTransaction(apiRouter, "/image", async (req, res, trx) => { } }) +/** + * Similar to the POST route, but for updating an existing image. Deletes the image + * from Cloudflare and re-uploads it with the new content. The filename will stay the same, + * but the dimensions will be updated. + */ +putRouteWithRWTransaction(apiRouter, "/images/:id", async (req, res, trx) => { + const { id } = req.params + + const image = await trx("images") + .where("id", "=", id) + .first() + + if (!image) { + throw new JsonError(`No image found for id ${id}`, 404) + } + + const originalCloudflareId = image.cloudflareId + const originalFilename = image.filename + + if (!originalCloudflareId) { + throw new JsonError( + `Image with id ${id} has no associated Cloudflare image`, + 400 + ) + } + + await deleteFromCloudflare(originalCloudflareId) + + const { type, content } = validateImagePayload(req.body) + const { asBlob, dimensions } = await processImageContent(content, type) + const newCloudflareId = await uploadToCloudflare(originalFilename, asBlob) + + if (!newCloudflareId) { + throw new JsonError("Failed to upload image", 500) + } + + await trx("images").where("id", "=", id).update({ + originalWidth: dimensions.width, + originalHeight: dimensions.height, + updatedAt: new Date().getTime(), + }) + + const updated = await db.getCloudflareImage(trx, originalFilename) + + return { + success: true, + image: updated, + } +}) + // Update alt text via patch patchRouteWithRWTransaction(apiRouter, "/images/:id", async (req, res, trx) => { const { id } = req.params @@ -3238,25 +3244,12 @@ deleteRouteWithRWTransaction( if (!image) { throw new JsonError(`No image found for id ${id}`, 404) } - - const response = await fetch( - `https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_IMAGES_ACCOUNT_ID}/images/v1/${encodeURIComponent(image.cloudflareId!)}`, - { - method: "DELETE", - headers: { - Authorization: `Bearer ${CLOUDFLARE_IMAGES_API_KEY}`, - }, - } - ).then((res) => res.json()) - - if (response.errors.length) { - console.error( - "Error deleting image from Cloudflare:", - response.errors - ) - throw new JsonError(JSON.stringify(response.errors)) + if (!image.cloudflareId) { + throw new JsonError(`Image does not have a cloudflare ID`, 400) } + await deleteFromCloudflare(image.cloudflareId) + await trx("images").where({ id }).delete() return { diff --git a/adminSiteServer/imagesHelpers.ts b/adminSiteServer/imagesHelpers.ts new file mode 100644 index 00000000000..edda44f13a8 --- /dev/null +++ b/adminSiteServer/imagesHelpers.ts @@ -0,0 +1,103 @@ +import { JsonError } from "@ourworldindata/types" +import sharp from "sharp" +import { + CLOUDFLARE_IMAGES_ACCOUNT_ID, + CLOUDFLARE_IMAGES_API_KEY, +} from "../settings/serverSettings.js" + +export function validateImagePayload(body: any): { + filename: string + type: string + content: string +} { + const { filename, type, content } = body + if (!filename || !type || !content) { + throw new JsonError("Missing required fields", 400) + } + if ( + typeof filename !== "string" || + typeof type !== "string" || + typeof content !== "string" + ) { + throw new JsonError("Invalid field types", 400) + } + return { filename, type, content } +} + +export async function processImageContent( + content: string, + type: string +): Promise<{ + asBlob: Blob + dimensions: { width: number; height: number } +}> { + const stripped = content.slice(content.indexOf(",") + 1) + const asBuffer = Buffer.from(stripped, "base64") + const asBlob = new Blob([asBuffer], { type }) + const { width, height } = await sharp(asBuffer) + .metadata() + .then(({ width, height }) => ({ width, height })) + + if (!width || !height) { + throw new JsonError("Invalid image dimensions", 400) + } + + return { + asBlob, + dimensions: { + width, + height, + }, + } +} + +export async function uploadToCloudflare(filename: string, blob: Blob) { + const body = new FormData() + body.append("file", blob, filename) + body.append("id", encodeURIComponent(filename)) + body.append("metadata", JSON.stringify({ filename })) + body.append("requireSignedURLs", "false") + + console.log("Uploading image:", filename) + const response = await fetch( + `https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_IMAGES_ACCOUNT_ID}/images/v1`, + { + method: "POST", + headers: { + Authorization: `Bearer ${CLOUDFLARE_IMAGES_API_KEY}`, + }, + body, + } + ).then((res) => res.json()) + + if (response.errors.length) { + if (response.errors.length === 1 && response.errors[0].code === 5409) { + throw new JsonError( + "An image with this filename already exists in Cloudflare Images but isn't tracked in the database. Please contact a developer for help." + ) + } + console.error("Error uploading image to Cloudflare:", response.errors) + throw new JsonError(JSON.stringify(response.errors)) + } + + return response.result.id +} + +export async function deleteFromCloudflare(cloudflareId: string) { + const response = await fetch( + `https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_IMAGES_ACCOUNT_ID}/images/v1/${encodeURIComponent( + cloudflareId + )}`, + { + method: "DELETE", + headers: { + Authorization: `Bearer ${CLOUDFLARE_IMAGES_API_KEY}`, + }, + } + ).then((res) => res.json()) + + if (response.errors.length) { + console.error("Error deleting image from Cloudflare:", response.errors) + throw new JsonError(JSON.stringify(response.errors)) + } +} From d6fb57b916cc5f1f093d5e2af976eef8d11c2e9d Mon Sep 17 00:00:00 2001 From: Ike Saunders Date: Thu, 5 Dec 2024 18:40:12 -0500 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=94=A8=20don't=20swallow=20error=20on?= =?UTF-8?q?=20image=20refresh?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteClient/ImagesIndexPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/adminSiteClient/ImagesIndexPage.tsx b/adminSiteClient/ImagesIndexPage.tsx index 0eeff9194b2..4f67354ece9 100644 --- a/adminSiteClient/ImagesIndexPage.tsx +++ b/adminSiteClient/ImagesIndexPage.tsx @@ -179,8 +179,8 @@ function ImgWithRefresh({ ref.current.src = src } }) - .catch(() => { - console.log("Something went wrong refreshing the image") + .catch((e) => { + console.log("Something went wrong refreshing the image", e) }) } })