Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔨 Refactor apiRouter into smaller files #4350

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

🔨 Refactor apiRouter into smaller files #4350

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac08e30
🛠️ first round of apiRouter refactor
danyx23 Dec 18, 2024
54d0941
🔨 refactor request handler lambdas to named functions
danyx23 Dec 20, 2024
572b3e3
🔨 finish refactoring of api routes
danyx23 Dec 20, 2024
dbe1aa1
🐝 fix unused imports
danyx23 Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
370 changes: 190 additions & 180 deletions adminSiteServer/apiRoutes/bulkUpdates.ts
View file
Open in desktop

Large diffs are not rendered by default.

238 changes: 124 additions & 114 deletions adminSiteServer/apiRoutes/chartViews.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {

import * as db from "../../db/db.js"
import { expectChartById } from "./charts.js"
import { Request } from "../authentication.js"
import e from "express"
const createPatchConfigAndQueryParamsForChartView = async (
knex: db.KnexReadonlyTransaction,
parentChartId: number,
Expand Down Expand Up @@ -65,7 +67,11 @@ const createPatchConfigAndQueryParamsForChartView = async (
return { patchConfig: patchConfigToSave, fullConfig, queryParams }
}

getRouteWithROTransaction(apiRouter, "/chartViews", async (req, res, trx) => {
export async function getChartViews(
req: Request,
_res: e.Response<any, Record<string, any>>,
trx: db.KnexReadonlyTransaction
) {
type ChartViewRow = Pick<DbPlainChartView, "id" | "name" | "updatedAt"> & {
lastEditedByUser: string
chartConfigId: string
Expand Down Expand Up @@ -109,30 +115,28 @@ getRouteWithROTransaction(apiRouter, "/chartViews", async (req, res, trx) => {
}))

return { chartViews }
})

getRouteWithROTransaction(
apiRouter,
"/chartViews/:id",
async (req, res, trx) => {
const id = expectInt(req.params.id)

type ChartViewRow = Pick<
DbPlainChartView,
"id" | "name" | "updatedAt"
> & {
lastEditedByUser: string
chartConfigId: string
configFull: JsonString
configPatch: JsonString
parentChartId: number
parentConfigFull: JsonString
queryParamsForParentChart: JsonString
}

const row = await db.knexRawFirst<ChartViewRow>(
trx,
`-- sql
}

export async function getChartViewById(
req: Request,
res: e.Response<any, Record<string, any>>,
Comment on lines +114 to +115
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not specific to this route, just noticed it here. Why are we using Request from our authentication module, which is just — IMO — an unnecessary alias for express.Request and Response from Express, when we have our own Response in the same authentication module with additional properties? If anything it should be the other way around, right?

My preference would be to use Request and Response only from Express, if possible.

trx: db.KnexReadonlyTransaction
) {
const id = expectInt(req.params.id)

type ChartViewRow = Pick<DbPlainChartView, "id" | "name" | "updatedAt"> & {
lastEditedByUser: string
chartConfigId: string
configFull: JsonString
configPatch: JsonString
parentChartId: number
parentConfigFull: JsonString
queryParamsForParentChart: JsonString
}

const row = await db.knexRawFirst<ChartViewRow>(
trx,
`-- sql
SELECT
cv.id,
cv.name,
Expand All @@ -151,28 +155,29 @@ getRouteWithROTransaction(
JOIN users u ON cv.lastEditedByUserId = u.id
WHERE cv.id = ?
`,
[id]
)
[id]
)

if (!row) {
throw new JsonError(`No chart view found for id ${id}`, 404)
}

const chartView = {
...row,
configFull: parseChartConfig(row.configFull),
configPatch: parseChartConfig(row.configPatch),
parentConfigFull: parseChartConfig(row.parentConfigFull),
queryParamsForParentChart: JSON.parse(
row.queryParamsForParentChart
),
}

return chartView
if (!row) {
throw new JsonError(`No chart view found for id ${id}`, 404)
}
)

postRouteWithRWTransaction(apiRouter, "/chartViews", async (req, res, trx) => {
const chartView = {
...row,
configFull: parseChartConfig(row.configFull),
configPatch: parseChartConfig(row.configPatch),
parentConfigFull: parseChartConfig(row.parentConfigFull),
queryParamsForParentChart: JSON.parse(row.queryParamsForParentChart),
}

return chartView
}

export async function createChartView(
req: Request,
res: e.Response<any, Record<string, any>>,
trx: db.KnexReadWriteTransaction
) {
const { name, parentChartId } = req.body as Pick<
DbPlainChartView,
"name" | "parentChartId"
Expand All @@ -195,7 +200,6 @@ postRouteWithRWTransaction(apiRouter, "/chartViews", async (req, res, trx) => {
patchConfig,
fullConfig
)

// insert into chart_views
const insertRow: DbInsertChartView = {
name,
Expand All @@ -208,83 +212,89 @@ postRouteWithRWTransaction(apiRouter, "/chartViews", async (req, res, trx) => {
const [resultId] = result

return { chartViewId: resultId, success: true }
})

putRouteWithRWTransaction(
apiRouter,
"/chartViews/:id",
async (req, res, trx) => {
const id = expectInt(req.params.id)
const rawConfig = req.body.config as GrapherInterface
if (!rawConfig) {
throw new JsonError("Invalid request", 400)
}

const existingRow: Pick<
DbPlainChartView,
"chartConfigId" | "parentChartId"
> = await trx(ChartViewsTableName)
.select("parentChartId", "chartConfigId")
.where({ id })
.first()

if (!existingRow) {
throw new JsonError(`No chart view found for id ${id}`, 404)
}

const { patchConfig, fullConfig, queryParams } =
await createPatchConfigAndQueryParamsForChartView(
trx,
existingRow.parentChartId,
rawConfig
)

await updateChartConfigInDbAndR2(
}

export async function updateChartView(
req: Request,
res: e.Response<any, Record<string, any>>,
trx: db.KnexReadWriteTransaction
) {
const id = expectInt(req.params.id)
const rawConfig = req.body.config as GrapherInterface
if (!rawConfig) {
throw new JsonError("Invalid request", 400)
}

const existingRow: Pick<
DbPlainChartView,
"chartConfigId" | "parentChartId"
> = await trx(ChartViewsTableName)
.select("parentChartId", "chartConfigId")
.where({ id })
.first()

if (!existingRow) {
throw new JsonError(`No chart view found for id ${id}`, 404)
}

const { patchConfig, fullConfig, queryParams } =
await createPatchConfigAndQueryParamsForChartView(
trx,
existingRow.chartConfigId as Base64String,
patchConfig,
fullConfig
existingRow.parentChartId,
rawConfig
)

// update chart_views
await trx
.table(ChartViewsTableName)
.where({ id })
.update({
updatedAt: new Date(),
lastEditedByUserId: res.locals.user.id,
queryParamsForParentChart: JSON.stringify(queryParams),
})

return { success: true }
await updateChartConfigInDbAndR2(
trx,
existingRow.chartConfigId as Base64String,
patchConfig,
fullConfig
)

await trx
.table(ChartViewsTableName)
.where({ id })
.update({
updatedAt: new Date(),
lastEditedByUserId: res.locals.user.id,
queryParamsForParentChart: JSON.stringify(queryParams),
})

return { success: true }
}

export async function deleteChartView(
req: Request,
_res: e.Response<any, Record<string, any>>,
trx: db.KnexReadWriteTransaction
) {
const id = expectInt(req.params.id)

const chartConfigId: string | undefined = await trx(ChartViewsTableName)
.select("chartConfigId")
.where({ id })
.first()
.then((row) => row?.chartConfigId)

if (!chartConfigId) {
throw new JsonError(`No chart view found for id ${id}`, 404)
}
)

deleteRouteWithRWTransaction(
apiRouter,
"/chartViews/:id",
async (req, res, trx) => {
const id = expectInt(req.params.id)
await trx.table(ChartViewsTableName).where({ id }).delete()

const chartConfigId: string | undefined = await trx(ChartViewsTableName)
.select("chartConfigId")
.where({ id })
.first()
.then((row) => row?.chartConfigId)
await deleteGrapherConfigFromR2ByUUID(chartConfigId)

if (!chartConfigId) {
throw new JsonError(`No chart view found for id ${id}`, 404)
}
await trx.table(ChartConfigsTableName).where({ id: chartConfigId }).delete()

await trx.table(ChartViewsTableName).where({ id }).delete()
return { success: true }
}

await deleteGrapherConfigFromR2ByUUID(chartConfigId)
getRouteWithROTransaction(apiRouter, "/chartViews", getChartViews)

await trx
.table(ChartConfigsTableName)
.where({ id: chartConfigId })
.delete()
getRouteWithROTransaction(apiRouter, "/chartViews/:id", getChartViewById)

return { success: true }
}
)
postRouteWithRWTransaction(apiRouter, "/chartViews", createChartView)

putRouteWithRWTransaction(apiRouter, "/chartViews/:id", updateChartView)

deleteRouteWithRWTransaction(apiRouter, "/chartViews/:id", deleteChartView)
Loading