Skip to content
Open
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
4ab183b
refactor(game-sessions): cascade deletion
6xtvo Jul 20, 2025
e1c546c
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Jul 20, 2025
9cca008
refactor: revert changes
6xtvo Jul 20, 2025
4901e9b
feat(game-session-schedule): add cascade deletion
6xtvo Jul 25, 2025
3b41900
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Jul 25, 2025
afeb253
refactor: revert commit
6xtvo Jul 25, 2025
7669707
refactor: merge
6xtvo Jul 25, 2025
313c08f
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Jul 25, 2025
8df6a74
refactor(game-session-schedule): refactor transactions
6xtvo Jul 25, 2025
adac403
refactor: merge
6xtvo Jul 25, 2025
8c8ae03
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Jul 30, 2025
355406d
refactor: merge
6xtvo Jul 30, 2025
5de1088
refactor: merge
6xtvo Jul 30, 2025
69668ab
refactor: add methods to booking data service
6xtvo Aug 5, 2025
c85c560
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Aug 19, 2025
715b99f
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Sep 21, 2025
703d652
feat: merge branches
6xtvo Sep 21, 2025
9fcae39
fix: resolver conflicts
6xtvo Sep 21, 2025
40c44fa
refactor: use transaction adapter methods
6xtvo Sep 21, 2025
9f551d9
feat: implement booking data service for deletion
6xtvo Sep 24, 2025
60d3815
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Sep 24, 2025
507ac15
refactor(comments): update testing description
6xtvo Sep 24, 2025
542a9d8
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Sep 29, 2025
e127832
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 1, 2025
36dbcbb
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 6, 2025
3a31302
test(game-session-schedule): add tests for transactions
6xtvo Oct 7, 2025
6486519
test(game-session-schedule): fix tests
6xtvo Oct 7, 2025
2ac4dee
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 7, 2025
ad4b923
refactor(game-session-schedule): allow dynamic game session querying
6xtvo Oct 7, 2025
7f0c7d3
refactor: merge with master
6xtvo Oct 7, 2025
e7208af
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 7, 2025
ec0190c
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 12, 2025
d09f0f6
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 13, 2025
f60b2c4
refactor(biome): resolve biome issues
6xtvo Oct 14, 2025
1cfe0db
refactor(bookings): add booking and game session data service methods
6xtvo Oct 14, 2025
3820969
refactor(mocks): remove unneeded mock
6xtvo Oct 14, 2025
8aafd51
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 15, 2025
2dff7c5
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 20, 2025
7fae86c
refactor(game-session-schedule): add transaction support for service …
6xtvo Oct 20, 2025
29cac12
Merge branch 'master' into 545-cascade-deletion-gamesession
6xtvo Oct 21, 2025
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
200 changes: 168 additions & 32 deletions apps/backend/src/app/api/admin/game-session-schedules/[id]/route.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { AUTH_COOKIE_NAME, type UpdateGameSessionScheduleData } from "@repo/shared"
import { getReasonPhrase, StatusCodes } from "http-status-codes"
import { cookies } from "next/headers"
import { payload } from "@/data-layer/adapters/Payload"
import BookingDataService from "@/data-layer/services/BookingDataService"
import GameSessionDataService from "@/data-layer/services/GameSessionDataService"
import { createMockNextRequest } from "@/test-config/backend-utils"
import { bookingCreateMock } from "@/test-config/mocks/Booking.mock"
import { gameSessionCreateMock } from "@/test-config/mocks/GameSession.mock"
import { gameSessionScheduleCreateMock } from "@/test-config/mocks/GameSessionSchedule.mock"
import { adminToken, casualToken, memberToken } from "@/test-config/vitest.setup"
import { DELETE, GET, PATCH } from "./route"
Expand Down Expand Up @@ -179,64 +183,196 @@ describe("/api/admin/game-session-schedules/[id]", async () => {
})

describe("DELETE", () => {
it("should return 401 if user is a casual", async () => {
cookieStore.set(AUTH_COOKIE_NAME, casualToken)
const bookingDataService = new BookingDataService()
const gameSessionDataService = new GameSessionDataService()

const res = await DELETE(createMockNextRequest("", "DELETE"), {
params: Promise.resolve({ id: "some-id" }),
})
it("should delete game session schedule and its game session and bookings when cascade is true", async () => {
cookieStore.set(AUTH_COOKIE_NAME, adminToken)

expect(res.status).toBe(StatusCodes.UNAUTHORIZED)
expect(await res.json()).toStrictEqual({ error: "No scope" })
})
const newGameSessionSchedule = await gameSessionDataService.createGameSessionSchedule(
gameSessionScheduleCreateMock,
)
const newGameSession = await gameSessionDataService.createGameSession({
...gameSessionCreateMock,
gameSessionSchedule: newGameSessionSchedule,
})
const booking1 = await bookingDataService.createBooking({
...bookingCreateMock,
gameSession: newGameSession,
})
const booking2 = await bookingDataService.createBooking({
...bookingCreateMock,
gameSession: newGameSession,
})

it("should return 401 if user is member", async () => {
cookieStore.set(AUTH_COOKIE_NAME, memberToken)
const res = await DELETE(
createMockNextRequest("/api/admin/game-session-schedules?cascade=true", "DELETE"),
{
params: Promise.resolve({
id: newGameSessionSchedule.id,
}),
},
)

const res = await DELETE(createMockNextRequest("", "DELETE"), {
params: Promise.resolve({ id: "some-id" }),
})
expect(res.status).toBe(StatusCodes.NO_CONTENT)

expect(res.status).toBe(StatusCodes.UNAUTHORIZED)
expect(await res.json()).toStrictEqual({ error: "No scope" })
await expect(
gameSessionDataService.getGameSessionScheduleById(newGameSessionSchedule.id),
).rejects.toThrow("Not Found")
await expect(gameSessionDataService.getGameSessionById(newGameSession.id)).rejects.toThrow(
"Not Found",
)
await expect(bookingDataService.getBookingById(booking1.id)).rejects.toThrow("Not Found")
await expect(bookingDataService.getBookingById(booking2.id)).rejects.toThrow("Not Found")
})

it("should delete gameSessionSchedule if user is admin", async () => {
it.for([
// Test case 1: Explicit false boolean parameter
"/api/admin/game-session-schedules?cascade=false",
// Test case 2: Flag parameter without value (equivalent to true in query params)
"/api/admin/game-session-schedules?cascade",
// Test case 3: Unrelated query parameter (testing irrelevant params)
"/api/admin/game-session-schedules?straightZhao",
// Test case 4: Base URL with no query parameters
"/api/admin/game-session-schedules",
] as const)(
"should default to not delete related game session and bookings when cascade is false or not specified",
async (route) => {
cookieStore.set(AUTH_COOKIE_NAME, adminToken)

const newGameSessionSchedule = await gameSessionDataService.createGameSessionSchedule(
gameSessionScheduleCreateMock,
)
const newGameSession = await gameSessionDataService.createGameSession({
...gameSessionCreateMock,
gameSessionSchedule: newGameSessionSchedule,
})
const booking1 = await bookingDataService.createBooking({
...bookingCreateMock,
gameSession: newGameSession,
})
const booking2 = await bookingDataService.createBooking({
...bookingCreateMock,
gameSession: newGameSession,
})

const res = await DELETE(createMockNextRequest(route, "DELETE"), {
params: Promise.resolve({
id: newGameSessionSchedule.id,
}),
})

expect(res.status).toBe(StatusCodes.NO_CONTENT)

await expect(
gameSessionDataService.getGameSessionScheduleById(newGameSessionSchedule.id),
).rejects.toThrow("Not Found")
expect(await gameSessionDataService.getGameSessionById(newGameSession.id)).toBeDefined()
expect(await bookingDataService.getBookingById(booking1.id)).toBeDefined()
expect(await bookingDataService.getBookingById(booking2.id)).toBeDefined()
},
)

it("should rollback transaction if error occurs during cascade delete and return 500", async () => {
cookieStore.set(AUTH_COOKIE_NAME, adminToken)

const newGameSessionSchedule = await gameSessionDataService.createGameSessionSchedule(
gameSessionScheduleCreateMock,
)
const newGameSession = await gameSessionDataService.createGameSession({
...gameSessionCreateMock,
gameSessionSchedule: newGameSessionSchedule,
})
const booking = await bookingDataService.createBooking({
...bookingCreateMock,
gameSession: newGameSession,
})

const res = await DELETE(createMockNextRequest("", "DELETE"), {
params: Promise.resolve({ id: newGameSessionSchedule.id }),
const mockDeleteBookings = vi
.spyOn(GameSessionDataService.prototype, "deleteGameSessionSchedule")
.mockRejectedValueOnce(new Error("Delete failed"))

const res = await DELETE(
createMockNextRequest("/api/admin/game-session-schedules?cascade=true", "DELETE"),
{
params: Promise.resolve({
id: newGameSessionSchedule.id,
}),
},
)

expect(res.status).toBe(StatusCodes.INTERNAL_SERVER_ERROR)
expect(await res.json()).toStrictEqual({
error: getReasonPhrase(StatusCodes.INTERNAL_SERVER_ERROR),
})

expect(res.status).toBe(StatusCodes.NO_CONTENT)
await expect(
gameSessionDataService.getGameSessionScheduleById(newGameSessionSchedule.id),
).rejects.toThrow("Not Found")
// Verify game session schedule, game session and booking still exist (transaction rolled back)
expect(
await gameSessionDataService.getGameSessionScheduleById(newGameSessionSchedule.id),
).toBeDefined()
expect(await gameSessionDataService.getGameSessionById(newGameSession.id)).toBeDefined()
expect(await bookingDataService.getBookingById(booking.id)).toBeDefined()

mockDeleteBookings.mockRestore()
})

it("should return 404 if gameSessionSchedule is non-existent", async () => {
it("should handle transaction management correctly", async () => {
cookieStore.set(AUTH_COOKIE_NAME, adminToken)

const res = await DELETE(createMockNextRequest("", "DELETE"), {
params: Promise.resolve({ id: "non-existent" }),
// In a test environment beginTransaction does not return a transaction ID
vi.spyOn(payload.db, "beginTransaction").mockResolvedValue("transaction-id")
vi.spyOn(payload.db, "commitTransaction").mockResolvedValue(undefined)
vi.spyOn(payload.db, "rollbackTransaction").mockResolvedValue(undefined)

// Spy on transaction methods
const beginTransactionSpy = vi.spyOn(payload.db, "beginTransaction")
const commitTransactionSpy = vi.spyOn(payload.db, "commitTransaction")
const rollbackTransactionSpy = vi.spyOn(payload.db, "rollbackTransaction")

const newGameSessionSchedule = await gameSessionDataService.createGameSessionSchedule(
gameSessionScheduleCreateMock,
)
const newGameSession = await gameSessionDataService.createGameSession({
...gameSessionCreateMock,
gameSessionSchedule: newGameSessionSchedule,
})
await bookingDataService.createBooking({
...bookingCreateMock,
gameSession: newGameSession,
})

expect(res.status).toBe(StatusCodes.NOT_FOUND)
const res = await DELETE(
createMockNextRequest("/api/admin/game-session-schedules?cascade=true", "DELETE"),
{
params: Promise.resolve({
id: newGameSessionSchedule.id,
}),
},
)

expect(res.status).toBe(StatusCodes.NO_CONTENT)
expect(beginTransactionSpy).toHaveBeenCalled()
expect(commitTransactionSpy).toHaveBeenCalled()
expect(rollbackTransactionSpy).not.toHaveBeenCalled()

beginTransactionSpy.mockRestore()
commitTransactionSpy.mockRestore()
rollbackTransactionSpy.mockRestore()
})

it("should return a 500 error for internal server error", async () => {
it("should return 404 if game session schedule is not found", async () => {
cookieStore.set(AUTH_COOKIE_NAME, adminToken)

const res = await DELETE(createMockNextRequest("", "DELETE"), {
params: Promise.reject(new Error("Param parsing failed")),
})
const res = await DELETE(
createMockNextRequest("/api/admin/game-session-schedules?cascade=true", "DELETE"),
{
params: Promise.resolve({ id: "invalid-id" }),
},
)

expect(res.status).toBe(StatusCodes.INTERNAL_SERVER_ERROR)
expect(res.status).toBe(StatusCodes.NOT_FOUND)
const json = await res.json()
expect(json.error).toEqual(getReasonPhrase(StatusCodes.INTERNAL_SERVER_ERROR))
expect(json.error).toEqual("Game session schedule not found")
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ import { type NextRequest, NextResponse } from "next/server"
import { NotFound } from "payload"
import { ZodError } from "zod"
import { Security } from "@/business-layer/middleware/Security"
import { payload } from "@/data-layer/adapters/Payload"
import {
commitCascadeTransaction,
createTransactionId,
rollbackCascadeTransaction,
} from "@/data-layer/adapters/Transaction"
import BookingDataService from "@/data-layer/services/BookingDataService"
import GameSessionDataService from "@/data-layer/services/GameSessionDataService"

class RouteWrapper {
Expand Down Expand Up @@ -77,16 +84,52 @@ class RouteWrapper {
/**
* DELETE method to delete a game session schedule.
*
* @param _req The request object
* @param req The request object
* @param params Route parameters containing the GameSessionSchedule ID
* @returns No content status code
*/
@Security("jwt", ["admin"])
static async DELETE(_req: NextRequest, { params }: { params: Promise<{ id: string }> }) {
static async DELETE(req: NextRequest, { params }: { params: Promise<{ id: string }> }) {
try {
const { id } = await params
const cascade = req.nextUrl.searchParams.get("cascade") === "true"
const gameSessionDataService = new GameSessionDataService()
Comment on lines 93 to 95
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we've shifted to use deleteRelatedDocs

await gameSessionDataService.deleteGameSessionSchedule(id)
const transactionID = cascade && (await createTransactionId())
console.log(transactionID)

if (transactionID) {
try {
const gameSessionSchedule = await gameSessionDataService.getGameSessionScheduleById(id)

const { docs } = await payload.find({
collection: "gameSession",
where: {
or: [
{
gameSessionSchedule: {
equals: id,
},
},
{
gameSessionSchedule: {
equals: gameSessionSchedule,
},
},
],
},
})
const gameSession = docs[0]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use data service methods instead of directly using the db adapter here

this isn't too accurate as there could be multiple game sessions tied to a single schedule object. you should be creating something similar to the deleteRelatedBookingsForSession method but to delete game session's based on a game session schedule

await gameSessionDataService.deleteGameSessionSchedule(id)
await gameSessionDataService.deleteGameSession(gameSession.id)
await BookingDataService.deleteRelatedBookingsForSession(gameSession.id, transactionID)
await commitCascadeTransaction(transactionID)
} catch {
await rollbackCascadeTransaction(transactionID)
}
} else {
await gameSessionDataService.deleteGameSessionSchedule(id)
}

return new NextResponse(null, { status: StatusCodes.NO_CONTENT })
} catch (error) {
if (error instanceof NotFound) {
Expand Down
36 changes: 36 additions & 0 deletions apps/backend/src/data-layer/services/BookingDataService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,42 @@ export default class BookingDataService {
return await payload.find({ collection: "booking", limit, page })
}

/**
* Deletes all bookings related to a game session.
* @param sessionId the ID of the game session whose bookings are to be deleted
* @param transactionID an optional transaction ID for the request, useful for tracing
* @private
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

private?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I also understand that instantiations of data services without needing a constructor is poor practice but should be addressed outside of this ticket

please use public async instead of public static async

public static async deleteRelatedBookingsForSession(
sessionId: string,
transactionID?: string | number,
): Promise<Booking[]> {
const relatedBookings = (
await payload.find({
collection: "booking",
where: {
gameSession: {
equals: sessionId,
},
},
pagination: false,
})
).docs
const relatedBookingIds = relatedBookings.map((booking) => booking.id)

const bulkDeletionResult = await payload.delete({
collection: "booking",
where: {
id: { in: relatedBookingIds },
},
req: {
transactionID,
},
})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd recommend looking into advanced queries again, you could just delete all bookings where the gameSession equals the input id instead of fetching all bookings with target game session, then deleting it again.

tl;dr you're doing double the db operations when you could combine everything together

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops, don't know what I was thinking there


return bulkDeletionResult.docs
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need to return docs


/**
* Updates a {@link Booking} by ID.
*
Expand Down
5 changes: 5 additions & 0 deletions apps/backend/src/test-config/mocks/GameSession.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ export const gameSessionCreateMock: CreateGameSessionData = {
casualCapacity: 8,
}

export const gameSessionWithScheduleCreateMock: CreateGameSessionData = {
...gameSessionCreateMock,
gameSessionSchedule: gameSessionScheduleMock,
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this mock isn't getting used

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I didn't create the mock it was already there from a merge

export const oneOffGameSessionCreateMock: CreateGameSessionData = {
semester: semesterMock,
startTime: new Date().toISOString(),
Expand Down
Loading