diff --git a/adminSiteClient/SuggestedChartRevision.ts b/adminSiteClient/SuggestedChartRevision.ts deleted file mode 100644 index 5ba4e4ca7c5..00000000000 --- a/adminSiteClient/SuggestedChartRevision.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { GrapherInterface } from "@ourworldindata/types" -import { SuggestedChartRevisionStatus } from "@ourworldindata/utils" - -export interface SuggestedChartRevisionSerialized { - id: number - chartId: number - - createdAt: string - updatedAt?: string - - chartCreatedAt: string - chartUpdatedAt?: string - - createdById: number - updatedById?: number - - createdByFullName: string - updatedByFullName?: string - - originalConfig: GrapherInterface - suggestedConfig: GrapherInterface - existingConfig: GrapherInterface - - status: SuggestedChartRevisionStatus - suggestedReason?: string - decisionReason?: string - changesInDataSummary?: string - - canApprove?: boolean - canReject?: boolean - canFlag?: boolean - canPending?: boolean - - experimental?: SuggestedChartRevisionExperimentalSerialized -} - -export interface SuggestedChartRevisionExperimentalSerialized { - gpt?: { - model?: string - suggestions?: GPTSuggestionsSerialized[] - } - [key: string]: any -} - -export interface GPTSuggestionsSerialized { - title?: string - subtitle?: string -} diff --git a/adminSiteClient/SuggestedChartRevisionApproverPage.tsx b/adminSiteClient/SuggestedChartRevisionApproverPage.tsx index 1c09b81d285..0994e74ec69 100644 --- a/adminSiteClient/SuggestedChartRevisionApproverPage.tsx +++ b/adminSiteClient/SuggestedChartRevisionApproverPage.tsx @@ -12,6 +12,7 @@ import { Tippy, uniqBy, } from "@ourworldindata/utils" +import { SuggestedChartRevision } from "@ourworldindata/types" import { Grapher } from "@ourworldindata/grapher" import { TextAreaField, @@ -44,7 +45,6 @@ import { VisionDeficiencyDropdown, VisionDeficiencyEntity, } from "./VisionDeficiencies.js" -import { SuggestedChartRevisionSerialized } from "./SuggestedChartRevision.js" import { match } from "ts-pattern" import { ReferencesSection } from "./EditorReferencesTab.js" @@ -57,7 +57,7 @@ interface UserSelectOption { export class SuggestedChartRevisionApproverPage extends React.Component<{ suggestedChartRevisionId?: number }> { - @observable.ref suggestedChartRevisions?: SuggestedChartRevisionSerialized[] + @observable.ref suggestedChartRevisions?: SuggestedChartRevision[] @observable currentlyActiveUserId?: number @observable.ref originalGrapherElement?: JSX.Element @observable.ref suggestedGrapherElement?: JSX.Element @@ -205,7 +205,7 @@ export class SuggestedChartRevisionApproverPage extends React.Component<{ console.log("fetchGraphers 2") runInAction(() => { this.suggestedChartRevisions = - json.suggestedChartRevisions as SuggestedChartRevisionSerialized[] + json.suggestedChartRevisions as SuggestedChartRevision[] }) console.log("fetchGraphers 3") this.decisionReasonInput = this.currentSuggestedChartRevision @@ -289,9 +289,8 @@ export class SuggestedChartRevisionApproverPage extends React.Component<{ if (this.currentSuggestedChartRevision) { // Get suggestions const suggestions = - this.currentSuggestedChartRevision?.experimental?.["gpt"]?.[ - "suggestions" - ] + this.currentSuggestedChartRevision?.experimental?.gpt + ?.suggestions if (suggestions !== undefined) { // Set title const title = suggestions?.[this.gptNum]?.["title"] @@ -498,7 +497,7 @@ export class SuggestedChartRevisionApproverPage extends React.Component<{ @computed get availableUsers(): UserSelectOption[] { const availableUserAccordingToRevisions = this.suggestedChartRevisions?.map((revision) => ({ - userId: revision.createdById, + userId: revision.createdBy, userName: revision.createdByFullName, })) ?? [] @@ -518,7 +517,7 @@ export class SuggestedChartRevisionApproverPage extends React.Component<{ if (this.currentlyActiveUserId === undefined) return this.suggestedChartRevisions return this.suggestedChartRevisions?.filter( - (revision) => revision.createdById === this.currentlyActiveUserId + (revision) => revision.createdBy === this.currentlyActiveUserId ) } diff --git a/adminSiteServer/adminRouter.tsx b/adminSiteServer/adminRouter.tsx index b144559abd8..c2b5941f3fa 100644 --- a/adminSiteServer/adminRouter.tsx +++ b/adminSiteServer/adminRouter.tsx @@ -148,7 +148,7 @@ adminRouter.get("/datasets/:datasetId.csv", async (req, res) => { callback(null) }, }) - await Dataset.writeCSV(datasetId, writeStream) + await Dataset.writeCSV(datasetId, writeStream, db.knexInstance()) res.end() }) @@ -167,13 +167,13 @@ adminRouter.get("/datasets/:datasetId/downloadZip", async (req, res) => { adminRouter.get("/posts/preview/:postId", async (req, res) => { const postId = expectInt(req.params.postId) - res.send(await renderPreview(postId)) + res.send(await renderPreview(postId, db.knexInstance())) }) adminRouter.get("/posts/compare/:postId", async (req, res) => { const postId = expectInt(req.params.postId) - const wpPage = await renderPreview(postId) + const wpPage = await renderPreview(postId, db.knexInstance()) const archieMlText = await Post.select( "archieml", "archieml_update_statistics" @@ -279,13 +279,16 @@ adminRouter.get("/datapage-preview/:id", async (req, res) => { await explorerAdminServer.getAllPublishedExplorersBySlugCached() res.send( - await renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing: true, - useIndicatorGrapherConfigs: true, - publishedExplorersBySlug, - }) + await renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing: true, + useIndicatorGrapherConfigs: true, + publishedExplorersBySlug, + }, + db.knexInstance() + ) ) }) @@ -300,6 +303,7 @@ adminRouter.get("/grapher/:slug", async (req, res) => { res.send( await renderPreviewDataPageOrGrapherPage( entity.config, + db.knexInstance(), publishedExplorersBySlug ) ) diff --git a/adminSiteServer/apiRouter.ts b/adminSiteServer/apiRouter.ts index 64789f6c2cb..4dbec1d58b5 100644 --- a/adminSiteServer/apiRouter.ts +++ b/adminSiteServer/apiRouter.ts @@ -49,11 +49,20 @@ import { TaggableType, DbChartTagJoin, OwidGdoc, + pick, } from "@ourworldindata/utils" import { + ChartRevisionsRowForInsert, GrapherInterface, OwidGdocLinkType, + SuggestedChartRevision, + SuggestedChartRevisionsRowForInsert, + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw, + DbPlainUser, + UsersTableName, grapherKeysToSerialize, + parseSuggestedChartRevisionsRowWithUsersAndExistingConfig, } from "@ourworldindata/types" import { getVariableDataRoute, @@ -64,7 +73,7 @@ import { CountryDefByKey, } from "../adminSiteClient/CountryNameFormat.js" import { Dataset } from "../db/model/Dataset.js" -import { User } from "../db/model/User.js" +import { getUserById, insertUser, updateUser } from "../db/model/User.js" import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { GdocBase, Tag as TagEntity } from "../db/model/Gdoc/GdocBase.js" import { Pageview } from "../db/model/Pageview.js" @@ -73,7 +82,18 @@ import { removeDatasetFromGitRepo, } from "./gitDataExport.js" import { ChartRevision } from "../db/model/ChartRevision.js" -import { SuggestedChartRevision } from "../db/model/SuggestedChartRevision.js" +import { + checkCanApprove, + checkCanFlag, + checkCanPending, + checkCanReject, + getAllSuggestedChartRevisionsWithStatus, + getConfigFromChartsOrChartRevisionsPrioritized, + getNumSuggestedChartRevisionsWithStatus, + getReducedSuggestedChartRevisionById, + getSuggestedChartRevisionsById, + isValidStatus, +} from "../db/model/SuggestedChartRevision.js" import { denormalizeLatestCountryData } from "../baker/countryProfiles.js" import { ChartRedirect, References } from "../adminSiteClient/ChartEditor.js" import { DeployQueueServer } from "../baker/DeployQueueServer.js" @@ -98,6 +118,7 @@ import { In } from "typeorm" import { GIT_CMS_DIR } from "../gitCms/GitCmsConstants.js" import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js" import { GdocFactory } from "../db/model/Gdoc/GdocFactory.js" +import { Knex } from "knex" const apiRouter = new FunctionalRouter() const explorerAdminServer = new ExplorerAdminServer(GIT_CMS_DIR) @@ -287,7 +308,7 @@ const expectChartById = async (chartId: any): Promise => { } const saveGrapher = async ( - transactionContext: db.TransactionContext, + transactionContext: Knex, user: CurrentUser, newConfig: GrapherInterface, existingConfig?: GrapherInterface, @@ -296,7 +317,7 @@ const saveGrapher = async ( ) => { // Slugs need some special logic to ensure public urls remain consistent whenever possible async function isSlugUsedInRedirect() { - const rows = await transactionContext.query( + const rows = await transactionContext.raw( `SELECT * FROM chart_slug_redirects WHERE chart_id != ? AND slug = ?`, // -1 is a placeholder ID that will never exist; but we cannot use NULL because // in that case we would always get back an empty resultset @@ -306,7 +327,7 @@ const saveGrapher = async ( } async function isSlugUsedInOtherGrapher() { - const rows = await transactionContext.query( + const rows = await transactionContext.raw( `SELECT id FROM charts WHERE id != ? AND config->>"$.isPublished" = "true" AND JSON_EXTRACT(config, "$.slug") = ?`, // -1 is a placeholder ID that will never exist; but we cannot use NULL because // in that case we would always get back an empty resultset @@ -333,11 +354,11 @@ const saveGrapher = async ( existingConfig.slug !== newConfig.slug ) { // Changing slug of an existing chart, delete any old redirect and create new one - await transactionContext.execute( + await transactionContext.raw( `DELETE FROM chart_slug_redirects WHERE chart_id = ? AND slug = ?`, [existingConfig.id, existingConfig.slug] ) - await transactionContext.execute( + await transactionContext.raw( `INSERT INTO chart_slug_redirects (chart_id, slug) VALUES (?, ?)`, [existingConfig.id, existingConfig.slug] ) @@ -359,12 +380,12 @@ const saveGrapher = async ( const newJsonConfig = JSON.stringify(newConfig) // todo: drop "isExplorable" if (existingConfig) - await transactionContext.query( + await transactionContext.raw( `UPDATE charts SET config=?, updatedAt=?, lastEditedAt=?, lastEditedByUserId=?, isExplorable=? WHERE id = ?`, [newJsonConfig, now, now, user.id, false, chartId] ) else { - const result = await transactionContext.execute( + const result = await transactionContext.raw( `INSERT INTO charts (config, createdAt, updatedAt, lastEditedAt, lastEditedByUserId, isExplorable) VALUES (?)`, [[newJsonConfig, now, now, now, user.id, false]] ) @@ -372,25 +393,25 @@ const saveGrapher = async ( } // Record this change in version history - const log = new ChartRevision() - log.chartId = chartId as number - log.userId = user.id - log.config = newConfig - // TODO: the orm needs to support this but it does not :( - log.createdAt = new Date() - log.updatedAt = new Date() - await transactionContext.manager.save(log) + const log = { + chartId: chartId as number, + userId: user.id, + config: JSON.stringify(newConfig), + } satisfies ChartRevisionsRowForInsert + transactionContext + .table("chart_revisions") + .insert(log) // Remove any old dimensions and store the new ones // We only note that a relationship exists between the chart and variable in the database; the actual dimension configuration is left to the json - await transactionContext.execute( + await transactionContext.raw( `DELETE FROM chart_dimensions WHERE chartId=?`, [chartId] ) const newDimensions = newConfig.dimensions ?? [] for (const [i, dim] of newDimensions.entries()) { - await transactionContext.execute( + await transactionContext.raw( `INSERT INTO chart_dimensions (chartId, variableId, property, \`order\`) VALUES (?)`, [[chartId, dim.variableId, dim.property, i]] ) @@ -407,7 +428,7 @@ const saveGrapher = async ( (!existingConfig || !existingConfig.isPublished) ) { // Newly published, set publication info - await transactionContext.execute( + await transactionContext.raw( `UPDATE charts SET publishedAt=?, publishedByUserId=? WHERE id = ? `, [now, user.id, chartId] ) @@ -418,7 +439,7 @@ const saveGrapher = async ( existingConfig.isPublished ) { // Unpublishing chart, delete any existing redirects to it - await transactionContext.execute( + await transactionContext.raw( `DELETE FROM chart_slug_redirects WHERE chart_id = ?`, [existingConfig.id] ) @@ -829,11 +850,7 @@ apiRouter.get( const sortOrder = isValidSortOrder(req.query.sortOrder as string) ? (req.query.sortOrder as string).toUpperCase() : "DESC" - const status = SuggestedChartRevision.isValidStatus( - req.query.status as SuggestedChartRevisionStatus - ) - ? req.query.status - : null + const status = isValidStatus(req.query.status) ? req.query.status : null let orderBy if (sortBy === "variableId") { @@ -847,74 +864,33 @@ apiRouter.get( orderBy = `scr.${sortBy}` } - const suggestedChartRevisions = await db.queryMysql( - `-- sql - SELECT scr.id, scr.chartId, scr.updatedAt, scr.createdAt, - scr.suggestedReason, scr.decisionReason, scr.status, - scr.suggestedConfig, scr.originalConfig, scr.changesInDataSummary, - scr.experimental, - createdByUser.id as createdById, - updatedByUser.id as updatedById, - createdByUser.fullName as createdByFullName, - updatedByUser.fullName as updatedByFullName, - c.config as existingConfig, c.updatedAt as chartUpdatedAt, - c.createdAt as chartCreatedAt - FROM suggested_chart_revisions as scr - LEFT JOIN charts c on c.id = scr.chartId - LEFT JOIN users createdByUser on createdByUser.id = scr.createdBy - LEFT JOIN users updatedByUser on updatedByUser.id = scr.updatedBy - ${status ? "WHERE scr.status = ?" : ""} - ORDER BY ${orderBy} ${sortOrder} - LIMIT ? OFFSET ? - `, - status ? [status, limit, offset] : [limit, offset] - ) - - let numTotalRows = ( - await db.queryMysql( - ` - SELECT COUNT(*) as count - FROM suggested_chart_revisions - ${status ? "WHERE status = ?" : ""} - `, - status ? [status] : [] + const suggestedChartRevisions: SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched[] = + await getAllSuggestedChartRevisionsWithStatus( + db.knexInstance(), + limit, + offset, + orderBy, + sortOrder, + status ) - )[0].count - numTotalRows = numTotalRows ? parseInt(numTotalRows) : numTotalRows - suggestedChartRevisions.map( - (suggestedChartRevision: SuggestedChartRevision) => { - suggestedChartRevision.suggestedConfig = JSON.parse( - suggestedChartRevision.suggestedConfig - ) - suggestedChartRevision.existingConfig = JSON.parse( - suggestedChartRevision.existingConfig - ) - suggestedChartRevision.originalConfig = JSON.parse( - suggestedChartRevision.originalConfig - ) - suggestedChartRevision.experimental = JSON.parse( - suggestedChartRevision.experimental - ) - suggestedChartRevision.canApprove = - SuggestedChartRevision.checkCanApprove( - suggestedChartRevision - ) - suggestedChartRevision.canReject = - SuggestedChartRevision.checkCanReject( - suggestedChartRevision - ) - suggestedChartRevision.canFlag = - SuggestedChartRevision.checkCanFlag(suggestedChartRevision) - suggestedChartRevision.canPending = - SuggestedChartRevision.checkCanPending( - suggestedChartRevision - ) - } + const numTotalRows = await getNumSuggestedChartRevisionsWithStatus( + db.knexInstance(), + status + ) + + const parsed = suggestedChartRevisions.map( + (suggestedChartRevision): SuggestedChartRevision => ({ + ...suggestedChartRevision, + canApprove: checkCanApprove(suggestedChartRevision), + canReject: checkCanReject(suggestedChartRevision), + canFlag: checkCanFlag(suggestedChartRevision), + canPending: checkCanPending(suggestedChartRevision), + }) ) return { - suggestedChartRevisions: suggestedChartRevisions, + suggestedChartRevisions: parsed, numTotalRows: numTotalRows, } } @@ -1044,7 +1020,7 @@ apiRouter.post( } }) - await db.transaction(async (t) => { + await db.knexInstance().transaction(async (t) => { const whereCond1 = suggestedConfigs .map( (config) => @@ -1062,29 +1038,19 @@ apiRouter.post( ) .join(" OR ") // retrieves original chart configs - let rows: any[] = await t.query( - ` - SELECT id, config, 1 as priority - FROM charts - WHERE ${whereCond1} - - UNION - - SELECT chartId as id, config, 2 as priority - FROM chart_revisions - WHERE ${whereCond2} - - ORDER BY priority - ` + const rawRows: { + id: number + config: GrapherInterface + priority: number + }[] = await getConfigFromChartsOrChartRevisionsPrioritized( + t, + whereCond1, + whereCond2 ) - rows.map((row) => { - row.config = JSON.parse(row.config) - }) - // drops duplicate id-version rows (keeping the row from the // `charts` table when available). - rows = rows.filter( + const filteredRows = rawRows.filter( (v, i, a) => a.findIndex( (el) => @@ -1092,11 +1058,11 @@ apiRouter.post( el.config.version === v.config.version ) === i ) - if (rows.length < suggestedConfigs.length) { + if (filteredRows.length < suggestedConfigs.length) { // identifies which particular chartId-version combinations have // not been found in the DB const missingConfigs = suggestedConfigs.filter((config) => { - const i = rows.findIndex((row) => { + const i = filteredRows.findIndex((row) => { return ( row.id === config.id && row.config.version === config.version @@ -1116,13 +1082,13 @@ apiRouter.post( "\n" )}\nPlease check that each chartId and version exists.` ) - } else if (rows.length > suggestedConfigs.length) { + } else if (filteredRows.length > suggestedConfigs.length) { throw new JsonError( "Retrieved more chart configs than expected. This may be due to a bug on the server." ) } const originalConfigs: Record = - rows.reduce( + filteredRows.reduce( (obj: any, row: any) => ({ ...obj, [row.id]: row.config, @@ -1199,7 +1165,7 @@ apiRouter.post( }) // inserts suggested chart revisions - const result = await t.execute( + const result = await t.raw( ` INSERT INTO suggested_chart_revisions (chartId, suggestedConfig, originalConfig, suggestedReason, changesInDataSummary, status, createdBy, createdAt, updatedAt) @@ -1235,26 +1201,10 @@ apiRouter.get( req.params.suggestedChartRevisionId ) - const suggestedChartRevision = await db.mysqlFirst( - `-- sql - SELECT scr.id, scr.chartId, scr.updatedAt, scr.createdAt, - scr.suggestedReason, scr.decisionReason, scr.status, - scr.suggestedConfig, scr.changesInDataSummary, scr.originalConfig, - createdByUser.id as createdById, - updatedByUser.id as updatedById, - createdByUser.fullName as createdByFullName, - updatedByUser.fullName as updatedByFullName, - c.config as existingConfig, c.updatedAt as chartUpdatedAt, - c.createdAt as chartCreatedAt - FROM suggested_chart_revisions as scr - LEFT JOIN charts c on c.id = scr.chartId - LEFT JOIN users createdByUser on createdByUser.id = scr.createdBy - LEFT JOIN users updatedByUser on updatedByUser.id = scr.updatedBy - WHERE scr.id = ? - `, - [suggestedChartRevisionId] + const suggestedChartRevision = await getSuggestedChartRevisionsById( + db.knexInstance(), + suggestedChartRevisionId ) - if (!suggestedChartRevision) { throw new JsonError( `No suggested chart revision by id '${suggestedChartRevisionId}'`, @@ -1262,27 +1212,14 @@ apiRouter.get( ) } - suggestedChartRevision.suggestedConfig = JSON.parse( - suggestedChartRevision.suggestedConfig - ) - suggestedChartRevision.originalConfig = JSON.parse( - suggestedChartRevision.originalConfig - ) - suggestedChartRevision.existingConfig = JSON.parse( - suggestedChartRevision.existingConfig - ) - suggestedChartRevision.canApprove = - SuggestedChartRevision.checkCanApprove(suggestedChartRevision) - suggestedChartRevision.canReject = - SuggestedChartRevision.checkCanReject(suggestedChartRevision) - suggestedChartRevision.canFlag = SuggestedChartRevision.checkCanFlag( - suggestedChartRevision - ) - suggestedChartRevision.canPending = - SuggestedChartRevision.checkCanPending(suggestedChartRevision) - return { - suggestedChartRevision: suggestedChartRevision, + suggestedChartRevision: { + ...suggestedChartRevision, + canApprove: checkCanApprove(suggestedChartRevision), + canReject: checkCanReject(suggestedChartRevision), + canFlag: checkCanFlag(suggestedChartRevision), + canPending: checkCanPending(suggestedChartRevision), + } satisfies SuggestedChartRevision, } } ) @@ -1300,43 +1237,37 @@ apiRouter.post( decisionReason: string } - await db.transaction(async (t) => { - const suggestedChartRevision = await db.mysqlFirst( - `SELECT id, chartId, suggestedConfig, originalConfig, status FROM suggested_chart_revisions WHERE id=?`, - [suggestedChartRevisionId] + await db.knexInstance().transaction(async (t) => { + const suggestedChartRevisionFromDb: + | Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + | "id" + | "chartId" + | "suggestedConfig" + | "originalConfig" + | "status" + > + | undefined = await getReducedSuggestedChartRevisionById( + t, + suggestedChartRevisionId ) - if (!suggestedChartRevision) { + if (!suggestedChartRevisionFromDb) { throw new JsonError( `No suggested chart revision found for id '${suggestedChartRevisionId}'`, 404 ) } - if (suggestedConfig !== undefined && suggestedConfig !== null) { - suggestedChartRevision.suggestedConfig = suggestedConfig - } else { - suggestedChartRevision.suggestedConfig = JSON.parse( - suggestedChartRevision.suggestedConfig - ) + const suggestedChartRevision = { + ...suggestedChartRevisionFromDb, + existingConfig: await expectChartById( + suggestedChartRevisionFromDb.chartId + ), } - suggestedChartRevision.originalConfig = JSON.parse( - suggestedChartRevision.originalConfig - ) - suggestedChartRevision.existingConfig = await expectChartById( - suggestedChartRevision.chartId - ) - const canApprove = SuggestedChartRevision.checkCanApprove( - suggestedChartRevision - ) - const canReject = SuggestedChartRevision.checkCanReject( - suggestedChartRevision - ) - const canFlag = SuggestedChartRevision.checkCanFlag( - suggestedChartRevision - ) - const canPending = SuggestedChartRevision.checkCanPending( - suggestedChartRevision - ) + const canApprove = checkCanApprove(suggestedChartRevision) + const canReject = checkCanReject(suggestedChartRevision) + const canFlag = checkCanFlag(suggestedChartRevision) + const canPending = checkCanPending(suggestedChartRevision) const canUpdate = (status === "approved" && canApprove) || @@ -1351,7 +1282,7 @@ apiRouter.post( ) } - await t.execute( + await t.raw( ` UPDATE suggested_chart_revisions SET status=?, decisionReason=?, updatedAt=?, updatedBy=? @@ -1369,7 +1300,7 @@ apiRouter.post( // Update config ONLY when APPROVE button is clicked // Makes sense when the suggested config is a sugegstion by GPT, otherwise is redundant but we are cool with it if (status === SuggestedChartRevisionStatus.approved) { - await t.execute( + await t.raw( ` UPDATE suggested_chart_revisions SET suggestedConfig=? @@ -1412,38 +1343,29 @@ apiRouter.post( ) apiRouter.get("/users.json", async (req: Request, res: Response) => ({ - users: await User.find({ - select: [ - "id", - "email", - "fullName", - "isActive", - "isSuperuser", - "createdAt", - "updatedAt", - "lastLogin", - "lastSeen", - ], - order: { lastSeen: "DESC" }, - }), + users: await db + .knexInstance() + .select( + "id" satisfies keyof DbPlainUser, + "email" satisfies keyof DbPlainUser, + "fullName" satisfies keyof DbPlainUser, + "isActive" satisfies keyof DbPlainUser, + "isSuperuser" satisfies keyof DbPlainUser, + "createdAt" satisfies keyof DbPlainUser, + "updatedAt" satisfies keyof DbPlainUser, + "lastLogin" satisfies keyof DbPlainUser, + "lastSeen" satisfies keyof DbPlainUser + ) + .from(UsersTableName) + .orderBy("lastSeen", "desc"), })) -apiRouter.get("/users/:userId.json", async (req: Request, res: Response) => ({ - user: await User.findOne({ - where: { id: parseIntOrUndefined(req.params.userId) }, - select: { - id: true, - email: true, - fullName: true, - isActive: true, - isSuperuser: true, - createdAt: true, - updatedAt: true, - lastLogin: true, - lastSeen: true, - }, - }), -})) +apiRouter.get("/users/:userId.json", async (req: Request, res: Response) => { + const id = parseIntOrUndefined(req.params.userId) + if (!id) throw new JsonError("No user id given") + const user = await getUserById(db.knexInstance(), id) + return { user } +}) apiRouter.delete("/users/:userId", async (req: Request, res: Response) => { if (!res.locals.user.isSuperuser) @@ -1461,16 +1383,18 @@ apiRouter.put("/users/:userId", async (req: Request, res: Response) => { if (!res.locals.user.isSuperuser) throw new JsonError("Permission denied", 403) - const userId = parseIntOrUndefined(req.params.userId) - const user = - userId !== undefined ? await User.findOneBy({ id: userId }) : null - if (!user) throw new JsonError("No such user", 404) + return db.knexInstance().transaction(async (t) => { + const userId = parseIntOrUndefined(req.params.userId) + const user = userId !== undefined ? await getUserById(t, userId) : null + if (!user) throw new JsonError("No such user", 404) - user.fullName = req.body.fullName - user.isActive = req.body.isActive - await user.save() + user.fullName = req.body.fullName + user.isActive = req.body.isActive - return { success: true } + await updateUser(t, userId!, pick(user, ["fullName", "isActive"])) + + return { success: true } + }) }) apiRouter.post("/users/add", async (req: Request, res: Response) => { @@ -1479,13 +1403,9 @@ apiRouter.post("/users/add", async (req: Request, res: Response) => { const { email, fullName } = req.body - await transaction(async (ctx) => { - const user = new User() - user.email = email - user.fullName = fullName - user.createdAt = new Date() - user.updatedAt = new Date() - await ctx.manager.getRepository(User).save(user) + await insertUser(db.knexInstance(), { + email, + fullName, }) return { success: true } @@ -1494,7 +1414,7 @@ apiRouter.post("/users/add", async (req: Request, res: Response) => { apiRouter.get("/variables.json", async (req) => { const limit = parseIntOrUndefined(req.query.limit as string) ?? 50 const query = req.query.search as string - return await searchVariables(query, limit) + return await searchVariables(query, limit, db.knexInstance()) }) apiRouter.get( @@ -1712,8 +1632,10 @@ apiRouter.get( await Chart.assignTagsForCharts(charts) - const grapherConfig = - await getMergedGrapherConfigForVariable(variableId) + const grapherConfig = await getMergedGrapherConfigForVariable( + variableId, + db.knexInstance() + ) if ( grapherConfig && (!grapherConfig.dimensions || grapherConfig.dimensions.length === 0) diff --git a/adminSiteServer/gitDataExport.ts b/adminSiteServer/gitDataExport.ts index 1d4a8a2f687..fb906f672b5 100644 --- a/adminSiteServer/gitDataExport.ts +++ b/adminSiteServer/gitDataExport.ts @@ -104,7 +104,7 @@ export async function syncDatasetToGitRepo( await Promise.all([ fs.writeFile( path.join(tmpDatasetDir, `${dataset.filename}.csv`), - await dataset.toCSV() + await dataset.toCSV(db.knexInstance()) ), fs.writeFile( path.join(tmpDatasetDir, `datapackage.json`), diff --git a/adminSiteServer/mockSiteRouter.tsx b/adminSiteServer/mockSiteRouter.tsx index 7dd809be0d6..326bc7fcd78 100644 --- a/adminSiteServer/mockSiteRouter.tsx +++ b/adminSiteServer/mockSiteRouter.tsx @@ -53,6 +53,7 @@ import { } from "../baker/GrapherBaker.js" import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { GdocDataInsight } from "../db/model/Gdoc/GdocDataInsight.js" +import * as db from "../db/db.js" require("express-async-errors") @@ -160,6 +161,7 @@ mockSiteRouter.get("/grapher/:slug", async (req, res) => { res.send( await renderPreviewDataPageOrGrapherPage( entity.config, + db.knexInstance(), publishedExplorersBySlug ) ) @@ -227,19 +229,28 @@ mockSiteRouter.get("/datapage-preview/:id", async (req, res) => { await explorerAdminServer.getAllPublishedExplorersBySlugCached() res.send( - await renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing: true, - useIndicatorGrapherConfigs: true, - publishedExplorersBySlug, - }) + await renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing: true, + useIndicatorGrapherConfigs: true, + publishedExplorersBySlug, + }, + db.knexInstance() + ) ) }) countryProfileSpecs.forEach((spec) => mockSiteRouter.get(`/${spec.rootPath}/:countrySlug`, async (req, res) => - res.send(await countryProfileCountryPage(spec, req.params.countrySlug)) + res.send( + await countryProfileCountryPage( + spec, + req.params.countrySlug, + db.knexInstance() + ) + ) ) ) @@ -345,7 +356,7 @@ mockSiteRouter.get("/*", async (req, res) => { } try { - res.send(await renderPageBySlug(slug)) + res.send(await renderPageBySlug(slug, db.knexInstance())) } catch (e) { console.error(e) res.status(404).send(await renderNotFoundPage()) diff --git a/baker/DeployUtils.ts b/baker/DeployUtils.ts index b3203874ee8..88b2ea794bb 100644 --- a/baker/DeployUtils.ts +++ b/baker/DeployUtils.ts @@ -11,6 +11,7 @@ import { import { SiteBaker } from "../baker/SiteBaker.js" import { WebClient } from "@slack/web-api" import { DeployChange, DeployMetadata } from "@ourworldindata/utils" +import { Knex } from "knex" const deployQueueServer = new DeployQueueServer() @@ -34,6 +35,7 @@ export const defaultCommitMessage = async (): Promise => { */ const triggerBakeAndDeploy = async ( deployMetadata: DeployMetadata, + knex: Knex, lightningQueue?: DeployChange[] ) => { // deploy to Buildkite if we're on master and BUILDKITE_API_ACCESS_TOKEN is set @@ -60,7 +62,7 @@ const triggerBakeAndDeploy = async ( await baker.bakeGDocPosts(lightningQueue.map((c) => c.slug!)) } else { - await baker.bakeAll() + await baker.bakeAll(knex) } } } @@ -151,7 +153,7 @@ let deploying = false * the end of the current one, as long as there are changes in the queue. * If there are no changes in the queue, a deploy won't be initiated. */ -export const deployIfQueueIsNotEmpty = async () => { +export const deployIfQueueIsNotEmpty = async (knex: Knex) => { if (deploying) return deploying = true let failures = 0 @@ -184,6 +186,7 @@ export const deployIfQueueIsNotEmpty = async () => { await getChangesSlackMentions(parsedQueue) await triggerBakeAndDeploy( { title: changesAuthorNames[0], changesSlackMentions }, + knex, // If every DeployChange is a lightning change, then we can do a // lightning deploy. In the future, we might want to separate // lightning updates from regular deploys so we could prioritize diff --git a/baker/GrapherBaker.tsx b/baker/GrapherBaker.tsx index 37d52ff0ecf..f1de4417a69 100644 --- a/baker/GrapherBaker.tsx +++ b/baker/GrapherBaker.tsx @@ -63,10 +63,13 @@ import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { getShortPageCitation } from "../site/gdocs/utils.js" import { isEmpty } from "lodash" import { getSlugForTopicTag, getTagToSlugMap } from "./GrapherBakingUtils.js" +import { Knex } from "knex" +import { knexRaw } from "../db/db.js" const renderDatapageIfApplicable = async ( grapher: GrapherInterface, isPreviewing: boolean, + knex: Knex, publishedExplorersBySlug?: Record, imageMetadataDictionary?: Record ) => { @@ -102,15 +105,18 @@ const renderDatapageIfApplicable = async ( !isEmpty(variableMetadata.descriptionFromProducer) || !isEmpty(variableMetadata.presentation?.titlePublic)) ) { - return await renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing: isPreviewing, - useIndicatorGrapherConfigs: false, - pageGrapher: grapher, - publishedExplorersBySlug, - imageMetadataDictionary, - }) + return await renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing: isPreviewing, + useIndicatorGrapherConfigs: false, + pageGrapher: grapher, + publishedExplorersBySlug, + imageMetadataDictionary, + }, + knex + ) } } return undefined @@ -122,12 +128,14 @@ const renderDatapageIfApplicable = async ( */ export const renderDataPageOrGrapherPage = async ( grapher: GrapherInterface, + knex: Knex, publishedExplorersBySlug?: Record, imageMetadataDictionary?: Record ): Promise => { const datapage = await renderDatapageIfApplicable( grapher, false, + knex, publishedExplorersBySlug, imageMetadataDictionary ) @@ -147,25 +155,30 @@ type EnrichedFaqLookupSuccess = { type EnrichedFaqLookupResult = EnrichedFaqLookupError | EnrichedFaqLookupSuccess -export async function renderDataPageV2({ - variableId, - variableMetadata, - isPreviewing, - useIndicatorGrapherConfigs, - pageGrapher, - publishedExplorersBySlug, - imageMetadataDictionary = {}, -}: { - variableId: number - variableMetadata: OwidVariableWithSource - isPreviewing: boolean - useIndicatorGrapherConfigs: boolean - pageGrapher?: GrapherInterface - publishedExplorersBySlug?: Record - imageMetadataDictionary?: Record -}) { - const grapherConfigForVariable = - await getMergedGrapherConfigForVariable(variableId) +export async function renderDataPageV2( + { + variableId, + variableMetadata, + isPreviewing, + useIndicatorGrapherConfigs, + pageGrapher, + publishedExplorersBySlug, + imageMetadataDictionary = {}, + }: { + variableId: number + variableMetadata: OwidVariableWithSource + isPreviewing: boolean + useIndicatorGrapherConfigs: boolean + pageGrapher?: GrapherInterface + publishedExplorersBySlug?: Record + imageMetadataDictionary?: Record + }, + knex: Knex +) { + const grapherConfigForVariable = await getMergedGrapherConfigForVariable( + variableId, + knex + ) // Only merge the grapher config on the indicator if the caller tells us to do so - // this is true for preview pages for datapages on the indicator level but false // if we are on Grapher pages. Once we have a good way in the grapher admin for how @@ -345,11 +358,13 @@ export async function renderDataPageV2({ */ export const renderPreviewDataPageOrGrapherPage = async ( grapher: GrapherInterface, + knex: Knex, publishedExplorersBySlug?: Record ) => { const datapage = await renderDatapageIfApplicable( grapher, true, + knex, publishedExplorersBySlug ) if (datapage) return datapage @@ -398,7 +413,8 @@ const chartIsSameVersion = async ( const bakeGrapherPageAndVariablesPngAndSVGIfChanged = async ( bakedSiteDir: string, imageMetadataDictionary: Record, - grapher: GrapherInterface + grapher: GrapherInterface, + knex: Knex ) => { const htmlPath = `${bakedSiteDir}/grapher/${grapher.slug}.html` const isSameVersion = await chartIsSameVersion(htmlPath, grapher.version) @@ -415,7 +431,12 @@ const bakeGrapherPageAndVariablesPngAndSVGIfChanged = async ( const outPath = `${bakedSiteDir}/grapher/${grapher.slug}.html` await fs.writeFile( outPath, - await renderDataPageOrGrapherPage(grapher, {}, imageMetadataDictionary) + await renderDataPageOrGrapherPage( + grapher, + knex, + {}, + imageMetadataDictionary + ) ) console.log(outPath) @@ -477,7 +498,8 @@ export interface BakeSingleGrapherChartArguments { } export const bakeSingleGrapherChart = async ( - args: BakeSingleGrapherChartArguments + args: BakeSingleGrapherChartArguments, + knex: Knex ) => { const grapher: GrapherInterface = JSON.parse(args.config) grapher.id = args.id @@ -492,20 +514,24 @@ export const bakeSingleGrapherChart = async ( await bakeGrapherPageAndVariablesPngAndSVGIfChanged( args.bakedSiteDir, args.imageMetadataDictionary, - grapher + grapher, + knex ) return args } export const bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers = - async (bakedSiteDir: string) => { + async (bakedSiteDir: string, knex: Knex) => { const chartsToBake: { id: number; config: string; slug: string }[] = - await db.queryMysql(` + await knexRaw( + ` SELECT id, config, config->>'$.slug' as slug FROM charts WHERE JSON_EXTRACT(config, "$.isPublished")=true ORDER BY JSON_EXTRACT(config, "$.slug") ASC - `) + `, + knex + ) const newSlugs = chartsToBake.map((row) => row.slug) await fs.mkdirp(bakedSiteDir + "/grapher") @@ -538,7 +564,7 @@ export const bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers = if (MAX_NUM_BAKE_PROCESSES === 1) { await Promise.all( jobs.map(async (job) => { - await bakeSingleGrapherChart(job) + await bakeSingleGrapherChart(job, knex) progressBar.tick({ name: `slug ${job.slug}` }) }) ) diff --git a/baker/SiteBaker.tsx b/baker/SiteBaker.tsx index 60c6a3f5086..994937b9d5f 100644 --- a/baker/SiteBaker.tsx +++ b/baker/SiteBaker.tsx @@ -84,6 +84,7 @@ import { import pMap from "p-map" import { GdocDataInsight } from "../db/model/Gdoc/GdocDataInsight.js" import { fullGdocToMinimalGdoc } from "../db/model/Gdoc/gdocUtils.js" +import { Knex } from "knex" type PrefetchedAttachments = { linkedDocuments: Record @@ -194,7 +195,7 @@ export class SiteBaker { this.progressBar.tick({ name: "✅ baked embeds" }) } - private async bakeCountryProfiles() { + private async bakeCountryProfiles(knex: Knex) { if (!this.bakeSteps.has("countryProfiles")) return await Promise.all( countryProfileSpecs.map(async (spec) => { @@ -207,6 +208,7 @@ export class SiteBaker { const html = await renderCountryProfile( spec, country, + knex, this.grapherExports ).catch(() => console.error( @@ -240,8 +242,13 @@ export class SiteBaker { } // Bake an individual post/page - private async bakePost(post: FullPost) { - const html = await renderPost(post, this.baseUrl, this.grapherExports) + private async bakePost(post: FullPost, knex: Knex) { + const html = await renderPost( + post, + knex, + this.baseUrl, + this.grapherExports + ) const outPath = path.join(this.bakedSiteDir, `${post.slug}.html`) await fs.mkdirp(path.dirname(outPath)) @@ -409,7 +416,7 @@ export class SiteBaker { this.progressBar.tick({ name: "✅ removed deleted posts" }) } - private async bakePosts() { + private async bakePosts(knex: Knex) { if (!this.bakeSteps.has("wordpressPosts")) return // TODO: the knex instance should be handed down as a parameter const alreadyPublishedViaGdocsSlugsSet = @@ -423,7 +430,9 @@ export class SiteBaker { await pMap( postsApi, async (postApi) => - wpdb.getFullPost(postApi).then((post) => this.bakePost(post)), + wpdb + .getFullPost(postApi) + .then((post) => this.bakePost(post, knex)), { concurrency: 10 } ) @@ -824,26 +833,27 @@ export class SiteBaker { this.progressBar.tick({ name: "✅ baked redirects" }) } - async bakeWordpressPages() { + async bakeWordpressPages(knex: Knex) { await this.bakeRedirects() await this.bakeEmbeds() await this.bakeBlogIndex() await this.bakeRSS() await this.bakeAssets() await this.bakeGoogleScholar() - await this.bakePosts() + await this.bakePosts(knex) } - private async _bakeNonWordpressPages() { + private async _bakeNonWordpressPages(knex: Knex) { if (this.bakeSteps.has("countries")) { await bakeCountries(this) } await this.bakeSpecialPages() - await this.bakeCountryProfiles() + await this.bakeCountryProfiles(knex) await this.bakeExplorers() if (this.bakeSteps.has("charts")) { await bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers( - this.bakedSiteDir + this.bakedSiteDir, + knex ) this.progressBar.tick({ name: "✅ bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers", @@ -856,7 +866,7 @@ export class SiteBaker { await this.bakeDriveImages() } - async bakeNonWordpressPages() { + async bakeNonWordpressPages(knex: Knex) { await db.getConnection() const progressBarTotal = nonWordpressSteps .map((step) => this.bakeSteps.has(step)) @@ -867,15 +877,15 @@ export class SiteBaker { total: progressBarTotal, } ) - await this._bakeNonWordpressPages() + await this._bakeNonWordpressPages(knex) } - async bakeAll() { + async bakeAll(knex: Knex) { // Ensure caches are correctly initialized this.flushCache() await this.removeDeletedPosts() - await this.bakeWordpressPages() - await this._bakeNonWordpressPages() + await this.bakeWordpressPages(knex) + await this._bakeNonWordpressPages(knex) this.flushCache() } diff --git a/baker/algolia/indexToAlgolia.tsx b/baker/algolia/indexToAlgolia.tsx index 402490c36e0..7c3eaf97103 100644 --- a/baker/algolia/indexToAlgolia.tsx +++ b/baker/algolia/indexToAlgolia.tsx @@ -26,6 +26,7 @@ import { Pageview } from "../../db/model/Pageview.js" import { GdocPost } from "../../db/model/Gdoc/GdocPost.js" import { ArticleBlocks } from "../../site/gdocs/components/ArticleBlocks.js" import React from "react" +import { Knex } from "knex" interface TypeAndImportance { type: PageType @@ -73,7 +74,8 @@ function generateChunksFromHtmlText(htmlString: string) { async function generateWordpressRecords( postsApi: PostRestApi[], - pageviews: Record + pageviews: Record, + knex: Knex ): Promise { const getPostTypeAndImportance = ( post: FormattedPost, @@ -101,7 +103,7 @@ async function generateWordpressRecords( continue } - const post = await formatPost(rawPost, { footnotes: false }) + const post = await formatPost(rawPost, { footnotes: false }, knex) const chunks = generateChunksFromHtmlText(post.html) const tags = await wpdb.getPostTags(post.id) const postTypeAndImportance = getPostTypeAndImportance(post, tags) @@ -186,7 +188,7 @@ function generateGdocRecords( } // Generate records for countries, WP posts (not including posts that have been succeeded by Gdocs equivalents), and Gdocs -const getPagesRecords = async () => { +const getPagesRecords = async (knex: Knex) => { const pageviews = await Pageview.getViewsByUrlObj() const gdocs = await GdocPost.getPublishedGdocs() const publishedGdocsBySlug = keyBy(gdocs, "slug") @@ -205,13 +207,17 @@ const getPagesRecords = async () => { }) const countryRecords = generateCountryRecords(countries, pageviews) - const wordpressRecords = await generateWordpressRecords(postsApi, pageviews) + const wordpressRecords = await generateWordpressRecords( + postsApi, + pageviews, + knex + ) const gdocsRecords = generateGdocRecords(gdocs, pageviews) return [...countryRecords, ...wordpressRecords, ...gdocsRecords] } -const indexToAlgolia = async () => { +const indexToAlgolia = async (knex: Knex) => { if (!ALGOLIA_INDEXING) return const client = getAlgoliaClient() @@ -222,7 +228,7 @@ const indexToAlgolia = async () => { const index = client.initIndex(SearchIndexName.Pages) await db.getConnection() - const records = await getPagesRecords() + const records = await getPagesRecords(knex) index.replaceAllObjects(records) @@ -235,4 +241,4 @@ process.on("unhandledRejection", (e) => { process.exit(1) }) -indexToAlgolia() +indexToAlgolia(db.knexInstance()) diff --git a/baker/buildLocalBake.ts b/baker/buildLocalBake.ts index 930f46a0750..1d7ed914854 100644 --- a/baker/buildLocalBake.ts +++ b/baker/buildLocalBake.ts @@ -5,6 +5,7 @@ import { hideBin } from "yargs/helpers" import { BakeStep, BakeStepConfig, bakeSteps, SiteBaker } from "./SiteBaker.js" import fs from "fs-extra" import { normalize } from "path" +import * as db from "../db/db.js" const bakeDomainToFolder = async ( baseUrl = "http://localhost:3000/", @@ -15,7 +16,7 @@ const bakeDomainToFolder = async ( fs.mkdirp(dir) const baker = new SiteBaker(dir, baseUrl, bakeSteps) console.log(`Baking site locally with baseUrl '${baseUrl}' to dir '${dir}'`) - await baker.bakeAll() + await baker.bakeAll(db.knexInstance()) } yargs(hideBin(process.argv)) diff --git a/baker/countryProfiles.tsx b/baker/countryProfiles.tsx index 1b50f74c0e9..4f77261e142 100644 --- a/baker/countryProfiles.tsx +++ b/baker/countryProfiles.tsx @@ -1,7 +1,13 @@ import React from "react" import * as db from "../db/db.js" import { CountriesIndexPage } from "../site/CountriesIndexPage.js" -import { GrapherInterface } from "@ourworldindata/types" +import { + GrapherInterface, + DbEnrichedVariable, + DbRawVariable, + VariablesTableName, + parseVariablesRow, +} from "@ourworldindata/types" import * as lodash from "lodash" import { CountryProfileIndicator, @@ -10,12 +16,7 @@ import { import { SiteBaker } from "./SiteBaker.js" import { countries, getCountryBySlug, JsonError } from "@ourworldindata/utils" import { renderToHtmlPage } from "./siteRenderers.js" -import { - parseVariableRows, - VariableRow, - variableTable, - dataAsDF, -} from "../db/model/Variable.js" +import { dataAsDF } from "../db/model/Variable.js" import pl from "nodejs-polars" export const countriesIndexPage = (baseUrl: string) => @@ -51,14 +52,17 @@ const countryIndicatorGraphers = async (): Promise => return graphers.filter(checkShouldShowIndicator) }) -export const countryIndicatorVariables = async (): Promise => +export const countryIndicatorVariables = async (): Promise< + DbEnrichedVariable[] +> => bakeCache(countryIndicatorVariables, async () => { const variableIds = (await countryIndicatorGraphers()).map( (c) => c.dimensions![0]!.variableId ) - return parseVariableRows( - await db.knexTable(variableTable).whereIn("id", variableIds) - ) + const rows: DbRawVariable[] = await db + .knexTable(VariablesTableName) + .whereIn("id", variableIds) + return rows.map(parseVariablesRow) }) export const denormalizeLatestCountryData = async (variableIds?: number[]) => { @@ -92,7 +96,7 @@ export const denormalizeLatestCountryData = async (variableIds?: number[]) => { const currentYear = new Date().getUTCFullYear() - const df = (await dataAsDF(variableIds)) + const df = (await dataAsDF(variableIds, db.knexInstance())) .filter( pl .col("entityId") diff --git a/baker/formatWordpressPost.tsx b/baker/formatWordpressPost.tsx index eeebf32b6e0..002d4578fe0 100644 --- a/baker/formatWordpressPost.tsx +++ b/baker/formatWordpressPost.tsx @@ -61,6 +61,7 @@ import { renderKeyInsights, renderProminentLinks } from "./siteRenderers.js" import { KEY_INSIGHTS_CLASS_NAME } from "../site/blocks/KeyInsights.js" import { RELATED_CHARTS_CLASS_NAME } from "../site/blocks/RelatedCharts.js" import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js" +import { Knex } from "knex" const initMathJax = () => { const adaptor = liteAdaptor() @@ -130,6 +131,7 @@ const formatLatex = async ( export const formatWordpressPost = async ( post: FullPost, formattingOptions: FormattingOptions, + knex: Knex, grapherExports?: GrapherExports ): Promise => { let html = post.content @@ -183,18 +185,19 @@ export const formatWordpressPost = async ( const { queryArgs, template } = dataValueConfiguration const { variableId, chartId } = queryArgs const { value, year, unit, entityName } = - (await getDataValue(queryArgs)) || {} + (await getDataValue(queryArgs, knex)) || {} if (!value || !year || !entityName || !template) continue let formattedValue if (variableId && chartId) { const legacyVariableDisplayConfig = - await getOwidVariableDisplayConfig(variableId) + await getOwidVariableDisplayConfig(variableId, knex) const legacyChartDimension = await getOwidChartDimensionConfigForVariable( variableId, - chartId + chartId, + knex ) formattedValue = formatDataValue( value, @@ -606,6 +609,7 @@ export const formatWordpressPost = async ( export const formatPost = async ( post: FullPost, formattingOptions: FormattingOptions, + knex: Knex, grapherExports?: GrapherExports ): Promise => { // No formatting applied, plain source HTML returned @@ -625,5 +629,5 @@ export const formatPost = async ( ...formattingOptions, } - return formatWordpressPost(post, options, grapherExports) + return formatWordpressPost(post, options, knex, grapherExports) } diff --git a/baker/runBakeGraphers.ts b/baker/runBakeGraphers.ts index 4136be0ba16..d034bea8682 100755 --- a/baker/runBakeGraphers.ts +++ b/baker/runBakeGraphers.ts @@ -1,5 +1,6 @@ #! /usr/bin/env node import { bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers } from "./GrapherBaker.js" +import * as db from "../db/db.js" /** * This bakes all the Graphers to a folder on your computer, running the same baking code as the SiteBaker. @@ -9,7 +10,8 @@ import { bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers } fro const main = async (folder: string) => { await bakeAllChangedGrapherPagesVariablesPngSvgAndDeleteRemovedGraphers( - folder + folder, + db.knexInstance() ) } diff --git a/baker/siteRenderers.tsx b/baker/siteRenderers.tsx index e805c32d833..dd022517a92 100644 --- a/baker/siteRenderers.tsx +++ b/baker/siteRenderers.tsx @@ -92,6 +92,7 @@ import { GdocPost } from "../db/model/Gdoc/GdocPost.js" import { logErrorAndMaybeSendToBugsnag } from "../serverUtils/errorLog.js" import { GdocFactory } from "../db/model/Gdoc/GdocFactory.js" import { SiteNavigationStatic } from "../site/SiteNavigation.js" +import { Knex } from "knex" export const renderToHtmlPage = (element: any) => `${ReactDOMServer.renderToStaticMarkup(element)}` @@ -193,14 +194,20 @@ export const renderGdoc = (gdoc: OwidGdoc, isPreviewing: boolean = false) => { ) } -export const renderPageBySlug = async (slug: string) => { +export const renderPageBySlug = async ( + slug: string, + knex: Knex +) => { const post = await getPostBySlug(slug) - return renderPost(post) + return renderPost(post, knex) } -export const renderPreview = async (postId: number): Promise => { +export const renderPreview = async ( + postId: number, + knex: Knex +): Promise => { const postApi = await getLatestPostRevision(postId) - return renderPost(postApi) + return renderPost(postApi, knex) } export const renderMenuJson = async () => { @@ -209,6 +216,7 @@ export const renderMenuJson = async () => { export const renderPost = async ( post: FullPost, + knex: Knex, baseUrl: string = BAKED_BASE_URL, grapherExports?: GrapherExports ) => { @@ -229,7 +237,12 @@ export const renderPost = async ( // Extract formatting options from post HTML comment (if any) const formattingOptions = extractFormattingOptions(post.content) - const formatted = await formatPost(post, formattingOptions, grapherExports) + const formatted = await formatPost( + post, + formattingOptions, + knex, + grapherExports + ) const pageOverrides = await getPageOverrides(post, formattingOptions) const citationStatus = @@ -482,6 +495,7 @@ export const feedbackPage = () => const getCountryProfilePost = memoize( async ( profileSpec: CountryProfileSpec, + knex: Knex, grapherExports?: GrapherExports ): Promise<[FormattedPost, FormattingOptions]> => { // Get formatted content from generic covid country profile page. @@ -495,6 +509,7 @@ const getCountryProfilePost = memoize( const formattedPost = await formatPost( genericCountryProfilePost, profileFormattingOptions, + knex, grapherExports ) @@ -512,10 +527,12 @@ const getCountryProfileLandingPost = memoize( export const renderCountryProfile = async ( profileSpec: CountryProfileSpec, country: Country, + knex: Knex, grapherExports?: GrapherExports ) => { const [formatted, formattingOptions] = await getCountryProfilePost( profileSpec, + knex, grapherExports ) @@ -546,13 +563,14 @@ export const renderCountryProfile = async ( export const countryProfileCountryPage = async ( profileSpec: CountryProfileSpec, - countrySlug: string + countrySlug: string, + knex: Knex ) => { const country = getCountryBySlug(countrySlug) if (!country) throw new JsonError(`No such country ${countrySlug}`, 404) // Voluntarily not dealing with grapherExports on devServer for simplicity - return renderCountryProfile(profileSpec, country) + return renderCountryProfile(profileSpec, country, knex) } export const flushCache = () => getCountryProfilePost.cache.clear?.() diff --git a/baker/startDeployQueueServer.ts b/baker/startDeployQueueServer.ts index 0f7ae181a46..2e82864cbe5 100644 --- a/baker/startDeployQueueServer.ts +++ b/baker/startDeployQueueServer.ts @@ -35,7 +35,7 @@ const main = async () => { setTimeout(deployIfQueueIsNotEmpty, 10 * 1000) }) - deployIfQueueIsNotEmpty() + deployIfQueueIsNotEmpty(db.knexInstance()) } main() diff --git a/db/Variable.test.ts b/db/Variable.test.ts index 537040f4738..30cf95002f9 100644 --- a/db/Variable.test.ts +++ b/db/Variable.test.ts @@ -5,6 +5,7 @@ import * as Variable from "./model/Variable.js" import pl from "nodejs-polars" import { Writable } from "stream" import { OwidVariableId } from "@ourworldindata/utils" +import * as db from "./db.js" import { jest } from "@jest/globals" @@ -45,7 +46,7 @@ describe("writeVariableCSV", () => { callback(null) }, }) - await writeVariableCSV(variableIds, writeStream) + await writeVariableCSV(variableIds, writeStream, db.knexInstance()) return out } @@ -164,7 +165,7 @@ describe("_dataAsDFfromS3", () => { }, } mockS3data(s3data) - const df = await _dataAsDFfromS3([1]) + const df = await _dataAsDFfromS3([1], db.knexInstance()) expect(df.toObject()).toEqual({ entityCode: ["code", "code"], entityId: [1, 1], diff --git a/db/model/Dataset.ts b/db/model/Dataset.ts index ebed46be3aa..fb243ea1a6c 100644 --- a/db/model/Dataset.ts +++ b/db/model/Dataset.ts @@ -13,10 +13,17 @@ import { User } from "./User.js" import { Source } from "./Source.js" import * as db from "../db.js" -import { slugify } from "@ourworldindata/utils" +import { + DbPlainTag, + DbRawVariable, + VariablesTableName, + slugify, +} from "@ourworldindata/utils" import filenamify from "filenamify" -import { VariableRow, variableTable, writeVariableCSV } from "./Variable.js" +import { writeVariableCSV } from "./Variable.js" import _ from "lodash" +import { Knex } from "knex" +import { knexRaw } from "../db.js" @Entity("datasets") @Unique(["name", "namespace"]) @@ -38,20 +45,25 @@ export class Dataset extends BaseEntity { createdByUser!: Relation // Export dataset variables to CSV (not including metadata) - static async writeCSV(datasetId: number, stream: Writable): Promise { + static async writeCSV( + datasetId: number, + stream: Writable, + knex: Knex + ): Promise { // get variables of a dataset const variableIds = ( - await db.queryMysql( + await knexRaw( ` SELECT id as variableId FROM variables v WHERE datasetId=?`, + knex, [datasetId] ) ).map((row: any) => row.variableId) - await writeVariableCSV(variableIds, stream) + await writeVariableCSV(variableIds, stream, knex) } static async setTags(datasetId: number, tagIds: number[]): Promise { @@ -68,12 +80,16 @@ export class Dataset extends BaseEntity { }) } - async toCSV(): Promise { + async toCSV(knex: Knex): Promise { let csv = "" - await Dataset.writeCSV(this.id, { - write: (s: string) => (csv += s), - end: () => null, - } as any) + await Dataset.writeCSV( + this.id, + { + write: (s: string) => (csv += s), + end: () => null, + } as any, + knex + ) return csv } @@ -90,10 +106,11 @@ export class Dataset extends BaseEntity { // XXX const sources = await Source.findBy({ datasetId: this.id }) const variables = (await db - .knexTable(variableTable) - .where({ datasetId: this.id })) as VariableRow[] - const tags = await db.queryMysql( + .knexTable(VariablesTableName) + .where({ datasetId: this.id })) as DbRawVariable[] + const tags: Pick[] = await knexRaw( `SELECT t.id, t.name FROM dataset_tags dt JOIN tags t ON t.id=dt.tagId WHERE dt.datasetId=?`, + db.knexInstance(), // TODO: pass down the knex instance instead of using the global one [this.id] ) @@ -119,7 +136,7 @@ export class Dataset extends BaseEntity { schema: { fields: initialFields.concat( variables.map((v) => ({ - name: v.name, + name: v.name ?? "", type: "any", description: v.description, owidDisplaySettings: v.display, diff --git a/db/model/SuggestedChartRevision.ts b/db/model/SuggestedChartRevision.ts index 55c23a5f25b..03528a6ebb4 100644 --- a/db/model/SuggestedChartRevision.ts +++ b/db/model/SuggestedChartRevision.ts @@ -1,9 +1,21 @@ import { Entity, PrimaryGeneratedColumn, Column, BaseEntity } from "typeorm" -import { SuggestedChartRevisionStatus } from "@ourworldindata/utils" +import { + GrapherInterface, + SuggestedChartRevisionStatus, + SuggestedChartRevisionsRowEnriched, + SuggestedChartRevisionsRowRaw, + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw, + parseChartConfig, + parseNullableChartConfig, + parseSuggestedChartRevisionsRowWithUsersAndExistingConfig, +} from "@ourworldindata/utils" +import * as db from "../db.js" +import { Knex } from "knex" @Entity("suggested_chart_revisions") -export class SuggestedChartRevision extends BaseEntity { +export class SuggestedChartRevisionClass extends BaseEntity { @PrimaryGeneratedColumn() id!: number @Column() chartId!: number @Column({ type: "json" }) suggestedConfig: any @@ -24,106 +36,284 @@ export class SuggestedChartRevision extends BaseEntity { @Column() createdAt!: Date @Column() updatedAt!: Date - existingConfig?: any - canApprove?: boolean canReject?: boolean canFlag?: boolean canPending?: boolean @Column({ type: "json" }) experimental?: any +} - static isValidStatus(status: SuggestedChartRevisionStatus): boolean { - return Object.values(SuggestedChartRevisionStatus).includes(status) - } +export function isValidStatus( + status: any +): status is SuggestedChartRevisionStatus { + return Object.values(SuggestedChartRevisionStatus).includes(status) +} - static checkCanApprove( - suggestedChartRevision: SuggestedChartRevision - ): boolean { - // note: a suggestion can be approved if status == "rejected" | - // "flagged" | "pending" AND the original config version equals - // the existing config version (i.e. the existing chart has not - // been changed since the suggestion was created). - const status = suggestedChartRevision.status - const originalVersion = suggestedChartRevision.originalConfig?.version - const existingVersion = suggestedChartRevision.existingConfig?.version - const originalVersionExists = - originalVersion !== null && originalVersion !== undefined - const existingVersionExists = - existingVersion !== null && existingVersion !== undefined - if ( - [ - SuggestedChartRevisionStatus.rejected, - SuggestedChartRevisionStatus.flagged, - SuggestedChartRevisionStatus.pending, - ].includes(status) && - originalVersionExists && - existingVersionExists && - originalVersion === existingVersion - ) { - return true - } - return false +export function checkCanApprove( + suggestedChartRevision: Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + "status" | "originalConfig" | "existingConfig" + > +): boolean { + // note: a suggestion can be approved if status == "rejected" | + // "flagged" | "pending" AND the original config version equals + // the existing config version (i.e. the existing chart has not + // been changed since the suggestion was created). + const status = suggestedChartRevision.status + const originalVersion = suggestedChartRevision.originalConfig?.version + const existingVersion = suggestedChartRevision.existingConfig?.version + const originalVersionExists = + originalVersion !== null && originalVersion !== undefined + const existingVersionExists = + existingVersion !== null && existingVersion !== undefined + if ( + [ + SuggestedChartRevisionStatus.rejected, + SuggestedChartRevisionStatus.flagged, + SuggestedChartRevisionStatus.pending, + ].includes(status) && + originalVersionExists && + existingVersionExists && + originalVersion === existingVersion + ) { + return true } + return false +} - static checkCanReject( - suggestedChartRevision: SuggestedChartRevision - ): boolean { - // note: a suggestion can be rejected if: (1) status == - // "pending" | "flagged"; or (2) status == "approved" and the - // suggested config version equals the existing chart version - // (i.e. the existing chart has not changed since the suggestion - // was approved). - const status = suggestedChartRevision.status - const suggestedVersion = suggestedChartRevision.suggestedConfig?.version - const existingVersion = suggestedChartRevision.existingConfig?.version - const suggestedVersionExists = - suggestedVersion !== null && suggestedVersion !== undefined - const existingVersionExists = - existingVersion !== null && existingVersion !== undefined - if ( - [ - SuggestedChartRevisionStatus.flagged, - SuggestedChartRevisionStatus.pending, - ].includes(status) - ) { - return true - } - if ( - status === "approved" && - suggestedVersionExists && - existingVersionExists && - suggestedVersion === existingVersion - ) { - return true - } - return false +export function checkCanReject( + suggestedChartRevision: Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + "status" | "suggestedConfig" | "existingConfig" + > +): boolean { + // note: a suggestion can be rejected if: (1) status == + // "pending" | "flagged"; or (2) status == "approved" and the + // suggested config version equals the existing chart version + // (i.e. the existing chart has not changed since the suggestion + // was approved). + const status = suggestedChartRevision.status + const suggestedVersion = suggestedChartRevision.suggestedConfig?.version + const existingVersion = suggestedChartRevision.existingConfig?.version + const suggestedVersionExists = + suggestedVersion !== null && suggestedVersion !== undefined + const existingVersionExists = + existingVersion !== null && existingVersion !== undefined + if ( + [ + SuggestedChartRevisionStatus.flagged, + SuggestedChartRevisionStatus.pending, + ].includes(status) + ) { + return true + } + if ( + status === "approved" && + suggestedVersionExists && + existingVersionExists && + suggestedVersion === existingVersion + ) { + return true } + return false +} - static checkCanFlag( - suggestedChartRevision: SuggestedChartRevision - ): boolean { - // note: a suggestion can be flagged if status == "pending" or - // if it is already flagged. Flagging a suggestion that is - // already flagged is a hack for updating the decisionReason - // column in the SuggestedChartRevisionApproverPage UI without - // changing the status column. - const status = suggestedChartRevision.status - if ( - [ - SuggestedChartRevisionStatus.flagged, - SuggestedChartRevisionStatus.pending, - ].includes(status) - ) { - return true - } - return false +export function checkCanFlag( + suggestedChartRevision: Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + "status" + > +): boolean { + // note: a suggestion can be flagged if status == "pending" or + // if it is already flagged. Flagging a suggestion that is + // already flagged is a hack for updating the decisionReason + // column in the SuggestedChartRevisionApproverPage UI without + // changing the status column. + const status = suggestedChartRevision.status + if ( + [ + SuggestedChartRevisionStatus.flagged, + SuggestedChartRevisionStatus.pending, + ].includes(status) + ) { + return true } + return false +} + +export function checkCanPending( + _suggestedChartRevision: Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + "status" + > +): boolean { + // note: a suggestion cannot be altered to pending from another status + return false +} + +export async function getAllSuggestedChartRevisionsWithStatus( + knex: Knex, + limit: number, + offset: number, + orderBy: string, + sortOrder: string, + status: SuggestedChartRevisionStatus | null +): Promise { + const rawRows = await knex.raw( + `-- sql + SELECT scr.id, + scr.chartId, + scr.updatedAt, + scr.createdAt, + scr.suggestedReason, + scr.decisionReason, + scr.status, + scr.suggestedConfig, + scr.originalConfig, + scr.changesInDataSummary, + scr.experimental, + scr.createdBy, + scr.updatedBy, + createdByUser.fullName as createdByFullName, + updatedByUser.fullName as updatedByFullName, + c.config as existingConfig, + c.updatedAt as chartUpdatedAt, + c.createdAt as chartCreatedAt + FROM suggested_chart_revisions as scr + LEFT JOIN charts c on c.id = scr.chartId + LEFT JOIN users createdByUser on createdByUser.id = scr.createdBy + LEFT JOIN users updatedByUser on updatedByUser.id = scr.updatedBy + ${status ? "WHERE scr.status = ?" : ""} + ORDER BY ${orderBy} ${sortOrder} + LIMIT ? OFFSET ? + `, + status ? [status, limit, offset] : [limit, offset] + ) + return rawRows.map( + parseSuggestedChartRevisionsRowWithUsersAndExistingConfig + ) +} +export async function getSuggestedChartRevisionsById( + knex: Knex, + id: number +): Promise< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched | undefined +> { + const suggestedChartRevision: SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw[] = + await knex.raw( + `-- sql + SELECT scr.id, + scr.chartId, + scr.updatedAt, + scr.createdAt, + scr.suggestedReason, scr.decisionReason, + scr.status, + scr.suggestedConfig, scr.changesInDataSummary, + scr.originalConfig, + scr.createdBy, + scr.updatedBy, + createdByUser.fullName as createdByFullName, + updatedByUser.fullName as updatedByFullName, + c.config as existingConfig, + c.updatedAt as chartUpdatedAt, + c.createdAt as chartCreatedAt + FROM suggested_chart_revisions as scr + LEFT JOIN charts c on c.id = scr.chartId + LEFT JOIN users createdByUser on createdByUser.id = scr.createdBy + LEFT JOIN users updatedByUser on updatedByUser.id = scr.updatedBy + WHERE scr.id = ? + `, + [id] + ) + if (suggestedChartRevision.length === 0) return undefined + return parseSuggestedChartRevisionsRowWithUsersAndExistingConfig( + suggestedChartRevision[0] + ) +} - static checkCanPending( - _suggestedChartRevision: SuggestedChartRevision - ): boolean { - // note: a suggestion cannot be altered to pending from another status - return false +export async function getReducedSuggestedChartRevisionById( + knex: Knex, + id: number +): Promise< + | Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + "id" | "chartId" | "suggestedConfig" | "originalConfig" | "status" + > + | undefined +> { + const rowRaw: Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw, + "id" | "chartId" | "suggestedConfig" | "originalConfig" | "status" + >[] = await knex.raw( + `SELECT id, chartId, suggestedConfig, originalConfig, status FROM suggested_chart_revisions WHERE id=?`, + [id] + ) + if (rowRaw.length === 0) return undefined + const rowEnriched: Pick< + SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + "id" | "chartId" | "suggestedConfig" | "originalConfig" | "status" + > = { + ...rowRaw[0], + suggestedConfig: parseChartConfig(rowRaw[0].suggestedConfig), + originalConfig: parseChartConfig(rowRaw[0].originalConfig), } + return rowEnriched +} + +export async function getNumSuggestedChartRevisionsWithStatus( + knex: Knex, + status: string | null +): Promise { + const numTotalRows = ( + await knex.raw( + ` + SELECT COUNT(*) as count + FROM suggested_chart_revisions + ${status ? "WHERE status = ?" : ""} + `, + status ? [status] : [] + ) + )[0].count + return numTotalRows ? parseInt(numTotalRows) : numTotalRows +} + +export async function getConfigFromChartsOrChartRevisionsPrioritized( + knex: Knex, + whereCond1: string, + whereCond2: string +): Promise< + { + id: number + config: GrapherInterface + priority: number + }[] +> { + const rawRows: { id: number; config: string; priority: number }[] = + await knex.raw( + ` + SELECT id, config, 1 as priority + FROM charts + WHERE ${whereCond1} + + UNION + + SELECT chartId as id, config, 2 as priority + FROM chart_revisions + WHERE ${whereCond2} + + ORDER BY priority + ` + ) + + const rows: { + id: number + config: GrapherInterface + priority: number + }[] = rawRows.map((row) => ({ + ...row, + config: parseChartConfig(row.config), + })) + + return rows } diff --git a/db/model/User.ts b/db/model/User.ts index 7b3d298fff5..4964b8198e4 100644 --- a/db/model/User.ts +++ b/db/model/User.ts @@ -10,6 +10,12 @@ import { Chart } from "./Chart.js" import { Dataset } from "./Dataset.js" import { ChartRevision } from "./ChartRevision.js" import { BCryptHasher } from "../hashers.js" +import { + DbPlainUser, + DbInsertUser, + UsersTableName, +} from "@ourworldindata/types" +import { Knex } from "knex" @Entity("users") export class User extends BaseEntity { @@ -35,9 +41,43 @@ export class User extends BaseEntity { @OneToMany(() => Dataset, (dataset) => dataset.createdByUser) createdDatasets!: Relation +} + +export async function setPassword( + knex: Knex, + id: number, + password: string +): Promise { + const h = new BCryptHasher() + const encrypted = await h.encode(password) + await updateUser(knex, id, { password: encrypted }) +} + +export async function getUserById( + knex: Knex, + id: number +): Promise { + return knex(UsersTableName).where({ id }).first() +} + +export async function insertUser( + knex: Knex, + user: DbInsertUser +): Promise<{ id: number }> { + return knex(UsersTableName).returning("id").insert(user) +} + +export async function updateUser( + knex: Knex, + id: number, + user: Partial +): Promise { + return knex(UsersTableName).where({ id }).update(user) +} - async setPassword(password: string): Promise { - const h = new BCryptHasher() - this.password = await h.encode(password) - } +export async function deleteUser( + knex: Knex, + id: number +): Promise { + return knex(UsersTableName).where({ id }).delete() } diff --git a/db/model/Variable.ts b/db/model/Variable.ts index a97d9e72350..c266b58e5c4 100644 --- a/db/model/Variable.ts +++ b/db/model/Variable.ts @@ -12,9 +12,10 @@ import { OwidVariableWithSourceAndDimension, OwidVariableId, retryPromise, - OwidLicense, GrapherInterface, - OwidProcessingLevel, + DbRawVariable, + VariablesTableName, + ChartsTableName, } from "@ourworldindata/utils" import { getVariableDataRoute, @@ -23,123 +24,23 @@ import { import pl from "nodejs-polars" import { DATA_API_URL } from "../../settings/serverSettings.js" import { escape } from "mysql" - -export interface VariableRow { - id: number - name: string - shortName?: string - code: string | null - unit: string - shortUnit: string | null - description: string | null - createdAt: Date - updatedAt: Date - datasetId: number - sourceId: number - display: OwidVariableDisplayConfigInterface - coverage?: string - timespan?: string - columnOrder?: number - catalogPath?: string - dimensions?: Dimensions - schemaVersion?: number - processingLevel?: OwidProcessingLevel - titlePublic?: string - titleVariant?: string - attributionShort?: string - citationInline?: string - descriptionShort?: string - descriptionFromProducer?: string - descriptionKey?: string[] // this is json in the db but by convention it is always a list of strings - descriptionProcessing?: string - licenses?: OwidLicense[] - grapherConfigAdmin?: GrapherInterface - grapherConfigETL?: GrapherInterface - license?: OwidLicense - updatePeriodDays?: number - datasetVersion?: string - version?: string - - // missing here but existsin the DB: - // originalMetadata -} - -interface Dimensions { - originalName: string - originalShortName: string - filters: { - name: string - value: string - }[] -} - -export type UnparsedVariableRow = Omit< - VariableRow, - | "display" - | "descriptionKey" - | "grapherConfigAdmin" - | "grapherConfigETL" - | "license" - | "licenses" -> & { - display: string - descriptionKey?: string - grapherConfigAdmin?: string - grapherConfigETL?: string - license?: string - licenses?: string -} - -export type VariableQueryRow = Readonly< - Omit & { - display: string - datasetName: string - nonRedistributable: number - sourceName: string - sourceDescription: string - dimensions: string - } -> - -export type Field = keyof VariableRow - -export const variableTable = "variables" - -export function parseVariableRows( - plainRows: UnparsedVariableRow[] -): VariableRow[] { - const parsedRows: VariableRow[] = [] - for (const plainRow of plainRows) { - const row = { - ...plainRow, - display: plainRow.display - ? JSON.parse(plainRow.display) - : undefined, - descriptionKey: plainRow.descriptionKey - ? JSON.parse(plainRow.descriptionKey) - : [], - grapherConfigAdmin: plainRow.grapherConfigAdmin - ? JSON.parse(plainRow.grapherConfigAdmin) - : [], - grapherConfigETL: plainRow.grapherConfigETL - ? JSON.parse(plainRow.grapherConfigETL) - : [], - licenses: plainRow.licenses ? JSON.parse(plainRow.licenses) : [], - license: plainRow.license ? JSON.parse(plainRow.license) : [], - } - parsedRows.push(row) - } - return parsedRows -} +import { Knex } from "knex/types" +import { knexRaw, knexRawFirst } from "../db.js" export async function getMergedGrapherConfigForVariable( - variableId: number + variableId: number, + knex: Knex ): Promise { - const row = await db.mysqlFirst( + const rows: Pick< + DbRawVariable, + "grapherConfigAdmin" | "grapherConfigETL" + >[] = await knexRaw( `SELECT grapherConfigAdmin, grapherConfigETL FROM variables WHERE id = ?`, + knex, [variableId] ) - if (!row) return + if (!rows.length) return + const row = rows[0] const grapherConfigAdmin = row.grapherConfigAdmin ? JSON.parse(row.grapherConfigAdmin) : undefined @@ -149,6 +50,7 @@ export async function getMergedGrapherConfigForVariable( return _.merge({}, grapherConfigAdmin, grapherConfigETL) } +// TODO: these are domain functions and should live somewhere else export async function getVariableMetadata( variableId: number ): Promise { @@ -189,19 +91,20 @@ export async function getDataForMultipleVariables( export async function writeVariableCSV( variableIds: number[], - stream: Writable + stream: Writable, + knex: Knex ): Promise { // get variables as dataframe const variablesDF = ( - await readSQLasDF( - ` + await knex.raw( + `-- sql SELECT id as variableId, name as variableName, columnOrder - FROM variables v + FROM ?? v WHERE id IN (?)`, - [variableIds] + [VariablesTableName, variableIds] ) ).withColumn(pl.col("variableId").cast(pl.Int32)) @@ -214,7 +117,8 @@ export async function writeVariableCSV( // get data values as dataframe const dataValuesDF = await dataAsDF( - variablesDF.getColumn("variableId").toArray() + variablesDF.getColumn("variableId").toArray(), + knex ) dataValuesDF @@ -230,26 +134,26 @@ export async function writeVariableCSV( .writeCSV(stream) } -export const getDataValue = async ({ - variableId, - entityId, - year, -}: DataValueQueryArgs): Promise => { +export const getDataValue = async ( + { variableId, entityId, year }: DataValueQueryArgs, + knex: Knex +): Promise => { if (!variableId || !entityId) return - let df = (await dataAsDF([variableId])).filter( + let df = (await dataAsDF([variableId], knex)).filter( pl.col("entityId").eq(entityId) ) const unit = ( - await db.mysqlFirst( - ` - SELECT unit FROM variables + await knexRawFirst>( + `-- sql + SELECT unit FROM ?? WHERE id = ? `, - [variableId] + knex, + [VariablesTableName, variableId] ) - ).unit + )?.unit if (year) { df = df.filter(pl.col("year").eq(year)) @@ -276,17 +180,19 @@ export const getDataValue = async ({ export const getOwidChartDimensionConfigForVariable = async ( variableId: OwidVariableId, - chartId: number + chartId: number, + knex: Knex ): Promise => { - const row = await db.mysqlFirst( + const row = await db.knexRawFirst<{ dimensions: string }>( ` SELECT config->"$.dimensions" AS dimensions - FROM charts + FROM ?? WHERE id = ? `, - [chartId] + knex, + [ChartsTableName, chartId] ) - if (!row.dimensions) return + if (!row?.dimensions) return const dimensions = JSON.parse(row.dimensions) return dimensions.find( (dimension: OwidChartDimensionInterface) => @@ -295,18 +201,21 @@ export const getOwidChartDimensionConfigForVariable = async ( } export const getOwidVariableDisplayConfig = async ( - variableId: OwidVariableId + variableId: OwidVariableId, + knex: Knex ): Promise => { - const row = await db.mysqlFirst( + const row = await knexRawFirst>( `SELECT display FROM variables WHERE id = ?`, + knex, [variableId] ) - if (!row.display) return + if (!row?.display) return return JSON.parse(row.display) } export const entitiesAsDF = async ( - entityIds: number[] + entityIds: number[], + knex: Knex ): Promise => { return ( await readSQLasDF( @@ -317,7 +226,8 @@ export const entitiesAsDF = async ( code AS entityCode FROM entities WHERE id in (?) `, - [_.uniq(entityIds)] + [_.uniq(entityIds)], + knex ) ).select( pl.col("entityId").cast(pl.Int32), @@ -351,7 +261,8 @@ const emptyDataDF = (): pl.DataFrame => { } export const _dataAsDFfromS3 = async ( - variableIds: OwidVariableId[] + variableIds: OwidVariableId[], + knex: Knex ): Promise => { if (variableIds.length === 0) { return emptyDataDF() @@ -388,15 +299,19 @@ export const _dataAsDFfromS3 = async ( return emptyDataDF() } - const entityDF = await entitiesAsDF(df.getColumn("entityId").toArray()) + const entityDF = await entitiesAsDF( + df.getColumn("entityId").toArray(), + knex + ) return _castDataDF(df.join(entityDF, { on: "entityId" })) } export const dataAsDF = async ( - variableIds: OwidVariableId[] + variableIds: OwidVariableId[], + knex: Knex ): Promise => { - return _dataAsDFfromS3(variableIds) + return _dataAsDFfromS3(variableIds, knex) } export const fetchS3Values = async ( @@ -465,9 +380,10 @@ export const createDataFrame = (data: unknown): pl.DataFrame => { export const readSQLasDF = async ( sql: string, - params: any[] + params: any[], + knex: Knex ): Promise => { - return createDataFrame(await db.queryMysql(sql, params)) + return createDataFrame(await knexRaw(sql, knex, params)) } /** @@ -475,7 +391,8 @@ export const readSQLasDF = async ( */ export const searchVariables = async ( query: string, - limit: number + limit: number, + knex: Knex ): Promise => { const whereClauses = buildWhereClauses(query) @@ -505,9 +422,9 @@ export const searchVariables = async ( ORDER BY d.dataEditedAt DESC LIMIT ${escape(limit)} ` - const rows = await queryRegexSafe(sqlResults) + const rows = await queryRegexSafe(sqlResults, knex) - const numTotalRows = await queryRegexCount(sqlCount) + const numTotalRows = await queryRegexCount(sqlCount, knex) rows.forEach((row: any) => { if (row.catalogPath) { @@ -627,9 +544,12 @@ const buildWhereClauses = (query: string): string[] => { * This is useful if the regex is user-supplied and we want them to be able * to construct it incrementally. */ -const queryRegexSafe = async (query: string): Promise => { +const queryRegexSafe = async ( + query: string, + knex: Knex +): Promise => { // catch regular expression failures in MySQL and return empty result - return await db.queryMysql(query).catch((err) => { + return await knexRaw(query, knex).catch((err) => { if (err.message.includes("regular expression")) { return [] } @@ -637,8 +557,11 @@ const queryRegexSafe = async (query: string): Promise => { }) } -const queryRegexCount = async (query: string): Promise => { - const results = await queryRegexSafe(query) +const queryRegexCount = async ( + query: string, + knex: Knex +): Promise => { + const results = await queryRegexSafe(query, knex) if (!results.length) { return 0 } diff --git a/db/tests/basic.test.ts b/db/tests/basic.test.ts index a048075e89a..1b8465e252b 100644 --- a/db/tests/basic.test.ts +++ b/db/tests/basic.test.ts @@ -3,10 +3,11 @@ import sqlFixtures from "sql-fixtures" import { dbTestConfig } from "./dbTestConfig.js" import { dataSource } from "./dataSource.dbtests.js" import { knex, Knex } from "knex" -import { getConnection } from "../db.js" +import { getConnection, knexRaw } from "../db.js" import { DataSource } from "typeorm" -import { User } from "../model/User.js" +import { insertUser, updateUser, User } from "../model/User.js" import { Chart } from "../model/Chart.js" +import { DbPlainUser, UsersTableName } from "@ourworldindata/types" let knexInstance: Knex | undefined = undefined let typeOrmConnection: DataSource | undefined = undefined @@ -91,6 +92,100 @@ test("timestamps are automatically created and updated", async () => { test("knex interface", async () => { if (!knexInstance) throw new Error("Knex connection not initialized") - // const result = await knexInstance< - // expect(result.rows[0].solution).toBe(2) + + // Create a transaction and run all tests inside it + knexInstance.transaction(async (trx) => { + // Fetch all users into memory + const users = await trx + .from(UsersTableName) + .select("isSuperuser", "email") + expect(users.length).toBe(1) + + // Fetch all users in a streaming fashion, iterate over them async to avoid having to load everything into memory + const usersStream = trx + .from(UsersTableName) + .select("isSuperuser", "email") + .stream() + + for await (const user of usersStream) { + expect(user.isSuperuser).toBe(0) + expect(user.email).toBe("admin@example.com") + } + + // Use the insert helper method + await insertUser(trx, { + email: "test@example.com", + fullName: "Test User", + }) + + // Use the update helper method + await updateUser(trx, 2, { isSuperuser: 1 }) + + // Check results after update and insert + const afterUpdate = await trx + .from(UsersTableName) + .select("isSuperuser", "email") + .orderBy("id") + 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( + "select email from ??", + trx, + [UsersTableName] + ) + expect(usersFromRawQuery.length).toBe(2) + }) +}) + +test("knex interface raw", async () => { + if (!knexInstance) throw new Error("Knex connection not initialized") + + // Create a transaction and run all tests inside it + knexInstance.transaction(async (trx) => { + // Fetch all users into memory + const users = await trx + .from(UsersTableName) + .select("isSuperuser", "email") + expect(users.length).toBe(1) + + // Fetch all users in a streaming fashion, iterate over them async to avoid having to load everything into memory + const usersStream = trx + .from(UsersTableName) + .select("isSuperuser", "email") + .stream() + + for await (const user of usersStream) { + expect(user.isSuperuser).toBe(0) + expect(user.email).toBe("admin@example.com") + } + + // Use the insert helper method + await insertUser(trx, { + email: "test@example.com", + fullName: "Test User", + }) + + // Use the update helper method + await updateUser(trx, 2, { isSuperuser: 1 }) + + // Check results after update and insert + const afterUpdate = await trx + .from(UsersTableName) + .select("isSuperuser", "email") + .orderBy("id") + 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( + "select email from ??", + trx, + [UsersTableName] + ) + expect(usersFromRawQuery.length).toBe(2) + }) }) diff --git a/packages/@ourworldindata/types/src/dbTypes/Charts.ts b/packages/@ourworldindata/types/src/dbTypes/Charts.ts index cecb8ba6b02..ae7c39fca7f 100644 --- a/packages/@ourworldindata/types/src/dbTypes/Charts.ts +++ b/packages/@ourworldindata/types/src/dbTypes/Charts.ts @@ -30,6 +30,18 @@ export function serializeChartConfig(config: GrapherInterface): JsonString { return JSON.stringify(config) } +export function parseNullableChartConfig( + config: JsonString | null +): GrapherInterface | null { + return config ? JSON.parse(config) : null +} + +export function serializeNullableChartConfig( + config: GrapherInterface | null +): JsonString | null { + return config ? JSON.stringify(config) : null +} + export function parseChartsRow(row: DbRawChart): DbEnrichedChart { return { ...row, config: parseChartConfig(row.config) } } diff --git a/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts b/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts index 5248fea42c3..90e7dcc1fc7 100644 --- a/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts +++ b/packages/@ourworldindata/types/src/dbTypes/SuggestedChartRevisions.ts @@ -1,17 +1,11 @@ +import { + SuggestedChartRevisionStatus, + SuggestedChartRevisionsExperimental, +} from "../domainTypes/SuggestedChartRevisions.js" import { JsonString } from "../domainTypes/Various.js" import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" import { parseChartConfig, serializeChartConfig } from "./Charts.js" -export interface SuggestedChartRevisionsExperimental { - gpt: { - model: string - suggestions: { - title: string - subtitle: string - }[] - } -} - export const SuggestedChartRevisionsTableName = "suggested_chart_revisions" export interface DbInsertSuggestedChartRevision { changesInDataSummary?: string | null @@ -24,7 +18,7 @@ export interface DbInsertSuggestedChartRevision { isPendingOrFlagged?: number | null originalConfig: JsonString originalVersion: number - status: string + status: SuggestedChartRevisionStatus suggestedConfig: JsonString suggestedReason?: string | null suggestedVersion: number @@ -80,3 +74,62 @@ export function serializeSuggestedChartRevisionsRow( ), } } + +export interface SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw { + chartId: number + createdAt: Date + createdBy: number + decisionReason: string | null + experimental: JsonString | null + id: number + originalConfig: JsonString + status: SuggestedChartRevisionStatus + suggestedConfig: JsonString + suggestedReason: string | null + suggestedVersion: number + updatedAt: Date | null + updatedBy: number | null + createdByFullName: string | null + updatedByFullName: string | null + existingConfig: JsonString + chartCreatedAt: Date + chartUpdatedAt: Date | null +} + +export type SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched = Omit< + SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw, + "originalConfig" | "suggestedConfig" | "experimental" | "existingConfig" +> & { + originalConfig: GrapherInterface + existingConfig: GrapherInterface + suggestedConfig: GrapherInterface + experimental: SuggestedChartRevisionsExperimental | null +} + +export function parseSuggestedChartRevisionsRowWithUsersAndExistingConfig( + row: SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw +): SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched { + return { + ...row, + originalConfig: parseChartConfig(row.originalConfig), + existingConfig: parseChartConfig(row.existingConfig), + suggestedConfig: parseChartConfig(row.suggestedConfig), + experimental: parseSuggestedChartRevisionsExperimental( + row.experimental + ), + } +} + +export function serializeSuggestedChartRevisionsRowWithUsersAndExistingConfig( + row: SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched +): SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw { + return { + ...row, + originalConfig: serializeChartConfig(row.originalConfig), + existingConfig: serializeChartConfig(row.existingConfig), + suggestedConfig: serializeChartConfig(row.suggestedConfig), + experimental: serializeSuggestedChartRevisionsExperimental( + row.experimental + ), + } +} diff --git a/packages/@ourworldindata/types/src/dbTypes/Variables.ts b/packages/@ourworldindata/types/src/dbTypes/Variables.ts index 7f0fd857b61..fd12fcaf44f 100644 --- a/packages/@ourworldindata/types/src/dbTypes/Variables.ts +++ b/packages/@ourworldindata/types/src/dbTypes/Variables.ts @@ -1,6 +1,11 @@ import { OwidVariableDisplayConfigInterface } from "../OwidVariableDisplayConfigInterface.js" import { JsonString } from "../domainTypes/Various.js" import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" +import { + parseChartConfig, + parseNullableChartConfig, + serializeNullableChartConfig, +} from "./Charts.js" export const VariablesTableName = "variables" export interface DbInsertVariable { @@ -143,30 +148,6 @@ export function serializeVariableOriginalMetadata( return originalMetadata ? JSON.stringify(originalMetadata) : null } -export function parseVariableGrapherConfigAdmin( - grapherConfigAdmin: JsonString | null -): GrapherInterface { - return grapherConfigAdmin ? JSON.parse(grapherConfigAdmin) : null -} - -export function serializeVariableGrapherConfigAdmin( - grapherConfigAdmin: GrapherInterface | null -): JsonString | null { - return grapherConfigAdmin ? JSON.stringify(grapherConfigAdmin) : null -} - -export function parseVariableGrapherConfigETL( - grapherConfigETL: JsonString | null -): GrapherInterface { - return grapherConfigETL ? JSON.parse(grapherConfigETL) : null -} - -export function serializeVariableGrapherConfigETL( - grapherConfigETL: GrapherInterface | null -): JsonString | null { - return grapherConfigETL ? JSON.stringify(grapherConfigETL) : null -} - export function parseVariableProcessingLog( processingLog: JsonString | null ): any { @@ -188,10 +169,8 @@ export function parseVariablesRow(row: DbRawVariable): DbEnrichedVariable { dimensions: parseVariableDimensions(row.dimensions), descriptionKey: parseVariableDescriptionKey(row.descriptionKey), originalMetadata: parseVariableOriginalMetadata(row.originalMetadata), - grapherConfigAdmin: parseVariableGrapherConfigAdmin( - row.grapherConfigAdmin - ), - grapherConfigETL: parseVariableGrapherConfigETL(row.grapherConfigETL), + grapherConfigAdmin: parseNullableChartConfig(row.grapherConfigAdmin), + grapherConfigETL: parseNullableChartConfig(row.grapherConfigETL), processingLog: parseVariableProcessingLog(row.processingLog), } } @@ -207,12 +186,10 @@ export function serializeVariablesRow(row: DbEnrichedVariable): DbRawVariable { originalMetadata: serializeVariableOriginalMetadata( row.originalMetadata ), - grapherConfigAdmin: serializeVariableGrapherConfigAdmin( + grapherConfigAdmin: serializeNullableChartConfig( row.grapherConfigAdmin ), - grapherConfigETL: serializeVariableGrapherConfigETL( - row.grapherConfigETL - ), + grapherConfigETL: serializeNullableChartConfig(row.grapherConfigETL), processingLog: serializeVariableProcessingLog(row.processingLog), } } diff --git a/packages/@ourworldindata/types/src/domainTypes/SuggestedChartRevisions.ts b/packages/@ourworldindata/types/src/domainTypes/SuggestedChartRevisions.ts new file mode 100644 index 00000000000..a08d34dda02 --- /dev/null +++ b/packages/@ourworldindata/types/src/domainTypes/SuggestedChartRevisions.ts @@ -0,0 +1,45 @@ +import { GrapherInterface } from "../grapherTypes/GrapherTypes.js" + +export enum SuggestedChartRevisionStatus { + pending = "pending", + approved = "approved", + rejected = "rejected", + flagged = "flagged", +} + +export interface SuggestedChartRevisionsExperimental { + gpt?: { + model?: string + suggestions?: { + title?: string + subtitle?: string + }[] + } +} + +export interface SuggestedChartRevision { + canApprove?: boolean + canFlag?: boolean + canPending?: boolean + canReject?: boolean + changesInDataSummary?: string | null + chartCreatedAt: Date + chartId: number + chartUpdatedAt?: Date | null + createdAt?: Date + createdBy: number + createdByFullName: string + decisionReason?: string | null + existingConfig: GrapherInterface + experimental?: SuggestedChartRevisionsExperimental | null + id?: number + isPendingOrFlagged?: number | null + originalConfig: GrapherInterface + status: SuggestedChartRevisionStatus + suggestedConfig: GrapherInterface + suggestedReason?: string | null + suggestedVersion: number + updatedAt?: Date | null + updatedBy?: number | null + updatedByFullName?: string | null +} diff --git a/packages/@ourworldindata/types/src/domainTypes/Various.ts b/packages/@ourworldindata/types/src/domainTypes/Various.ts index 0f491eff454..946339baa14 100644 --- a/packages/@ourworldindata/types/src/domainTypes/Various.ts +++ b/packages/@ourworldindata/types/src/domainTypes/Various.ts @@ -53,13 +53,6 @@ export enum TaggableType { export type TopicId = number -export enum SuggestedChartRevisionStatus { - pending = "pending", - approved = "approved", - rejected = "rejected", - flagged = "flagged", -} - // Exception format that can be easily given as an API error export class JsonError extends Error { status: number diff --git a/packages/@ourworldindata/types/src/index.ts b/packages/@ourworldindata/types/src/index.ts index bc64d838285..a13dc931ea9 100644 --- a/packages/@ourworldindata/types/src/index.ts +++ b/packages/@ourworldindata/types/src/index.ts @@ -12,7 +12,6 @@ export { JsonError, type SerializedGridProgram, SiteFooterContext, - SuggestedChartRevisionStatus, TaggableType, type TopicId, type OwidVariableId, @@ -20,6 +19,11 @@ export { type UserCountryInformation, type QueryParams, } from "./domainTypes/Various.js" +export { + SuggestedChartRevisionStatus, + type SuggestedChartRevision, + type SuggestedChartRevisionsExperimental, +} from "./domainTypes/SuggestedChartRevisions.js" export { type DataValueConfiguration, type DataValueProps, @@ -406,6 +410,8 @@ export { ChartsTableName, parseChartConfig, serializeChartConfig, + parseNullableChartConfig, + serializeNullableChartConfig, parseChartsRow, serializeChartsRow, } from "./dbTypes/Charts.js" @@ -562,6 +568,10 @@ export { serializeSuggestedChartRevisionsExperimental, parseSuggestedChartRevisionsRow, serializeSuggestedChartRevisionsRow, + type SuggestedChartRevisionsRowWithUsersAndExistingConfigRaw, + type SuggestedChartRevisionsRowWithUsersAndExistingConfigEnriched, + parseSuggestedChartRevisionsRowWithUsersAndExistingConfig, + serializeSuggestedChartRevisionsRowWithUsersAndExistingConfig, } from "./dbTypes/SuggestedChartRevisions.js" export { type DbInsertTag, @@ -595,10 +605,6 @@ export { serializeVariableOriginalMetadata, parseVariableLicenses, serializeVariableLicenses, - parseVariableGrapherConfigAdmin, - serializeVariableGrapherConfigAdmin, - parseVariableGrapherConfigETL, - serializeVariableGrapherConfigETL, parseVariableProcessingLog, serializeVariableProcessingLog, type License,