feat(bookings): support casuals when computing remaining bookings, allowing for casual bookings by choden-dev · Pull Request #966 · UoaWDCC/uabc-web

choden-dev · 2026-03-08T04:22:05Z

Description

Fixes #862

Main changes

Introduce new source of truth for remainingSessionswith GET /me/bookings/remaining
- Uses a strategy depending on casual vs admin/member
- Casuals will have 1 session available as long as they haven't booked in the week (see apps/backend/src/data-layer/utils/GameSessionUtils.ts)
- This is called whenever remaining sessions is needed for the user
Make it clear on the bookings confirmation that the user is booking the casual slot (Attendees --> Casual Attendees)

Background

The computation for totalSessionsLeft shows that remainingSessions is always $0$ for casuals. There are no references to appropriately update the remainingSessions field, except for in the resetAllMemberships method. This implies that the main issue beyond a lack of UX for users with 0 bookings is that casuals must have their remaining sessions manually set before.

Warning

The callback at validateUserRemaningSessions makes the source of truth for the user role remainingSessions, meaning we have two options here:

Accept this and create a different experience for casuals (users with 0 remainingSession)
a. This might be the only option seeing as sessions need to be manually added to users, and the casuals require a different process (pay if accepted)
Add credits to users every week
a. This would require a rework of the user roles given the tight coupling between remainingSessionand role

The best option here would be to stop using user.remainingSessions as the source of truth for computing the remaining sessions in both BookFlow and useBookingLimits and rather use a strategy-based approach for the different user roles.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature or improvement (non-breaking change which adds/modifies functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Booking as casual user

Booking as member/admin

Note

This is the same behaviour as before

Booking Process

After booking

Bookings go from 5 --> 4

Manual testing (requires screenshots or videos)
Integration tests written (requires checks to pass)
This does not require tests (please leave a note as to why)

Not in scope

There is an off-by-one error for users who use their last booking, as in countAttendees the capacity is determined by counting user roles, while validateUserRemaningSessions ensures that those with 0 bookings are casual, therefore some members will be wrongly counted as casual.
- Not a blocker for this issue, would likely need to follow-up by attaching an additional field to bookings stating what role the user made the booking as, such as member or casual
[FRONTEND] Revamp book page with sessions user has already booked #962 is not in scope for this issue as that is a visual rather than technical change

Checklist before requesting a review

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added thorough tests that prove my fix is effective and that my feature works
Additional issues have been created for any tech debt left in this PR
I've requested a review from another user

github-actions · 2026-03-08T04:22:35Z

Preview Deployment Status

✅ Storybook Preview: https://0989b559.uabc-storybook.pages.dev

✅ Frontend Preview: https://ffff6f41.uabc.pages.dev

…flow

…ainingSessions prop

… for users

…er booking limits

github-actions · 2026-03-08T04:38:41Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	83.91% (🎯 70%) ⬆️ +0.07%	2702 / 3220
🔵	Statements	83.8% (🎯 70%) ⬆️ +0.08%	2767 / 3302
🔵	Functions	78.28% (🎯 80%) ⬆️ +0.06%	645 / 824
🔵	Branches	78.36% (🎯 60%) 🟰 ±0%	1553 / 1982

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
apps/frontend/src/components/client/book/BookClient.tsx	20% ⬇️ -5.00%	0% 🟰 ±0%	0% 🟰 ±0%	20% ⬇️ -5.00%	18-26
apps/frontend/src/components/client/book/BookFlow.tsx	90.69% 🟰 ±0%	66.66% 🟰 ±0%	100% 🟰 ±0%	90.47% 🟰 ±0%	112-115
apps/frontend/src/components/client/user/ProfileSection.tsx	62.96% ⬆️ +1.43%	58.82% ⬆️ +20.36%	50% 🟰 ±0%	65.38% ⬆️ +1.38%	37-38, 42-57, 62
apps/backend/src/app/api/bookings/route.ts	100% 🟰 ±0%	89.28% ⬆️ +0.82%	100% 🟰 ±0%	100% 🟰 ±0%
apps/backend/src/app/api/me/bookings/remaining/route.ts	100%	100%	100%	100%
apps/backend/src/data-layer/utils/GameSessionUtils.ts	96.77% ⬆️ +1.54%	95% ⬆️ +3.34%	100% 🟰 ±0%	96.77% ⬆️ +1.54%	54
packages/ui/src/components/Composite/BookingConfirmation/BookingConfirmation.tsx	100% 🟰 ±0%	61.53% ⬇️ -2.10%	100% 🟰 ±0%	100% 🟰 ±0%
packages/ui/src/components/Composite/SelectACourt/SelectACourt.tsx	0% 🟰 ±0%	0% 🟰 ±0%	0% 🟰 ±0%	0% 🟰 ±0%	82-288
packages/ui/src/components/Composite/UserPanel/UserPanel.tsx	100% 🟰 ±0%	50% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%
packages/ui/src/hooks/useBookingLimits/useBookingLimits.ts	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%

Generated in workflow #3970 for commit 7761177 by the Vitest Coverage Report Action

…or consistency

jji05 · 2026-03-08T10:30:32Z

@choden-dev Thank you for the PR!

A couple things I'd like to clarify up that I should have left in the original user story!

validateUserRemainingSessions was created as a fail safe due to club admins getting access to payload, in case they'd like to use that instead of the admin dashboard, editing a user's sessions ensured that

0 sessions = casual
≥1 sessions = member

This also meant any db writes would result in a correction of membership status (leaves less work to designate a db request updating it). We want to keep this

As for the warning block,

The callback at validateUserRemaningSessions makes the source of truth for the user role remainingSessions, meaning we have two options here:

Accept this and create a different experience for casuals (users with 0 remainingSession)
a. This might be the only option seeing as sessions need to be manually added to users, and the casuals require a different process (pay if accepted)

The admins initially wanted and quite firm about a -1 remaining sessions system (as this closely aligned with how their flow worked best for a pay as you go system although this was not followed in the frontend). We would want to keep casuals at 0 sessions although visually display a slightly different system, so that they know they are not under the booking credit system (displaying 1 would perhaps be confusing).

TL;DR; The only difference being members use their credits to pay, while casuals are allocated MAX 1 session per week dependent on an existing booking within the week.

Clarifications on the out of scope, I am aware of the first bug and is a P1, I've created a ticket (#967), thank you for reminding me!

jji05

Great work, a couple comments left! Thank you 👍

jji05 · 2026-03-09T07:01:13Z

apps/backend/src/app/api/bookings/route.ts

+      const remainingSessionsBasedOnRole = await getRemainingSessions(
+        userData,
+        semesterDataService,
+        bookingDataService,
+        userDataService,
+      )
+
+      if (remainingSessionsBasedOnRole <= 0)
        return NextResponse.json(
          { error: "No remaining sessions" },


nit: I like the logic here although it seems to be overlapping with standard member logic and could be refined down to calling getAllCurrentWeekBookingsByUserId once for both casuals and regulars. Note that this logic is used again in lines:

uabc-web/apps/backend/src/app/api/bookings/route.ts

Lines 89 to 99 in 7761177

const allUpcomingBookings = await bookingDataService.getAllCurrentWeekBookingsByUserId(

req.user.id,

currentSemester,

)

if (allUpcomingBookings.length >= getMaxBookingSize(req.user)) {

return NextResponse.json(

{ error: "Weekly booking limit reached" },

{ status: StatusCodes.FORBIDDEN },

)

}

For casuals this would be the correct message for their role?

Both of them are correct, in this case, it would be ideal to simplify the logic as the same logic is being checked twice

I agree from an efficiency point of view, but realistically you should not know that the implementation of getRemainingSessions does the check, so they are actually doing different things from a black box perspective (weekly bookings vs getting remaining sessions)

In that case, I'm happy with your implementation

apps/backend/src/app/api/bookings/route.ts

jji05 · 2026-03-09T07:03:53Z

apps/backend/src/app/api/bookings/route.test.ts

@@ -284,12 +310,16 @@ describe("/api/bookings", async () => {
    const res = await POST(req)

    expect(res.status).toBe(StatusCodes.FORBIDDEN)
-    expect(await res.json()).toStrictEqual({ error: "Session is full for the selected user role" })
+    expect(await res.json()).toStrictEqual({ error: "No remaining sessions" })
  })


based on the test description, it should be returning "Session is full for the selected user role". since the logic was changed to:

check remaining sessions (for casual user mock) -> check game session capacities

we need to change the description and add a test to test for the initial test topic

For casuals this would be the correct message for their role?

No, the error is only because booking create mock is using casual user and that causes it to show no remaining sessions. The point of the test was to test for casual capacities

jji05 · 2026-03-09T07:10:11Z

apps/backend/src/app/api/bookings/route.test.ts

@@ -162,7 +167,7 @@ describe("/api/bookings", async () => {

      const res = await POST(req)
      expect(res.status).toBe(StatusCodes.FORBIDDEN)
-      expect(await res.json()).toStrictEqual({ error: "Weekly booking limit reached" })
+      expect(await res.json()).toStrictEqual({ error: "No remaining sessions" })
    })


on second thought, I'd prefer the error message be "Weekly booking limit reached" as it makes more sense for casuals to see a weekly limit reached rather than no remaining sessions (they never have remaining sessions as 1 remaining session indicates "member", but if casuals are displayed "1" too which makes the role system confusing), this is open for discussion.

the proposed logic change would involve only checking remainingSessions for members (essentially casuals bypass the initial remainingSessionsBasedOnRole check and only just check existing weekly bookings over here:

uabc-web/apps/backend/src/app/api/bookings/route.ts

Lines 88 to 99 in 7761177

// Important: this controls the weekly booking limit for both casual and member users, as it counts all bookings regardless of user role.

const allUpcomingBookings = await bookingDataService.getAllCurrentWeekBookingsByUserId(

req.user.id,

currentSemester,

)

if (allUpcomingBookings.length >= getMaxBookingSize(req.user)) {

return NextResponse.json(

{ error: "Weekly booking limit reached" },

{ status: StatusCodes.FORBIDDEN },

)

}

I disagree, having a role based on another value is confusing and users do not need to know that they become a casual if they have 0 sessions, rather it should be enough to specify that their sessions only apply to the casual slots.

Users need to know that they become casual when they have 0 sessions, this was specified by client as they have a very different booking criteria as far as I am aware.

Isn't it quite easy to show the user they are casual without linking it to the sessions? It's important from a business logic perspective but from a UX perspective they only need to know if they are casual/member and the implications of such.

It is important business logic as now we display 1 session and if a user tops up, thinking they'd get 3 sessions would be wrong! Hence adding a tooltip would clarify it in a business perspective

I thought this comment you made addresses that? i.e it shouldn't be a backend thing

jji05 · 2026-03-09T07:16:53Z

apps/backend/src/app/api/me/bookings/remaining/route.test.ts

+      const json = await response.json()
+      expect(typeof json.data.remainingSessions).toBe("number")
+    })


this doesn't necessarily make the test correct, please check if the remainingSessions is equal to memberUserMock.remainingSessions

uabc-web/packages/shared/src/mocks/User.mock.ts

Line 38 in 7761177

remainingSessions: 5,

jji05 · 2026-03-09T07:21:24Z

apps/backend/src/data-layer/utils/GameSessionUtils.test.ts

+  describe("getRemainingSessions", () => {
+    let mockSemesterDataService: {
+      getCurrentSemester: Mock
+    }
+    let mockBookingDataService: {
+      getAllCurrentWeekBookingsByUserId: Mock
+    }
+    let mockUserDataService: {
+      getUserById: Mock
+    }
+
+    beforeEach(() => {
+      mockSemesterDataService = {
+        getCurrentSemester: vi.fn(),
+      }
+      mockBookingDataService = {
+        getAllCurrentWeekBookingsByUserId: vi.fn(),
+      }
+      mockUserDataService = {
+        getUserById: vi.fn(),
+      }
+    })
+
+    it("should return 1 for casual user with no existing bookings", async () => {
+      const user = { id: casualUserMock.id, role: MembershipType.casual, remainingSessions: null }
+      mockSemesterDataService.getCurrentSemester.mockResolvedValue({ id: "semester-1" })
+      mockBookingDataService.getAllCurrentWeekBookingsByUserId.mockResolvedValue([])


tests we write are mocked as little as possible to avoid mocks being inaccurate from actual expected functionality. I would recommend using actual data services to run tests

jji05 · 2026-03-09T07:32:16Z

apps/frontend/src/components/client/book/BookFlow.tsx

  numberBookedSessions?: number
+
+  /**
+   * The number of remaining sessions for the user.
+   */
+  remainingSessions: number


nit: extra space and more descriptive description

Suggested change

numberBookedSessions?: number

/**

* The number of remaining sessions for the user.

*/

remainingSessions: number

numberBookedSessions?: number

/**

* The number of remaining sessions for the user.

* @remarks This is used instead of `auth` as this provides a more accurate session count

* for casuals by checking their exisiting bookings within the week.

*/

remainingSessions: number

jji05 · 2026-03-09T07:38:06Z

apps/frontend/src/components/client/user/ProfileSection.tsx

@@ -63,7 +65,16 @@ export const ProfileSection = memo(({ auth }: { auth: AuthContextValue }) => {
  return (
    <Container centerContent gap="xl" layerStyle="container">
      <Grid gap="xl" templateColumns={{ base: "1fr", lg: "1fr 1.5fr" }} w="full">
-        <GridItem>{!user ? <UserPanelSkeleton /> : <UserPanel user={user} />}</GridItem>
+        <GridItem>
+          {isRemainingSessionsLoading || !user ? (
+            <UserPanelSkeleton />
+          ) : (
+            <UserPanel
+              remainingSessions={remainingSessionsResponse?.data.remainingSessions ?? 0}
+              user={user}
+            />
+          )}
+        </GridItem>


would be ideal to not display 1 for casuals as a member could also have 1 and it isn't clear enough of a distinction!

you can leave it as is if you add the Tooltip component over an InfoIcon for casuals so they are aware it is a weekly thing

apps/frontend/src/components/client/user/ProfileSection.test.tsx

choden-dev requested a review from jji05 March 8, 2026 04:22

choden-dev added 5 commits March 8, 2026 17:25

feat(bookings): add remaining sessions tracking for users in booking …

03fcf35

…flow

feat(bookings): rename future bookings query key to remaining sessions

7cd930c

feat(bookings): update attendees label based on user role

9750afc

feat(bookings): update BookFlow and related components to include rem…

56f39d0

…ainingSessions prop

feat(bookings): integrate UserDataService to fetch remaining sessions…

673f8fc

… for users

choden-dev force-pushed the 862-casuals-cant-book branch from b13ce69 to 673f8fc Compare March 8, 2026 04:25

choden-dev added 3 commits March 8, 2026 17:27

test(bookFlow): update tests to reflect remainingSessions value change

03fd0a7

test(bookFlow, UserPanel): add remainingSessions prop to tests for us…

e964cd9

…er booking limits

fix(route): remove nullish coalescing for remainingSessions check

b54422c

test(bookingLimits, UserPanel): set remainingSessions to 0 in tests f…

7761177

…or consistency

choden-dev marked this pull request as ready for review March 8, 2026 04:51

jji05 requested changes Mar 9, 2026

View reviewed changes

	const allUpcomingBookings = await bookingDataService.getAllCurrentWeekBookingsByUserId(
	req.user.id,
	currentSemester,
	)

	if (allUpcomingBookings.length >= getMaxBookingSize(req.user)) {
	return NextResponse.json(
	{ error: "Weekly booking limit reached" },
	{ status: StatusCodes.FORBIDDEN },
	)
	}

	// Important: this controls the weekly booking limit for both casual and member users, as it counts all bookings regardless of user role.
	const allUpcomingBookings = await bookingDataService.getAllCurrentWeekBookingsByUserId(
	req.user.id,
	currentSemester,
	)

	if (allUpcomingBookings.length >= getMaxBookingSize(req.user)) {
	return NextResponse.json(
	{ error: "Weekly booking limit reached" },
	{ status: StatusCodes.FORBIDDEN },
	)
	}

Conversation

choden-dev commented Mar 8, 2026

Description

Main changes

Background

Type of change

How Has This Been Tested?

Booking as casual user

Booking as member/admin

Booking Process

After booking

Not in scope

Checklist before requesting a review

Uh oh!

github-actions bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preview Deployment Status

Uh oh!

github-actions bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

jji05 commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jji05 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

choden-dev Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

choden-dev Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

choden-dev Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Mar 8, 2026 •

edited

Loading

github-actions bot commented Mar 8, 2026 •

edited

Loading

jji05 commented Mar 8, 2026 •

edited

Loading

choden-dev Mar 9, 2026 •

edited

Loading

choden-dev Mar 9, 2026 •

edited

Loading

choden-dev Mar 9, 2026 •

edited

Loading