fix: Don't overwrite req with parsedReq (#2354)

* fix: Don't overwrite req with parsedReq * fix: Update schema, add tests
theopensystemslab · Nov 2, 2023 · cde4cd5 · cde4cd5
1 parent 4e29137
commit cde4cd5
Show file tree

Hide file tree

Showing 8 changed files with 97 additions and 48 deletions.
diff --git 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<string, never>
 >;
 
-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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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: "[email protected]" },
+    };
+
+    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: "[email protected]" },
+    };
     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: "[email protected]" },
+    };
     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: "[email protected]" },
+    };
     mockedCreateScheduledEvent.mockRejectedValue(new Error("Failed!"));
 
     await post(ENDPOINT)
@@ -143,20 +167,27 @@ 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: "[email protected]" },
+    };
     mockedCreateScheduledEvent.mockResolvedValue(mockScheduledEventResponse);
 
     await post(ENDPOINT)
       .set({ Authorization: process.env.HASURA_PLANX_API_KEY })
       .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: "[email protected]" },
+    };
     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: "[email protected]" },
+    };
     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
@@ -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
@@ -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<TSchema>["params"],
+  Request["params"],
   TResponse,
-  z.infer<TSchema>["body"],
-  z.infer<TSchema>["query"]
+  Request["body"],
+  Request["query"],
+  { parsedReq: z.infer<TSchema> }
 >;