From cde4cd51f0ffb1c085078eee4f322c6d4bb33939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dafydd=20Ll=C5=B7r=20Pearson?= Date: Thu, 2 Nov 2023 16:16:21 +0000 Subject: [PATCH] fix: Don't overwrite `req` with `parsedReq` (#2354) * fix: Don't overwrite req with parsedReq * fix: Update schema, add tests --- api.planx.uk/modules/analytics/controller.ts | 14 +++--- api.planx.uk/modules/team/controller.ts | 18 +++---- api.planx.uk/modules/user/controller.ts | 9 ++-- api.planx.uk/modules/webhooks/controller.ts | 37 +++++++++----- api.planx.uk/modules/webhooks/docs.yaml | 3 ++ .../service/lowcalSessionEvents/index.test.ts | 50 ++++++++++++++++--- .../service/lowcalSessionEvents/schema.ts | 1 + api.planx.uk/shared/middleware/validate.ts | 13 +++-- 8 files changed, 97 insertions(+), 48 deletions(-) diff --git a/api.planx.uk/modules/analytics/controller.ts b/api.planx.uk/modules/analytics/controller.ts index d47ec08e60..ebc618516f 100644 --- a/api.planx.uk/modules/analytics/controller.ts +++ b/api.planx.uk/modules/analytics/controller.ts @@ -4,7 +4,7 @@ import { ValidatedRequestHandler } from "../../shared/middleware/validate"; export const logAnalyticsSchema = z.object({ query: z.object({ - analyticsLogId: z.string(), + analyticsLogId: z.string().pipe(z.coerce.number()), }), }); @@ -13,14 +13,14 @@ export type LogAnalytics = ValidatedRequestHandler< Record >; -export const logUserExitController: LogAnalytics = async (req, res) => { - const { analyticsLogId } = req.query; - trackAnalyticsLogExit({ id: Number(analyticsLogId), isUserExit: true }); +export const logUserExitController: LogAnalytics = async (_req, res) => { + const { analyticsLogId } = res.locals.parsedReq.query; + trackAnalyticsLogExit({ id: analyticsLogId, isUserExit: true }); res.status(204).send(); }; -export const logUserResumeController: LogAnalytics = async (req, res) => { - const { analyticsLogId } = req.query; - trackAnalyticsLogExit({ id: Number(analyticsLogId), isUserExit: false }); +export const logUserResumeController: LogAnalytics = async (_req, res) => { + const { analyticsLogId } = res.locals.parsedReq.query; + trackAnalyticsLogExit({ id: analyticsLogId, isUserExit: false }); res.status(204).send(); }; diff --git a/api.planx.uk/modules/team/controller.ts b/api.planx.uk/modules/team/controller.ts index 4c12cf6e07..55ff9f1400 100644 --- a/api.planx.uk/modules/team/controller.ts +++ b/api.planx.uk/modules/team/controller.ts @@ -36,9 +36,9 @@ export type RemoveMember = ValidatedRequestHandler< TeamMemberResponse >; -export const addMember: UpsertMember = async (req, res, next) => { - const { teamSlug } = req.params; - const { userEmail, role } = req.body; +export const addMember: UpsertMember = async (_req, res, next) => { + const { teamSlug } = res.locals.parsedReq.params; + const { userEmail, role } = res.locals.parsedReq.body; try { await Service.addMember({ userEmail, teamSlug, role }); @@ -53,9 +53,9 @@ export const addMember: UpsertMember = async (req, res, next) => { } }; -export const changeMemberRole: UpsertMember = async (req, res, next) => { - const { teamSlug } = req.params; - const { userEmail, role } = req.body; +export const changeMemberRole: UpsertMember = async (_req, res, next) => { + const { teamSlug } = res.locals.parsedReq.params; + const { userEmail, role } = res.locals.parsedReq.body; try { await Service.changeMemberRole({ userEmail, teamSlug, role }); @@ -67,9 +67,9 @@ export const changeMemberRole: UpsertMember = async (req, res, next) => { } }; -export const removeMember: RemoveMember = async (req, res, next) => { - const { teamSlug } = req.params; - const { userEmail } = req.body; +export const removeMember: RemoveMember = async (_req, res, next) => { + const { teamSlug } = res.locals.parsedReq.params; + const { userEmail } = res.locals.parsedReq.body; try { await Service.removeMember({ userEmail, teamSlug }); diff --git a/api.planx.uk/modules/user/controller.ts b/api.planx.uk/modules/user/controller.ts index 9a3355c2d6..c61f3c7107 100644 --- a/api.planx.uk/modules/user/controller.ts +++ b/api.planx.uk/modules/user/controller.ts @@ -21,10 +21,11 @@ export type CreateUser = ValidatedRequestHandler< UserResponse >; -export const createUser: CreateUser = async (req, res, next) => { +export const createUser: CreateUser = async (_req, res, next) => { try { + const newUser = res.locals.parsedReq.body; const $client = getClient(); - await $client.user.create(req.body); + await $client.user.create(newUser); return res.send({ message: "Successfully created user" }); } catch (error) { return next( @@ -44,9 +45,9 @@ export type DeleteUser = ValidatedRequestHandler< UserResponse >; -export const deleteUser: DeleteUser = async (req, res, next) => { +export const deleteUser: DeleteUser = async (_req, res, next) => { try { - const { email } = req.params; + const { email } = res.locals.parsedReq.params; const $client = getClient(); const user = await $client.user.getByEmail(email); diff --git a/api.planx.uk/modules/webhooks/controller.ts b/api.planx.uk/modules/webhooks/controller.ts index 1621159778..edb43ac4eb 100644 --- a/api.planx.uk/modules/webhooks/controller.ts +++ b/api.planx.uk/modules/webhooks/controller.ts @@ -16,7 +16,7 @@ import { SanitiseApplicationData } from "./service/sanitiseApplicationData/types import { sanitiseApplicationData } from "./service/sanitiseApplicationData"; export const sendSlackNotificationController: SendSlackNotification = async ( - req, + _req, res, next, ) => { @@ -27,8 +27,9 @@ export const sendSlackNotificationController: SendSlackNotification = async ( }); } - const eventData = req.body.event.data.new; - const eventType = req.query.type; + const { body, query } = res.locals.parsedReq; + const eventData = body.event.data.new; + const eventType = query.type; try { const data = await sendSlackNotification(eventData, eventType); @@ -44,9 +45,11 @@ export const sendSlackNotificationController: SendSlackNotification = async ( }; export const createPaymentInvitationEventsController: CreatePaymentEventController = - async (req, res, next) => { + async (_req, res, next) => { try { - const response = await createPaymentInvitationEvents(req.body); + const response = await createPaymentInvitationEvents( + res.locals.parsedReq.body, + ); res.json(response); } catch (error) { return next( @@ -59,9 +62,11 @@ export const createPaymentInvitationEventsController: CreatePaymentEventControll }; export const createPaymentReminderEventsController: CreatePaymentEventController = - async (req, res, next) => { + async (_req, res, next) => { try { - const response = await createPaymentReminderEvents(req.body); + const response = await createPaymentReminderEvents( + res.locals.parsedReq.body, + ); res.json(response); } catch (error) { return next( @@ -74,9 +79,11 @@ export const createPaymentReminderEventsController: CreatePaymentEventController }; export const createPaymentExpiryEventsController: CreatePaymentEventController = - async (req, res, next) => { + async (_req, res, next) => { try { - const response = await createPaymentExpiryEvents(req.body); + const response = await createPaymentExpiryEvents( + res.locals.parsedReq.body, + ); res.json(response); } catch (error) { return next( @@ -89,9 +96,11 @@ export const createPaymentExpiryEventsController: CreatePaymentEventController = }; export const createSessionReminderEventController: CreateSessionEventController = - async (req, res, next) => { + async (_req, res, next) => { try { - const response = await createSessionReminderEvent(req.body); + const response = await createSessionReminderEvent( + res.locals.parsedReq.body, + ); res.json(response); } catch (error) { return next( @@ -104,9 +113,11 @@ export const createSessionReminderEventController: CreateSessionEventController }; export const createSessionExpiryEventController: CreateSessionEventController = - async (req, res, next) => { + async (_req, res, next) => { try { - const response = await createSessionExpiryEvent(req.body); + const response = await createSessionExpiryEvent( + res.locals.parsedReq.body, + ); res.json(response); } catch (error) { return next( diff --git a/api.planx.uk/modules/webhooks/docs.yaml b/api.planx.uk/modules/webhooks/docs.yaml index dc0d2af8d2..309976e562 100644 --- a/api.planx.uk/modules/webhooks/docs.yaml +++ b/api.planx.uk/modules/webhooks/docs.yaml @@ -123,6 +123,9 @@ components: properties: sessionId: type: string + email: + type: string + format: email responses: SlackNotificationSuccessMessage: content: diff --git a/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/index.test.ts b/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/index.test.ts index 8f557a4686..303310aa3b 100644 --- a/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/index.test.ts +++ b/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/index.test.ts @@ -32,8 +32,23 @@ describe("Create reminder event webhook", () => { it("returns a 400 if a required value is missing", async () => { const missingCreatedAt = { createdAt: null, payload: {} }; const missingPayload = { createdAt: new Date(), payload: null }; - - for (const body of [missingCreatedAt, missingPayload]) { + const missingEmail = { + createdAt: new Date(), + payload: { sessionId: "abc123" }, + }; + const missingSessionId = { + createdAt: new Date(), + payload: { email: "test@example.com" }, + }; + + const invalidRequestBodies = [ + missingCreatedAt, + missingPayload, + missingEmail, + missingSessionId, + ]; + + for (const body of invalidRequestBodies) { await post(ENDPOINT) .set({ Authorization: process.env.HASURA_PLANX_API_KEY }) .send(body) @@ -46,7 +61,10 @@ describe("Create reminder event webhook", () => { }); it("returns a 200 on successful event setup", async () => { - const body = { createdAt: new Date(), payload: { sessionId: "123" } }; + const body = { + createdAt: new Date(), + payload: { sessionId: "123", email: "test@example.com" }, + }; mockedCreateScheduledEvent.mockResolvedValue(mockScheduledEventResponse); await post(ENDPOINT) @@ -64,7 +82,10 @@ describe("Create reminder event webhook", () => { }); it("passes the correct arguments along to createScheduledEvent", async () => { - const body = { createdAt: new Date(), payload: { sessionId: "123" } }; + const body = { + createdAt: new Date(), + payload: { sessionId: "123", email: "test@example.com" }, + }; mockedCreateScheduledEvent.mockResolvedValue(mockScheduledEventResponse); await post(ENDPOINT) @@ -96,7 +117,10 @@ describe("Create reminder event webhook", () => { }); it("returns a 500 on event setup failure", async () => { - const body = { createdAt: new Date(), payload: { sessionId: "123" } }; + const body = { + createdAt: new Date(), + payload: { sessionId: "123", email: "test@example.com" }, + }; mockedCreateScheduledEvent.mockRejectedValue(new Error("Failed!")); await post(ENDPOINT) @@ -143,7 +167,10 @@ describe("Create expiry event webhook", () => { }); it("returns a 200 on successful event setup", async () => { - const body = { createdAt: new Date(), payload: { sessionId: "123" } }; + const body = { + createdAt: new Date(), + payload: { sessionId: "123", email: "test@example.com" }, + }; mockedCreateScheduledEvent.mockResolvedValue(mockScheduledEventResponse); await post(ENDPOINT) @@ -151,12 +178,16 @@ describe("Create expiry event webhook", () => { .send(body) .expect(200) .then((response) => { + // expect(mockedCreateScheduledEvent).toHaveBeenCalledWith(body.p) expect(response.body).toStrictEqual([mockScheduledEventResponse]); }); }); it("passes the correct arguments along to createScheduledEvent", async () => { - const body = { createdAt: new Date(), payload: { sessionId: "123" } }; + const body = { + createdAt: new Date(), + payload: { sessionId: "123", email: "test@example.com" }, + }; mockedCreateScheduledEvent.mockResolvedValue(mockScheduledEventResponse); await post(ENDPOINT) @@ -173,7 +204,10 @@ describe("Create expiry event webhook", () => { }); it("returns a 500 on event setup failure", async () => { - const body = { createdAt: new Date(), payload: { sessionId: "123" } }; + const body = { + createdAt: new Date(), + payload: { sessionId: "123", email: "test@example.com" }, + }; mockedCreateScheduledEvent.mockRejectedValue(new Error("Failed!")); await post(ENDPOINT) diff --git a/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/schema.ts b/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/schema.ts index 2f35683b0e..f8b17aa24f 100644 --- a/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/schema.ts +++ b/api.planx.uk/modules/webhooks/service/lowcalSessionEvents/schema.ts @@ -7,6 +7,7 @@ export const createSessionEventSchema = z.object({ createdAt: z.string().pipe(z.coerce.date()), payload: z.object({ sessionId: z.string(), + email: z.string().email(), }), }), }); diff --git a/api.planx.uk/shared/middleware/validate.ts b/api.planx.uk/shared/middleware/validate.ts index b2b740df00..037ed02a31 100644 --- a/api.planx.uk/shared/middleware/validate.ts +++ b/api.planx.uk/shared/middleware/validate.ts @@ -19,11 +19,9 @@ export const validate = query: req.query, }); - // Assign parsed values to the request object + // Assign parsed values to the response object // Required for schemas to transform or coerce raw requests - req.params = parsedReq.params; - req.body = parsedReq.body; - req.query = parsedReq.query; + res.locals.parsedReq = parsedReq; return next(); } catch (error) { @@ -40,8 +38,9 @@ export type ValidatedRequestHandler< TSchema extends ZodSchema, TResponse, > = RequestHandler< - z.infer["params"], + Request["params"], TResponse, - z.infer["body"], - z.infer["query"] + Request["body"], + Request["query"], + { parsedReq: z.infer } >;