Skip to content

Commit

Permalink
🔨 incorporate PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@danyx23
danyx23 committed Mar 20, 2024
1 parent 0055d9c commit bf7385a
Show file tree
Hide file tree
Showing 16 changed files with 315 additions and 319 deletions.
7 changes: 5 additions & 2 deletions .vscode/settings.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,8 @@
"javascript.preferences.importModuleSpecifierEnding": "js",
"[sql]": {
"editor.defaultFormatter": "inferrinizzard.prettier-sql-vscode"
}
}
},
"Prettier-SQL.keywordCase": "upper",
"Prettier-SQL.SQLFlavourOverride": "mysql",
"Prettier-SQL.expressionWidth": 80
}
6 changes: 3 additions & 3 deletions adminSiteClient/gdocsDeploy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ export const checkFullDeployFallback = (
*
*/
export const checkIsLightningUpdate = (
prevGdoc: DbEnrichedPostGdoc,
nextGdoc: DbEnrichedPostGdoc,
prevGdoc: OwidGdoc,
nextGdoc: OwidGdoc,
hasChanges: boolean
) => {
if (
Expand Down Expand Up @@ -59,6 +59,7 @@ export const checkIsLightningUpdate = (
relatedCharts: true,
revisionId: true,
updatedAt: true,
markdown: true,
createdAt: false, // weird case - can't be updated
id: false, // weird case - can't be updated
tags: false, // could require updating datapages, though it's currently not possible to have a difference between prevGdoc.tags and nextGdoc.tags
Expand All @@ -67,7 +68,6 @@ export const checkIsLightningUpdate = (
published: false, // requires an update of the blog roll
publishedAt: false, // could require an update of the blog roll
slug: false, // requires updating any articles that link to it
markdown: true,
}

const postlightningPropContentConfigMap: Record<
Expand Down
198 changes: 121 additions & 77 deletions adminSiteServer/apiRouter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,15 @@ import {
} from "./routerHelpers.js"
import { getPublishedLinksTo } from "../db/model/Link.js"
import {
GdocLinkUpdateMode,
createGdocAndInsertIntoDb,
gdocFromJSON,
getAllGdocIndexItemsOrderedByUpdatedAt,
getAndLoadGdocById,
getDbEnrichedGdocFromOwidGdoc,

Check warning on line 132 in adminSiteServer/apiRouter.ts

View workflow job for this annotation

GitHub Actions / eslint 

'getDbEnrichedGdocFromOwidGdoc' is defined but never used

Check warning on line 132 in adminSiteServer/apiRouter.ts

View workflow job for this annotation

GitHub Actions / eslint 

'getDbEnrichedGdocFromOwidGdoc' is defined but never used
getGdocBaseObjectById,
loadGdocFromGdocBase,
setLinksForGdoc,
setTagsForGdoc,
updateGdocContentOnly,
upsertGdoc,
Expand Down Expand Up @@ -215,7 +217,12 @@ const getReferencesByChartId = async (
)
const postGdocsPromise = getGdocsPostReferencesByChartId(chartId, knex)
const explorerSlugsPromise = db.queryMysql(
`select distinct explorerSlug from explorer_charts where chartId = ?`,
`SELECT DISTINCT
explorerSlug
FROM
explorer_charts
WHERE
chartId = ?`,
[chartId]
)
const [postsWordpress, postsGdocs, explorerSlugs] = await Promise.all([
Expand Down Expand Up @@ -554,7 +561,12 @@ getRouteWithROTransaction(

const pageviewsByUrl = await db.knexRawFirst(
trx,
"select * from analytics_pageviews where url = ?",
`-- sql
SELECT *
FROM
analytics_pageviews
WHERE
url = ?`,
[`https://ourworldindata.org/grapher/${slug}`]
)

Expand Down Expand Up @@ -1612,10 +1624,16 @@ apiRouter.patch("/variable-annotations", async (req) => {
})

apiRouter.get("/variables.usages.json", async (req) => {
const query = `SELECT variableId, COUNT(DISTINCT chartId) AS usageCount
FROM chart_dimensions
GROUP BY variableId
ORDER BY usageCount DESC`
const query = `-- sql
SELECT
variableId,
COUNT(DISTINCT chartId) AS usageCount
FROM
chart_dimensions
GROUP BY
variableId
ORDER BY
usageCount DESC`

const rows = await db.queryMysql(query)

Expand All @@ -1642,7 +1660,7 @@ getRouteWithROTransaction(

const charts = await db.knexRaw<OldChartFieldList>(
trx,
`
`-- sql
SELECT ${oldChartFieldList}
FROM charts
JOIN users lastEditedByUser ON lastEditedByUser.id = charts.lastEditedByUserId
Expand Down Expand Up @@ -1695,7 +1713,7 @@ getRouteWithROTransaction(
async (req, res, trx) => {
const datasets = await db.knexRaw<Record<string, any>>(
trx,
`
`-- sql
WITH variable_counts AS (
SELECT
v.datasetId,
Expand Down Expand Up @@ -1732,7 +1750,7 @@ getRouteWithROTransaction(
Pick<DbPlainDatasetTag, "datasetId">
>(
trx,
`
`-- sql
SELECT dt.datasetId, t.id, t.name FROM dataset_tags dt
JOIN tags t ON dt.tagId = t.id
`
Expand All @@ -1758,7 +1776,7 @@ getRouteWithROTransaction(

const dataset = await db.knexRawFirst<Record<string, any>>(
trx,
`
`-- sql
SELECT d.id,
d.namespace,
d.name,
Expand Down Expand Up @@ -1801,10 +1819,17 @@ getRouteWithROTransaction(
>
>(
trx,
`
SELECT v.id, v.name, v.description, v.display, v.catalogPath
FROM variables AS v
WHERE v.datasetId = ?
`-- sql
SELECT
v.id,
v.name,
v.description,
v.display,
v.catalogPath
FROM
VARIABLES AS v
WHERE
v.datasetId = ?
`,
[datasetId]
)
Expand All @@ -1818,13 +1843,15 @@ getRouteWithROTransaction(
// add all origins
const origins: DbRawOrigin[] = await db.knexRaw<DbRawOrigin>(
trx,
`
select distinct
o.*
from origins_variables as ov
join origins as o on ov.originId = o.id
join variables as v on ov.variableId = v.id
where v.datasetId = ?
`-- sql
SELECT DISTINCT
o.*
FROM
origins_variables AS ov
JOIN origins AS o ON ov.originId = o.id
JOIN VARIABLES AS v ON ov.variableId = v.id
WHERE
v.datasetId = ?
`,
[datasetId]
)
Expand Down Expand Up @@ -1859,7 +1886,7 @@ getRouteWithROTransaction(

const charts = await db.knexRaw<OldChartFieldList>(
trx,
`
`-- sql
SELECT ${oldChartFieldList}
FROM charts
JOIN chart_dimensions AS cd ON cd.chartId = charts.id
Expand Down Expand Up @@ -2349,37 +2376,59 @@ deleteRouteWithRWTransaction(
apiRouter.get("/posts.json", async (req) => {
const raw_rows = await db.queryMysql(
`-- sql
with posts_tags_aggregated as (
select post_id, if(count(tags.id) = 0, json_array(), json_arrayagg(json_object("id", tags.id, "name", tags.name))) as tags
from post_tags
left join tags on tags.id = post_tags.tag_id
group by post_id
), post_gdoc_slug_successors as (
select posts.id, if (count(gdocSlugSuccessor.id) = 0, json_array(), json_arrayagg(json_object("id", gdocSlugSuccessor.id, "published", gdocSlugSuccessor.published ))) as gdocSlugSuccessors
from posts
left join posts_gdocs gdocSlugSuccessor on gdocSlugSuccessor.slug = posts.slug
group by posts.id
)
select
posts.id as id,
posts.title as title,
posts.type as type,
posts.slug as slug,
status,
updated_at_in_wordpress,
posts.authors,
posts_tags_aggregated.tags as tags,
gdocSuccessorId,
gdocSuccessor.published as isGdocSuccessorPublished,
-- posts can either have explict successors via the gdocSuccessorId column
-- or implicit successors if a gdoc has been created that uses the same slug
-- as a Wp post (the gdoc one wins once it is published)
post_gdoc_slug_successors.gdocSlugSuccessors as gdocSlugSuccessors
from posts
left join post_gdoc_slug_successors on post_gdoc_slug_successors.id = posts.id
left join posts_gdocs gdocSuccessor on gdocSuccessor.id = posts.gdocSuccessorId
left join posts_tags_aggregated on posts_tags_aggregated.post_id = posts.id
order by updated_at_in_wordpress desc`,
WITH
posts_tags_aggregated AS (
SELECT
post_id,
IF(
COUNT(tags.id) = 0,
JSON_ARRAY(),
JSON_ARRAYAGG(JSON_OBJECT("id", tags.id, "name", tags.name))
) AS tags
FROM
post_tags
LEFT JOIN tags ON tags.id = post_tags.tag_id
GROUP BY
post_id
),
post_gdoc_slug_successors AS (
SELECT
posts.id,
IF(
COUNT(gdocSlugSuccessor.id) = 0,
JSON_ARRAY(),
JSON_ARRAYAGG(
JSON_OBJECT("id", gdocSlugSuccessor.id, "published", gdocSlugSuccessor.published)
)
) AS gdocSlugSuccessors
FROM
posts
LEFT JOIN posts_gdocs gdocSlugSuccessor ON gdocSlugSuccessor.slug = posts.slug
GROUP BY
posts.id
)
SELECT
posts.id AS id,
posts.title AS title,
posts.type AS TYPE,
posts.slug AS slug,
STATUS,
updated_at_in_wordpress,
posts.authors,
posts_tags_aggregated.tags AS tags,
gdocSuccessorId,
gdocSuccessor.published AS isGdocSuccessorPublished,
-- posts can either have explict successors via the gdocSuccessorId column
-- or implicit successors if a gdoc has been created that uses the same slug
-- as a Wp post (the gdoc one wins once it is published)
post_gdoc_slug_successors.gdocSlugSuccessors AS gdocSlugSuccessors
FROM
posts
LEFT JOIN post_gdoc_slug_successors ON post_gdoc_slug_successors.id = posts.id
LEFT JOIN posts_gdocs gdocSuccessor ON gdocSuccessor.id = posts.gdocSuccessorId
LEFT JOIN posts_tags_aggregated ON posts_tags_aggregated.post_id = posts.id
ORDER BY
updated_at_in_wordpress DESC`,
[]
)
const rows = raw_rows.map((row: any) => ({
Expand Down Expand Up @@ -2553,7 +2602,7 @@ apiRouter.put("/deploy", async (req, res) => {
triggerStaticBuild(res.locals.user, "Manually triggered deploy")
})

getRouteWithROTransaction(apiRouter, "/gdocs", async (req, res, trx) => {
getRouteWithROTransaction(apiRouter, "/gdocs", (req, res, trx) => {
return getAllGdocIndexItemsOrderedByUpdatedAt(trx)
})

Expand All @@ -2570,12 +2619,7 @@ getRouteNonIdempotentWithRWTransaction(
const gdoc = await getAndLoadGdocById(trx, id, contentSource)

if (!gdoc.published) {
await updateGdocContentOnly(
trx,
id,
gdoc,
gdoc.enrichedBlockSources.flat()
)
await updateGdocContentOnly(trx, id, gdoc)
}

res.set("Cache-Control", "no-store")
Expand Down Expand Up @@ -2651,15 +2695,14 @@ putRouteWithRWTransaction(apiRouter, "/gdocs/:id", async (req, res, trx) => {
}
}

await trx.table(PostsGdocsLinksTableName).where({ sourceId: id }).delete()

if (
nextGdoc.published &&
nextGdoc.links !== undefined &&
nextGdoc.links.length > 0
) {
await trx.table(PostsGdocsLinksTableName).insert(nextGdoc.links)
}
await setLinksForGdoc(
trx,
nextGdoc.id,
nextGdoc.links,
nextGdoc.published
? GdocLinkUpdateMode.DeleteAndInsert
: GdocLinkUpdateMode.DeleteOnly
)

//todo #gdocsvalidationserver: run validation before saving published
//articles, in addition to the first pass performed in front-end code (see
Expand All @@ -2681,8 +2724,8 @@ putRouteWithRWTransaction(apiRouter, "/gdocs/:id", async (req, res, trx) => {
await upsertGdoc(trx, nextGdoc)

const hasChanges = checkHasChanges(prevGdoc, nextGdoc)
const prevJson = getDbEnrichedGdocFromOwidGdoc(prevGdoc)
const nextJson = getDbEnrichedGdocFromOwidGdoc(nextGdoc)
const prevJson = prevGdoc.toJSON()
const nextJson = nextGdoc.toJSON()
if (checkIsLightningUpdate(prevJson, nextJson, hasChanges)) {
await enqueueLightningChange(
res.locals.user,
Expand Down Expand Up @@ -2727,12 +2770,13 @@ postRouteWithRWTransaction(
const { gdocId } = req.params
const { tagIds } = req.body

await trx.table(PostsGdocsXTagsTableName).where({ gdocId }).delete()
if (tagIds.length) {
await trx
.table(PostsGdocsXTagsTableName)
.insert(tagIds.map((tagId: number) => ({ gdocId, tagId })))
}
await setTagsForGdoc(
trx,
gdocId,
tagIds.map((id: number) => {
id
})
)

return { success: true }
}
Expand Down
2 changes: 1 addition & 1 deletion adminSiteServer/mockSiteRouter.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ mockSiteRouter.get("/thank-you", async (req, res) =>
)

mockSiteRouter.get("/data-insights/:pageNumberOrSlug?", async (req, res) => {
return await db.knexReadonlyTransaction(async (knex) => {
return db.knexReadonlyTransaction(async (knex) => {
const totalPageCount = calculateDataInsightIndexPageCount(
await db
.getPublishedDataInsights(knex)
Expand Down
Loading

0 comments on commit bf7385a

Please sign in to comment.