Skip to content

Commit

Permalink
✨ PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
ikesau committed Nov 29, 2024
1 parent 956cf17 commit 95bfa84
Show file tree
Hide file tree
Showing 10 changed files with 167 additions and 30 deletions.
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,14 @@ refresh.pageviews: node_modules
@echo '==> Refreshing pageviews'
yarn refreshPageviews

sync-images:
@echo 'Task has been deprecated.'

bake-images:
@echo 'Task has been deprecated.'

# Only needed to run once to seed the prod DB with initially
sync-cloudflare-images:
sync-cloudflare-images: node_modules
@echo '==> Syncing images table with Cloudflare Images'
@yarn syncCloudflareImages

Expand Down
2 changes: 1 addition & 1 deletion adminSiteClient/GdocsMoreMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ function DeleteModal({
<Form.Item
name="relatedLinkThumbnail"
label="Thumbnail file name"
extra="The image must be already be uploaded in the admin."
extra="The image must already be uploaded in the admin."
>
<Input type="text" />
</Form.Item>
Expand Down
21 changes: 13 additions & 8 deletions adminSiteClient/ImagesIndexPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React, {
useMemo,
useState,
} from "react"
import { Button, Flex, Input, Space, Table, Upload } from "antd"
import { Button, Flex, Input, Popconfirm, Space, Table, Upload } from "antd"

import { AdminLayout } from "./AdminLayout.js"
import { AdminAppContext } from "./AdminAppContext.js"
Expand All @@ -17,6 +17,7 @@ import { 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"

type ImageEditorApi = {
patchImage: (
Expand Down Expand Up @@ -71,7 +72,7 @@ function createColumns({
key: "cloudflareId",
render: (cloudflareId, { originalWidth }) => {
const srcFor = (w: number) =>
`https://imagedelivery.net/qLq-8BTgXU8yG0N6HnOy8g/${encodeURIComponent(cloudflareId)}/w=${w}`
`${CLOUDFLARE_IMAGES_URL}/${encodeURIComponent(cloudflareId)}/w=${w}`
return (
<div style={{ height: 100, width: 100 }} key={cloudflareId}>
<a
Expand Down Expand Up @@ -147,13 +148,17 @@ function createColumns({
width: 100,
render: (_, image) => (
<Space size="middle">
<Button
type="text"
danger
onClick={() => api.deleteImage(image)}
<Popconfirm
title="Are you sure?"
description="This will delete the image being used in production."
onConfirm={() => api.deleteImage(image)}
okText="Yes"
cancelText="No"
>
Delete
</Button>
<Button type="text" danger>
Delete
</Button>
</Popconfirm>
</Space>
),
},
Expand Down
9 changes: 6 additions & 3 deletions adminSiteServer/apiRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2999,10 +2999,13 @@ deleteRouteWithRWTransaction(apiRouter, "/gdocs/:id", async (req, res, trx) => {
const slug = gdocSlug.replace("/", "")
const { relatedLinkThumbnail } = tombstone
if (relatedLinkThumbnail) {
const images = await db.getCloudflareImages(trx)
if (!images.find((i) => i.filename === relatedLinkThumbnail)) {
const thumbnailExists = await db.checkIsImageInDB(
trx,
relatedLinkThumbnail
)
if (!thumbnailExists) {
throw new JsonError(
`Image "${relatedLinkThumbnail}" doesn't exist in the database`
`Image with filename "${relatedLinkThumbnail}" not found`
)
}
}
Expand Down
8 changes: 8 additions & 0 deletions db/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,14 @@ export const getHomepageId = (
).then((result) => result?.id)
}

export async function checkIsImageInDB(
trx: KnexReadonlyTransaction,
filename: string
): Promise<boolean> {
const image = await trx("images").where("filename", filename).first()
return !!image
}

export const getImageMetadataByFilenames = async (
knex: KnexReadonlyTransaction,
filenames: string[]
Expand Down
9 changes: 9 additions & 0 deletions db/migration/1732833869225-CloudflareImagesAuthors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { MigrationInterface, QueryRunner } from "typeorm"

export class CloudflareImagesAuthors1732833869225
implements MigrationInterface
{
public async up(queryRunner: QueryRunner): Promise<void> {}

Check warning on line 6 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

'queryRunner' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 6 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected empty async method 'up'

Check warning on line 6 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

'queryRunner' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 6 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected empty async method 'up'

public async down(queryRunner: QueryRunner): Promise<void> {}

Check warning on line 8 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

'queryRunner' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 8 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected empty async method 'down'

Check warning on line 8 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

'queryRunner' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 8 in db/migration/1732833869225-CloudflareImagesAuthors.ts

View workflow job for this annotation

GitHub Actions / eslint

Unexpected empty async method 'down'
}
21 changes: 7 additions & 14 deletions devTools/cloudflareImagesSync/cloudflareImagesSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ async function uploadImageToCloudflareImages(
async function getCloudflareImageDirectory() {
console.log("Fetching Cloudflare Images directory...")
const directory = await fetch(
`https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_IMAGES_ACCOUNT_ID}/images/v1?per_page=2000`,
`https://api.cloudflare.com/client/v4/accounts/${CLOUDFLARE_IMAGES_ACCOUNT_ID}/images/v1?per_page=10000`,
{
headers: {
Authorization: `Bearer ${CLOUDFLARE_IMAGES_API_KEY}`,
Expand All @@ -367,19 +367,12 @@ async function getCloudflareImageDirectory() {

async function fetchImagesFromDatabase(trx: db.KnexReadWriteTransaction) {
console.log("Fetching images from the database...")
return await trx
.raw<DbEnrichedImage[]>(
`-- sql
SELECT * FROM images WHERE id IN (
SELECT DISTINCT imageId FROM posts_gdocs_x_images
)`
)
.then((res) => res.flat())
.then(excludeNullish)
.then((images) => images.filter((image) => image && image.filename))
.then((images) =>
images.sort((a, b) => a.filename.localeCompare(b.filename))
)
return await trx("images")
.select("images.*")
.whereIn("images.id", trx("posts_gdocs_x_images").distinct("imageId"))
.whereNotNull("images.id")
.whereNotNull("images.filename")
.orderBy("filename", "asc")
}

async function uploadImagesToCloudflareImages(
Expand Down
114 changes: 114 additions & 0 deletions devTools/cloudflareImagesSync/invalidImages.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
[
{
"filename": "2019-Revision-–-World-Population-Growth-1700-2100.png",
"reason": "InvalidDimensions"
},
{
"filename": "Annual-World-Population-since-10-thousand-BCE-1.png",
"reason": "TooManyMegapixels"
},
{
"filename": "By-continent-and-decade-01.png",
"reason": "InvalidDimensions"
},
{
"filename": "cable-tv-access-and-preference-for-a-son.png",
"reason": "InvalidDimensions"
},
{
"filename": "calculating-risk-ratios-differences-odds.png",
"reason": "InvalidDimensions"
},
{
"filename": "cancer-death-rate-decline-who-mdb-desktop.png",
"reason": "InvalidFormat"
},
{
"filename": "cancer-death-rate-decline-who-mdb-mobile.png",
"reason": "InvalidFormat"
},
{
"filename": "cardiovascular-diseases-types.png",
"reason": "InvalidDimensions"
},
{
"filename": "causes-of-death-2019-full.png",
"reason": "InvalidDimensions"
},
{
"filename": "Decade-in-which-smallpox-ceased-to-be-endemic-map.png",
"reason": "InvalidDimensions"
},
{
"filename": "Energy-Units-01.png",
"reason": "InvalidDimensions"
},
{
"filename": "England-death-rates.png",
"reason": "InvalidDimensions"
},
{
"filename": "Famine-victims-since-1860s_March18.png",
"reason": "InvalidDimensions"
},
{
"filename": "FEATURED-IMAGE-World-Population-Growth.png",
"reason": "InvalidDimensions"
},
{
"filename": "Female-to-male-wage-ratio-01.png",
"reason": "InvalidDimensions"
},
{
"filename": "future-yields-climate-distribution.png",
"reason": "InvalidDimensions"
},
{
"filename": "Gender-Pay-Gap-01.png",
"reason": "InvalidDimensions"
},
{
"filename": "Global-land-use-breakdown.png",
"reason": "InvalidDimensions"
},
{
"filename": "Global-land-use-graphic.png",
"reason": "InvalidDimensions"
},
{
"filename": "height-distribution.png",
"reason": "InvalidDimensions"
},
{
"filename": "Multiple-risk-factors-full-1.png",
"reason": "TooManyMegapixels"
},
{
"filename": "Norway-death-rates.png",
"reason": "InvalidDimensions"
},
{
"filename": "not-reading-with-comprehension.png",
"reason": "InvalidDimensions"
},
{
"filename": "Pandemics-Timeline-Death-Tolls-OWID.png",
"reason": "InvalidDimensions"
},
{
"filename": "period-vs-cohort-explanation.png",
"reason": "TooManyMegapixels"
},
{
"filename": "proportion-dying-latex.svg",
"reason": "InvalidFormat"
},
{
"filename": "record-female-life-expectancy-since-1840-updated-2023.png",
"reason": "InvalidDimensions"
},
{
"filename": "the-history-of-global-inequality-featured-image.png",
"reason": "InvalidDimensions"
}
]
2 changes: 1 addition & 1 deletion site/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ filename: my_image.png

where `my_image.png` is an image that has been uploaded via the `/admin/images` view in the admin client, and thus exists in Cloudflare Images.

We store information about the image's dimensions and alt text in the database, which is shared via React content to any component that needs to render them. See `Image.tsx` for the (many) implementation details.
We store information about the image's dimensions and alt text in the database, which is shared via React context to any component that needs to render them. See `Image.tsx` for the (many) implementation details.

## Data Catalog

Expand Down
3 changes: 1 addition & 2 deletions site/gdocs/components/Image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ export default function Image(props: {
shouldLightbox = true,
} = props

const className = cx(props.className, {
image: true,
const className = cx("image", props.className, {
"image--has-outline": hasOutline,
})

Expand Down

0 comments on commit 95bfa84

Please sign in to comment.