Skip to content

Commit

Permalink
🔨 incorporate PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
danyx23 committed Jan 9, 2025
1 parent 50fb028 commit 3cc565e
Show file tree
Hide file tree
Showing 22 changed files with 36 additions and 117 deletions.
60 changes: 0 additions & 60 deletions adminSiteServer/FunctionalRouter.ts

This file was deleted.

13 changes: 8 additions & 5 deletions adminSiteServer/apiRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ import {
deleteVariablesVariableIdGrapherConfigAdmin,
getVariablesVariableIdChartsJson,
} from "./apiRoutes/variables.js"
import { FunctionalRouter } from "./FunctionalRouter.js"
import {
patchRouteWithRWTransaction,
getRouteWithROTransaction,
Expand All @@ -123,8 +122,9 @@ import {
updateChart,
deleteChart,
} from "./apiRoutes/charts.js"
import e, { Router } from "express"

const apiRouter = new FunctionalRouter()
const apiRouter = Router()

// Bulk chart update routes
patchRouteWithRWTransaction(
Expand Down Expand Up @@ -428,8 +428,11 @@ apiRouter.get("/deploys.json", async () => ({
deploys: await new DeployQueueServer().getDeploys(),
}))

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.

apiRouter.put("/deploy", async (req, res) => {
return triggerStaticBuild(res.locals.user, "Manually triggered deploy")
})
apiRouter.put(
"/deploy",
async (_req: e.Request, res: e.Response<any, Record<string, any>>) => {
return triggerStaticBuild(res.locals.user, "Manually triggered deploy")
}
)

export { apiRouter }
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/bulkUpdates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@ import {
updateGrapherConfigAdminOfVariable,
} from "../../db/model/Variable.js"
import { saveGrapher } from "./charts.js"
import e, { Request } from "express"
import * as db from "../../db/db.js"
import * as lodash from "lodash"
import { Request } from "../authentication.js"
import e from "express"

export async function getChartBulkUpdate(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/chartViews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ import { deleteGrapherConfigFromR2ByUUID } from "../chartConfigR2Helpers.js"

import * as db from "../../db/db.js"
import { expectChartById } from "./charts.js"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"
const createPatchConfigAndQueryParamsForChartView = async (
knex: db.KnexReadonlyTransaction,
parentChartId: number,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/charts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ import * as db from "../../db/db.js"
import { getLogsByChartId } from "../getLogsByChartId.js"
import { getPublishedLinksTo } from "../../db/model/Link.js"

import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"
export const getReferencesByChartId = async (
chartId: number,
knex: db.KnexReadonlyTransaction
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/datasets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ import {
import { triggerStaticBuild } from "./routeUtils.js"
import * as db from "../../db/db.js"
import * as lodash from "lodash"
import { Request } from "express"
import * as e from "express"
import e, { Request } from "express"

export async function getDatasets(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/explorer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { JsonError } from "@ourworldindata/types"
import { Request } from "express"
import * as e from "express"
import e, { Request } from "express"

import * as db from "../../db/db.js"
export async function addExplorerTags(
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/gdocs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ import { GdocPost } from "../../db/model/Gdoc/GdocPost.js"
import { triggerStaticBuild, enqueueLightningChange } from "./routeUtils.js"
import * as db from "../../db/db.js"
import * as lodash from "lodash"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"

export async function getAllGdocIndexItems(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { triggerStaticBuild } from "./routeUtils.js"
import * as db from "../../db/db.js"
import * as lodash from "lodash"

import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"
export async function getImagesHandler(
_: Request,
res: e.Response<any, Record<string, any>>,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/mdims.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ import {
} from "../../settings/clientSettings.js"
import { createMultiDimConfig } from "../multiDim.js"
import { triggerStaticBuild } from "./routeUtils.js"
import { Request } from "../authentication.js"
import * as db from "../../db/db.js"
import e from "express"
import e, { Request } from "express"

export async function handleMultiDimDataPageRequest(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import * as db from "../../db/db.js"
import * as lodash from "lodash"
import path from "path"
import { expectInt } from "../../serverUtils/serverUtil.js"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"
// using the alternate template, which highlights topics rather than articles.
export async function fetchAllWork(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/posts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import { GdocPost } from "../../db/model/Gdoc/GdocPost.js"
import { setTagsForPost, getTagsByPostId } from "../../db/model/Post.js"
import { expectInt } from "../../serverUtils/serverUtil.js"
import * as db from "../../db/db.js"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"
export async function handleGetPostsJson(
req: Request,
_res: e.Response<any, Record<string, any>>,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/redirects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import {
import { expectInt } from "../../serverUtils/serverUtil.js"
import { triggerStaticBuild } from "./routeUtils.js"
import * as db from "../../db/db.js"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"
export async function handleGetSiteRedirects(
req: Request,
res: e.Response<any, Record<string, any>>,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/suggest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import { getGptTopicSuggestions } from "../../db/model/Chart.js"
import { CLOUDFLARE_IMAGES_URL } from "../../settings/clientSettings.js"
import { fetchGptGeneratedAltText } from "../imagesHelpers.js"
import * as db from "../../db/db.js"
import e from "express"
import { Request } from "../authentication.js"
import e, { Request } from "express"

export async function suggestGptTopics(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/tagGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { JsonError, FlatTagGraph } from "@ourworldindata/types"
import { checkIsPlainObjectWithGuard } from "@ourworldindata/utils"
import * as db from "../../db/db.js"
import * as lodash from "lodash"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"

export async function handleGetFlatTagGraph(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/tags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import { expectInt } from "../../serverUtils/serverUtil.js"
import { UNCATEGORIZED_TAG_ID } from "../../settings/serverSettings.js"
import * as db from "../../db/db.js"
import * as lodash from "lodash"
import e from "express"
import { Request } from "../authentication.js"
import e, { Request } from "express"

export async function getTagById(
req: Request,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import { pick } from "lodash"
import { getUserById, updateUser, insertUser } from "../../db/model/User.js"
import { expectInt } from "../../serverUtils/serverUtil.js"
import * as db from "../../db/db.js"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"
export async function getUsers(
req: Request,
_res: e.Response<any, Record<string, any>>,
Expand Down
3 changes: 1 addition & 2 deletions adminSiteServer/apiRoutes/variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ import { expectInt } from "../../serverUtils/serverUtil.js"
import { triggerStaticBuild } from "./routeUtils.js"
import * as lodash from "lodash"
import { updateGrapherConfigsInR2 } from "./charts.js"
import { Request } from "../authentication.js"
import e from "express"
import e, { Request } from "express"

export async function getEditorVariablesJson(
req: Request,
Expand Down
4 changes: 2 additions & 2 deletions adminSiteServer/appClass.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ export class OwidAdminApp {
app.use("/fonts", express.static("public/fonts"))
app.use("/assets-admin", express.static("dist/assets-admin"))

app.use("/api", publicApiRouter.router)
app.use("/admin/api", apiRouter.router)
app.use("/api", publicApiRouter)
app.use("/admin/api", apiRouter)
app.use("/admin/test", testPageRouter)
app.use("/admin/storybook", express.static(".storybook/build"))
app.use("/admin", adminRouter)
Expand Down
6 changes: 0 additions & 6 deletions adminSiteServer/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ import { Secret, verify } from "jsonwebtoken"
import { DbPlainSession, DbPlainUser, JsonError } from "@ourworldindata/utils"
import { exec } from "child_process"

export type Request = express.Request

export interface Response extends express.Response {
locals: { user: DbPlainUser; session: Session }
}

interface Session {
id: string
expiryDate: Date
Expand Down
15 changes: 7 additions & 8 deletions adminSiteServer/functionalRouterHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { FunctionalRouter } from "./FunctionalRouter.js"
import { Request, Response } from "express"
import { Request, Response, Router } from "express"
import * as db from "../db/db.js"
export function getRouteWithROTransaction<T>(
router: FunctionalRouter,
router: Router,
targetPath: string,
handler: (
req: Request,
Expand All @@ -22,7 +21,7 @@ export function getRouteWithROTransaction<T>(
fetching it from the google API.
*/
export function getRouteNonIdempotentWithRWTransaction<T>(
router: FunctionalRouter,
router: Router,
targetPath: string,
handler: (
req: Request,
Expand All @@ -38,7 +37,7 @@ export function getRouteNonIdempotentWithRWTransaction<T>(
}

export function postRouteWithRWTransaction<T>(
router: FunctionalRouter,
router: Router,
targetPath: string,
handler: (
req: Request,
Expand All @@ -54,7 +53,7 @@ export function postRouteWithRWTransaction<T>(
}

export function putRouteWithRWTransaction<T>(
router: FunctionalRouter,
router: Router,
targetPath: string,
handler: (
req: Request,
Expand All @@ -70,7 +69,7 @@ export function putRouteWithRWTransaction<T>(
}

export function patchRouteWithRWTransaction<T>(
router: FunctionalRouter,
router: Router,
targetPath: string,
handler: (
req: Request,
Expand All @@ -86,7 +85,7 @@ export function patchRouteWithRWTransaction<T>(
}

export function deleteRouteWithRWTransaction<T>(
router: FunctionalRouter,
router: Router,
targetPath: string,
handler: (
req: Request,
Expand Down
7 changes: 3 additions & 4 deletions adminSiteServer/publicApiRouter.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { FunctionalRouter } from "./FunctionalRouter.js"
import { Request, Response } from "./authentication.js"
import { Router, Request, Response } from "express"
import * as db from "../db/db.js"

export const publicApiRouter = new FunctionalRouter()
export const publicApiRouter = Router()

function rejectAfterDelay(ms: number) {
return new Promise((resolve, reject) => setTimeout(reject, ms))
}

publicApiRouter.router.get("/health", async (req: Request, res: Response) => {
publicApiRouter.get("/health", async (req: Request, res: Response) => {
try {
const sqlPromise = db.knexRaw(
db.knexInstance() as db.KnexReadonlyTransaction,
Expand Down

0 comments on commit 3cc565e

Please sign in to comment.