From 9253472dc33481fb712597ae561563e55fec0b54 Mon Sep 17 00:00:00 2001 From: Daniel Bachler Date: Fri, 15 Mar 2024 12:11:17 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A8=20fix=20various=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- adminSiteServer/apiRouter.ts | 9 +++++---- adminSiteServer/mockSiteRouter.tsx | 5 +++-- db/db.ts | 14 +++++++++++++- db/model/Gdoc/GdocBase.ts | 13 +------------ db/model/Gdoc/GdocFactory.ts | 30 ++++++++++++++++++++++++++---- db/tests/basic.test.ts | 12 +++++++++++- 6 files changed, 59 insertions(+), 24 deletions(-) diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 4c960434016..9a4a7b85720 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -128,6 +128,7 @@ import { gdocFromJSON, getAllGdocIndexItemsOrderedByUpdatedAt, getAndLoadGdocById, + getDbEnrichedGdocFromOwidGdoc, getGdocBaseObjectById, loadGdocFromGdocBase, setTagsForGdoc, @@ -2463,7 +2464,7 @@ postRouteWithRWTransaction( gdoc.published = false gdoc.createdAt = new Date() gdoc.publishedAt = post.published_at - await upsertGdoc(trx, gdoc.toDbRawPostGdoc()) + await upsertGdoc(trx, gdoc) await setTagsForGdoc(trx, gdocId, tags) } return { googleDocsId: gdocId } @@ -2662,11 +2663,11 @@ putRouteWithRWTransaction(apiRouter, "/gdocs/:id", async (req, res, trx) => { // enqueue), so I opted for the version that matches the closest the current // baking model, which is "bake what is persisted in the DB". Ultimately, a // full sucessful deploy would resolve the state discrepancy either way. - await upsertGdoc(trx, nextGdoc.toDbRawPostGdoc()) + await upsertGdoc(trx, nextGdoc) const hasChanges = checkHasChanges(prevGdoc, nextGdoc) - const prevJson = prevGdoc.toDbRawPostGdoc() - const nextJson = nextGdoc.toDbRawPostGdoc() + const prevJson = getDbEnrichedGdocFromOwidGdoc(prevGdoc) + const nextJson = getDbEnrichedGdocFromOwidGdoc(nextGdoc) if (checkIsLightningUpdate(prevJson, nextJson, hasChanges)) { await enqueueLightningChange( res.locals.user, diff --git a/adminSiteServer/mockSiteRouter.tsx b/adminSiteServer/mockSiteRouter.tsx index 63866a16cba..242ec253586 100644 --- a/adminSiteServer/mockSiteRouter.tsx +++ b/adminSiteServer/mockSiteRouter.tsx @@ -392,13 +392,14 @@ mockSiteRouter.get("/*", async (req, res) => { await db.knexReadonlyTransaction(async (knex) => { try { - res.send(await renderGdocsPageBySlug(knex, slug)) + const page = await renderGdocsPageBySlug(knex, slug) + res.send(page) } catch (e) { console.error(e) } try { - const page = renderPageBySlug(slug, knex) + const page = await renderPageBySlug(slug, knex) res.send(page) } catch (e) { console.error(e) diff --git a/db/db.ts b/db/db.ts index 6c1211ff45b..4595eb0e434 100644 --- a/db/db.ts +++ b/db/db.ts @@ -144,7 +144,19 @@ export const knexRaw = async ( knex: Knex, str: string, params?: any[] | Record -): Promise => (await knex.raw(str, params ?? []))[0] +): Promise => { + try { + const rawReturnConstruct = await knex.raw(str, params ?? []) + return rawReturnConstruct[0] + } catch (e) { + console.error("Exception when executing SQL statement!", { + sql: str, + params, + error: e, + }) + throw e + } +} export const knexRawFirst = async ( knex: Knex, diff --git a/db/model/Gdoc/GdocBase.ts b/db/model/Gdoc/GdocBase.ts index 3e258eebb9d..f47c20116fa 100644 --- a/db/model/Gdoc/GdocBase.ts +++ b/db/model/Gdoc/GdocBase.ts @@ -823,18 +823,6 @@ export class GdocBase implements OwidGdocBaseInterface { await this._loadSubclassAttachments(knex) await this.validate(knex) } - - toDbRawPostGdoc(): DbEnrichedPostGdoc { - const readyToSerialize = omit(this, [ - "_enrichSubclassContent", - "_filenameProperties", - "_getSubclassEnrichedBlocks", - "_omittableFields", - "_validateSubclass", - ...this._omittableFields, - ]) as OwidGdocBaseInterface - return readyToSerialize - } } // This function would naturally live in GdocFactory but that would create a circular dependency @@ -842,6 +830,7 @@ export async function getMinimalGdocBaseObjectsByIds( knex: KnexReadonlyTransaction, ids: string[] ): Promise { + if (ids.length === 0) return [] const rows = await db.knexRaw<{ id: string title: string diff --git a/db/model/Gdoc/GdocFactory.ts b/db/model/Gdoc/GdocFactory.ts index 5875ae79dff..54a4e4471d0 100644 --- a/db/model/Gdoc/GdocFactory.ts +++ b/db/model/Gdoc/GdocFactory.ts @@ -1,4 +1,4 @@ -import { get, groupBy } from "lodash" +import { get, groupBy, omit } from "lodash" import { match, P } from "ts-pattern" import { DATA_INSIGHTS_INDEX_PAGE_SIZE, @@ -455,15 +455,37 @@ export async function setTagsForGdoc( .insert(tagIds.map(({ id: tagId }) => ({ gdocId, tagId }))) } +export function getDbEnrichedGdocFromOwidGdoc( + gdoc: OwidGdoc | GdocBase +): DbEnrichedPostGdoc { + const enrichedGdoc = { + breadcrumbs: gdoc.breadcrumbs, + content: gdoc.content, + createdAt: gdoc.createdAt, + id: gdoc.id, + markdown: gdoc.markdown, + publicationContext: gdoc.publicationContext, + published: gdoc.published, + publishedAt: gdoc.publishedAt, + revisionId: gdoc.revisionId, + slug: gdoc.slug, + updatedAt: gdoc.updatedAt, + } satisfies DbEnrichedPostGdoc + return enrichedGdoc +} export async function upsertGdoc( knex: KnexReadWriteTransaction, - gdoc: DbEnrichedPostGdoc + gdoc: OwidGdoc | GdocBase ): Promise { - return knex + const enrichedGdoc = getDbEnrichedGdocFromOwidGdoc(gdoc) + const rawPost = serializePostsGdocsRow(enrichedGdoc) + const query = knex .table(PostsGdocsTableName) - .insert(serializePostsGdocsRow(gdoc)) + .insert(rawPost) .onConflict("id") .merge() + console.error(query.toSQL()) + return query } // TODO: diff --git a/db/tests/basic.test.ts b/db/tests/basic.test.ts index c183cd8259c..4314d84cf48 100644 --- a/db/tests/basic.test.ts +++ b/db/tests/basic.test.ts @@ -156,7 +156,6 @@ test("knex interface", async () => { expect(afterUpdate.length).toBe(2) expect(afterUpdate[1].isSuperuser).toBe(1) - // Use raw queries, using ?? to specify the table name using the shared const value // The pick type is used to type the result row const usersFromRawQuery: Pick[] = await knexRaw( trx, @@ -164,6 +163,17 @@ test("knex interface", async () => { [] ) expect(usersFromRawQuery.length).toBe(2) + + // Check if in queries work as expected + const usersFromRawQueryWithInClause: Pick[] = + await knexRaw(trx, "select * from users where email in (:emails)", { + emails: [ + usersFromRawQuery[0].email, + usersFromRawQuery[1].email, + ], + }) + expect(usersFromRawQueryWithInClause.length).toBe(2) + await deleteUser(trx, 2) }, knexInstance) })