📜 mark todos everywhere we might want to revert the rw transaction ju…

…st for images

adminSiteServer/adminRouter.tsx

@@ -317,7 +317,7 @@ getPlainRouteWithROTransaction(
         return res.send(explorerPage)
     }
 )
-
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     adminRouter,
     "/datapage-preview/:id",
@@ -339,7 +339,7 @@ getPlainRouteNonIdempotentWithRWTransaction(
         )
     }
 )
-
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     adminRouter,
     "/grapher/:slug",

adminSiteServer/app.tsx

@@ -126,6 +126,7 @@ export class OwidAdminApp {
         // Public preview of a Gdoc document
         app.get("/gdocs/:id/preview", async (req, res) => {
             try {
+                // TODO: this transaction is only RW because somewhere inside it we fetch images
                 await db.knexReadWriteTransaction(async (knex) => {
                     const gdoc = await getAndLoadGdocById(
                         knex,

adminSiteServer/mockSiteRouter.tsx

@@ -71,6 +71,7 @@ const mockSiteRouter = Router()
 mockSiteRouter.use(express.urlencoded({ extended: true }))
 mockSiteRouter.use(express.json())
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/sitemap.xml",
@@ -81,6 +82,7 @@ getPlainRouteNonIdempotentWithRWTransaction(
     }
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/atom.xml",
@@ -91,6 +93,7 @@ getPlainRouteNonIdempotentWithRWTransaction(
     }
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/atom-no-topic-pages.xml",
@@ -191,6 +194,7 @@ mockSiteRouter.get("/collection/custom", async (_, res) => {
     return res.send(await renderDynamicCollectionPage())
 })
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/grapher/:slug",
@@ -207,6 +211,7 @@ getPlainRouteNonIdempotentWithRWTransaction(
     }
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/",
@@ -216,6 +221,7 @@ getPlainRouteNonIdempotentWithRWTransaction(
     }
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/donate",
@@ -226,6 +232,7 @@ mockSiteRouter.get("/thank-you", async (req, res) =>
     res.send(await renderThankYouPage())
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/data-insights/:pageNumberOrSlug?",
@@ -285,6 +292,7 @@ getPlainRouteWithROTransaction(
     }
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/datapage-preview/:id",
@@ -325,6 +333,7 @@ mockSiteRouter.get("/search", async (req, res) =>
     res.send(await renderSearchPage())
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/latest",
@@ -334,6 +343,7 @@ getPlainRouteNonIdempotentWithRWTransaction(
     }
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/latest/page/:pageno",
@@ -451,6 +461,7 @@ getPlainRouteWithROTransaction(
     }
 )
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 getPlainRouteNonIdempotentWithRWTransaction(
     mockSiteRouter,
     "/*",

baker/DatapageHelpers.ts

@@ -78,6 +78,7 @@ export const getDatapageDataV2 = async (
  * see https://github.com/owid/owid-grapher/issues/2121#issue-1676097164
  */
 export const getDatapageGdoc = async (
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     knex: KnexReadWriteTransaction,
     googleDocEditLinkOrId: string,
     isPreviewing: boolean

baker/DeployUtils.ts

@@ -33,6 +33,8 @@ export const defaultCommitMessage = async (): Promise<string> => {
 /**
  * Initiate a deploy, without any checks. Throws error on failure.
  */
+
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 const triggerBakeAndDeploy = async (
     deployMetadata: DeployMetadata,
     knex: KnexReadWriteTransaction,
@@ -160,6 +162,7 @@ let deploying = false
  * If there are no changes in the queue, a deploy won't be initiated.
  */
 export const deployIfQueueIsNotEmpty = async (
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     knex: KnexReadWriteTransaction
 ) => {
     if (deploying) return

baker/GrapherBaker.tsx

@@ -90,6 +90,8 @@ const renderDatapageIfApplicable = async (
  *
  * Render a datapage if available, otherwise render a grapher page.
  */
+
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export const renderDataPageOrGrapherPage = async (
     grapher: GrapherInterface,
     knex: db.KnexReadWriteTransaction,
@@ -133,6 +135,8 @@ export async function renderDataPageV2(
         pageGrapher?: GrapherInterface
         imageMetadataDictionary?: Record<string, ImageMetadata>
     },
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     knex: db.KnexReadWriteTransaction
 ) {
     const grapherConfigForVariable = await getMergedGrapherConfigForVariable(
@@ -174,7 +178,7 @@ export async function renderDataPageV2(
     )
     const imageMetadata: OwidGdocPostInterface["imageMetadata"] = merge(
         {},
-        // imageMetadataDictionary,
+        imageMetadataDictionary,
         ...compact(gdocs.map((gdoc) => gdoc?.imageMetadata))
     )
     const relatedCharts: OwidGdocPostInterface["relatedCharts"] = gdocs.flatMap(
@@ -317,6 +321,8 @@ export async function renderDataPageV2(
  */
 export const renderPreviewDataPageOrGrapherPage = async (
     grapher: GrapherInterface,
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     knex: db.KnexReadWriteTransaction
 ) => {
     const datapage = await renderDatapageIfApplicable(grapher, true, knex)
@@ -453,6 +459,8 @@ export interface BakeSingleGrapherChartArguments {
 
 export const bakeSingleGrapherChart = async (
     args: BakeSingleGrapherChartArguments,
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     knex: db.KnexReadWriteTransaction
 ) => {
     const grapher: GrapherInterface = JSON.parse(args.config)
@@ -475,6 +483,7 @@ export const bakeSingleGrapherChart = async (
 }
 
 export const bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers =
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     async (bakedSiteDir: string, knex: db.KnexReadWriteTransaction) => {
         const chartsToBake: { id: number; config: string; slug: string }[] =
             await knexRaw(

baker/SiteBaker.tsx

@@ -480,6 +480,8 @@ export class SiteBaker {
     }
 
     // Bake all GDoc posts, or a subset of them if slugs are provided
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     async bakeGDocPosts(knex: db.KnexReadWriteTransaction, slugs?: string[]) {
         if (!this.bakeSteps.has("gdocPosts")) return
         const publishedGdocs = await GdocPost.getPublishedGdocPosts(knex)
@@ -544,6 +546,8 @@ export class SiteBaker {
     }
 
     // Bake unique individual pages
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     private async bakeSpecialPages(knex: db.KnexReadWriteTransaction) {
         if (!this.bakeSteps.has("specialPages")) return
         await this.stageWrite(
@@ -700,6 +704,7 @@ export class SiteBaker {
         }
     }
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     private async bakeDataInsights(knex: db.KnexReadWriteTransaction) {
         if (!this.bakeSteps.has("dataInsights")) return
         const latestDataInsights = await db.getPublishedDataInsights(knex, 5)
@@ -768,6 +773,7 @@ export class SiteBaker {
         }
     }
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     private async bakeAuthors(knex: db.KnexReadWriteTransaction) {
         if (!this.bakeSteps.has("authors")) return
 
@@ -855,6 +861,8 @@ export class SiteBaker {
     }
 
     // Bake the blog index
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     private async bakeBlogIndex(knex: db.KnexReadWriteTransaction) {
         if (!this.bakeSteps.has("blogIndex")) return
         const allPosts = await getBlogIndex(knex)
@@ -869,6 +877,8 @@ export class SiteBaker {
     }
 
     // Bake the RSS feed
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     private async bakeRSS(knex: db.KnexReadWriteTransaction) {
         if (!this.bakeSteps.has("rss")) return
         await this.stageWrite(
@@ -954,6 +964,7 @@ export class SiteBaker {
         this.progressBar.tick({ name: "✅ baked redirects" })
     }
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     async bakeWordpressPages(knex: db.KnexReadWriteTransaction) {
         await this.bakeRedirects(knex)
         await this.bakeEmbeds(knex)
@@ -964,6 +975,7 @@ export class SiteBaker {
         await this.bakePosts(knex)
     }
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     private async _bakeNonWordpressPages(knex: db.KnexReadWriteTransaction) {
         if (this.bakeSteps.has("countries")) {
             await bakeCountries(this, knex)
@@ -988,6 +1000,7 @@ export class SiteBaker {
         await this.bakeDriveImages(knex)
     }
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     async bakeNonWordpressPages(knex: db.KnexReadWriteTransaction) {
         const progressBarTotal = nonWordpressSteps
             .map((step) => this.bakeSteps.has(step))
@@ -1001,6 +1014,7 @@ export class SiteBaker {
         await this._bakeNonWordpressPages(knex)
     }
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     async bakeAll(knex: db.KnexReadWriteTransaction) {
         // Ensure caches are correctly initialized
         this.flushCache()

baker/algolia/indexToAlgolia.tsx

@@ -194,6 +194,7 @@ function generateGdocRecords(
     return records
 }
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 // Generate records for countries, WP posts (not including posts that have been succeeded by Gdocs equivalents), and Gdocs
 const getPagesRecords = async (knex: db.KnexReadWriteTransaction) => {
     const pageviews = await getAnalyticsPageviewsByUrlObj(knex)
@@ -234,6 +235,7 @@ const indexToAlgolia = async () => {
     }
     const index = client.initIndex(getIndexName(SearchIndexName.Pages))
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     const records = await db.knexReadWriteTransaction(
         getPagesRecords,
         db.TransactionCloseMode.Close

baker/bakeGdocPost.ts

@@ -19,6 +19,7 @@ void yargs(hideBin(process.argv))
         async ({ slug }) => {
             const baker = new SiteBaker(BAKED_SITE_DIR, BAKED_BASE_URL)
 
+            // TODO: this transaction is only RW because somewhere inside it we fetch images
             await db.knexReadWriteTransaction(
                 (trx) => baker.bakeGDocPosts(trx, [slug]),
                 db.TransactionCloseMode.Close

baker/bakeGdocPosts.ts

@@ -24,6 +24,7 @@ void yargs(hideBin(process.argv))
         async ({ slugs }) => {
             const baker = new SiteBaker(BAKED_SITE_DIR, BAKED_BASE_URL)
 
+            // TODO: this transaction is only RW because somewhere inside it we fetch images
             await db.knexReadWriteTransaction(
                 (trx) => baker.bakeGDocPosts(trx, slugs),
                 db.TransactionCloseMode.Close

baker/buildLocalBake.ts

@@ -17,6 +17,8 @@ const bakeDomainToFolder = async (
     await fs.mkdirp(dir)
     const baker = new SiteBaker(dir, baseUrl, bakeSteps)
     console.log(`Baking site locally with baseUrl '${baseUrl}' to dir '${dir}'`)
+
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     await db.knexReadWriteTransaction(
         (trx) => baker.bakeAll(trx),
         db.TransactionCloseMode.Close

baker/runBakeGraphers.ts

@@ -9,6 +9,7 @@ import * as db from "../db/db.js"
  */
 
 const main = async (folder: string) => {
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     return db.knexReadWriteTransaction(
         (trx) =>
             bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers(

baker/siteRenderers.tsx

@@ -189,6 +189,7 @@ export function renderDynamicCollectionPage() {
     return renderToHtmlPage(<DynamicCollectionPage baseUrl={BAKED_BASE_URL} />)
 }
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export const renderGdocsPageBySlug = async (
     knex: KnexReadWriteTransaction,
     slug: string,
@@ -279,6 +280,7 @@ export const renderPost = async (
     )
 }
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export const renderFrontPage = async (knex: KnexReadWriteTransaction) => {
     const gdocHomepageId = await getHomepageId(knex)
 
@@ -296,6 +298,7 @@ export const renderFrontPage = async (knex: KnexReadWriteTransaction) => {
     }
 }
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export const renderDonatePage = async (knex: KnexReadWriteTransaction) => {
     const faqsGdoc = (await getAndLoadGdocById(
         knex,
@@ -336,6 +339,7 @@ export const renderDataInsightsIndexPage = (
     )
 }
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export const renderBlogByPageNum = async (
     pageNum: number,
     knex: KnexReadWriteTransaction
@@ -364,6 +368,7 @@ export const renderSearchPage = () =>
 export const renderNotFoundPage = () =>
     renderToHtmlPage(<NotFoundPage baseUrl={BAKED_BASE_URL} />)
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export async function makeAtomFeed(knex: KnexReadWriteTransaction) {
     const posts = (await getBlogIndex(knex)).slice(0, 10)
     return makeAtomFeedFromPosts(posts)
@@ -372,6 +377,8 @@ export async function makeAtomFeed(knex: KnexReadWriteTransaction) {
 // We don't want to include topic pages in the atom feed that is being consumed
 // by Mailchimp for sending the "immediate update" newsletter. Instead topic
 // pages announcements are sent out manually.
+
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export async function makeAtomFeedNoTopicPages(knex: KnexReadWriteTransaction) {
     const posts = (await getBlogIndex(knex))
         .filter((post: IndexPost) => post.type !== OwidGdocType.TopicPage)

baker/sitemap.ts

@@ -62,6 +62,7 @@ const explorerToSitemapUrl = (program: ExplorerProgram): SitemapUrl[] => {
     }
 }
 
+// TODO: this transaction is only RW because somewhere inside it we fetch images
 export const makeSitemap = async (
     explorerAdminServer: ExplorerAdminServer,
     knex: db.KnexReadWriteTransaction

baker/startDeployQueueServer.ts

@@ -33,6 +33,7 @@ const main = async () => {
         setTimeout(deployIfQueueIsNotEmpty, 10 * 1000)
     })
 
+    // TODO: this transaction is only RW because somewhere inside it we fetch images
     await db.knexReadWriteTransaction(
         deployIfQueueIsNotEmpty,
         db.TransactionCloseMode.Close