From 1970eb7b9c0bfc2f1562c812c295ac6c4113cfc5 Mon Sep 17 00:00:00 2001 From: Roba Boru Date: Tue, 7 Jul 2026 23:05:45 +0300 Subject: [PATCH] Fix duplicate hold seat --- .../src/modules/seats/seats.controller.ts | 23 ++- .../src/modules/seats/seats.dto.ts | 5 + .../src/app/api/seats/release-hold/route.ts | 37 ++++ .../portal/src/app/booking/seats/page.tsx | 164 ++++++++++++------ 4 files changed, 177 insertions(+), 52 deletions(-) create mode 100644 apps/edr-passenger-web/portal/src/app/api/seats/release-hold/route.ts diff --git a/apps/edr-passenger-api/src/modules/seats/seats.controller.ts b/apps/edr-passenger-api/src/modules/seats/seats.controller.ts index 4a9783101..fa233f45c 100644 --- a/apps/edr-passenger-api/src/modules/seats/seats.controller.ts +++ b/apps/edr-passenger-api/src/modules/seats/seats.controller.ts @@ -19,7 +19,7 @@ import { ApiResponse, } from "@nestjs/swagger"; import { SeatsService } from "./seats.service"; -import { HoldSeatsDto } from "./seats.dto"; +import { HoldSeatsDto, ReleaseHoldDto } from "./seats.dto"; import { JwtGuard } from "../../common/jwt.guard"; import { IamGuard } from "../../common/iam-adapter"; @@ -182,6 +182,27 @@ This makes it clear which segment of the route each seat is held for, enabling s return this.service.releaseHold(holdId); } + @Post("release") + @SetMetadata('isPublic', true) + @ApiOperation({ + summary: "Release a seat hold by holdId (portal-server use only)", + description: + "Frees a previously-created hold's seats immediately instead of waiting for it to " + + "expire — used when a guest or logged-in user changes their seat selection, so the " + + "stale hold doesn't linger and block that seat for other travellers.\n\n" + + "This is a public endpoint (no JWT), like POST /seats/hold, since guest sessions have " + + "no login to authenticate with. It must ONLY ever be called from the passenger portal's " + + "own Next.js server (a server-side route handler), never directly from browser code — " + + "calling it straight from client JS would let anyone script mass hold-cancellation " + + "against other travellers' in-progress seat selections. The portal's server-side proxy " + + "is what keeps this endpoint's existence out of the browser's network requests.", + }) + @ApiResponse({ status: 200, description: "Hold released" }) + @ApiResponse({ status: 404, description: "Hold not found" }) + releaseSeatById(@Body() dto: ReleaseHoldDto) { + return this.service.releaseHold(dto.holdId); + } + // ── Seat Block / Unblock ─────────────────────────────────────────────────── @Post(":seatId/block") @UseGuards(IamGuard) diff --git a/apps/edr-passenger-api/src/modules/seats/seats.dto.ts b/apps/edr-passenger-api/src/modules/seats/seats.dto.ts index 23c3b3d46..87d47588e 100644 --- a/apps/edr-passenger-api/src/modules/seats/seats.dto.ts +++ b/apps/edr-passenger-api/src/modules/seats/seats.dto.ts @@ -48,3 +48,8 @@ export class HoldSeatsDto { @Type(() => PassengerSeatDto) passengers: PassengerSeatDto[]; } + +export class ReleaseHoldDto { + @ApiProperty({ example: 'hold-uuid', description: 'SeatHold UUID to release' }) + @IsString() holdId: string; +} diff --git a/apps/edr-passenger-web/portal/src/app/api/seats/release-hold/route.ts b/apps/edr-passenger-web/portal/src/app/api/seats/release-hold/route.ts new file mode 100644 index 000000000..c2c1d19a0 --- /dev/null +++ b/apps/edr-passenger-web/portal/src/app/api/seats/release-hold/route.ts @@ -0,0 +1,37 @@ +// Server-side proxy for POST /seats/release on the passenger API. +// +// The browser calls THIS route (same-origin, /api/seats/release-hold) instead of the +// backend's public release endpoint directly. Route handlers run on the Next.js server, not +// in the browser, so the actual backend call — and its URL — never appears in client-side +// JS or network requests a user could copy and script against other travellers' holds. +// Keeping this indirection is the whole point: it doesn't add cryptographic protection (the +// backend endpoint is still public), it just keeps the release capability out of the +// browser's reach so it can't be trivially discovered and abused from client code. + +const API_URL = process.env.NEXT_PUBLIC_API_URL || "http://localhost:4000"; + +export async function POST(request: Request) { + let holdId: string | undefined; + try { + ({ holdId } = await request.json()); + } catch { + return Response.json({ message: "Invalid JSON body" }, { status: 400 }); + } + + if (!holdId || typeof holdId !== "string") { + return Response.json({ message: "holdId is required" }, { status: 400 }); + } + + try { + const backendResponse = await fetch(`${API_URL}/seats/release`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ holdId }), + }); + + const data = await backendResponse.json().catch(() => null); + return Response.json(data, { status: backendResponse.status }); + } catch { + return Response.json({ message: "Failed to reach booking service" }, { status: 502 }); + } +} diff --git a/apps/edr-passenger-web/portal/src/app/booking/seats/page.tsx b/apps/edr-passenger-web/portal/src/app/booking/seats/page.tsx index ea5fa4de1..9ae90c08b 100644 --- a/apps/edr-passenger-web/portal/src/app/booking/seats/page.tsx +++ b/apps/edr-passenger-web/portal/src/app/booking/seats/page.tsx @@ -4,7 +4,6 @@ export const dynamic = "force-dynamic"; import { useRouter } from "next/navigation"; import { useBookingStore } from "@/lib/booking-store"; -import { useAuthStore } from "@/lib/auth-store"; import { useQuery, useMutation } from "@tanstack/react-query"; import { apiClient } from "@/lib/api-client"; import { useState, useEffect, useCallback, useMemo, useRef, memo } from "react"; @@ -181,7 +180,6 @@ export default function SeatsPage() { packageDepartureStationName, setPackageContext, } = useBookingStore(); - const { isAuthenticated } = useAuthStore(); // Maps passenger index -> assigned seat id. A passenger can only get a seat while // they are the "active" passenger, which prevents bulk/batch selection across passengers. const [passengerSeatMap, setPassengerSeatMap] = useState>({}); @@ -209,6 +207,10 @@ export default function SeatsPage() { confirmText: "OK", }); const [autoAssigningReturn, setAutoAssigningReturn] = useState(false); + // Mobile summary bottom-sheet starts collapsed to a slim bar (badge + Continue button) so + // it doesn't cover the seat map — the full passenger list/progress bar only shows once the + // user explicitly expands it. + const [mobileSummaryExpanded, setMobileSummaryExpanded] = useState(false); const isRoundTrip = searchCriteria?.tripType === "ROUND_TRIP"; const isPackageBooking = !!packageName; @@ -946,9 +948,9 @@ export default function SeatsPage() { // already held (valid, unexpired), skip the API call entirely and reuse that hold — this // is what stops "back, then Continue again" from stacking up a second hold on the same // seats. If the user genuinely picked different seats than what was previously held, - // best-effort release the stale hold first (authenticated sessions only — the release - // endpoint requires a login) before holding the new selection, so at most one hold for - // this leg is ever active at a time. + // release the stale hold first (best-effort — an already-expired/released hold shouldn't + // block picking the new seat(s)) before holding the new selection, so at most one hold for + // this leg is ever active at a time — for both guest and authenticated sessions. const ensureLegHold = async (seatIdsForHold: string[]) => { const selectionMatchesExistingHold = isCurrentLegHoldValid && @@ -958,12 +960,17 @@ export default function SeatsPage() { return; } - if (isCurrentLegHoldValid && isAuthenticated && currentLegHoldId) { + if (isCurrentLegHoldValid && currentLegHoldId) { try { - await apiClient.delete(`/seats/hold/${currentLegHoldId}`); + // Routed through the portal's own server-side proxy (not called on the backend + // directly) so the release capability never appears in client-side network calls. + await fetch("/api/seats/release-hold", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ holdId: currentLegHoldId }), + }); } catch { - // Best-effort — an expired/already-released hold, or a guest session that can't - // call this endpoint, shouldn't block picking the new seat(s). + // Best-effort — an expired/already-released hold shouldn't block picking the new seat(s). } } @@ -1786,23 +1793,29 @@ export default function SeatsPage() { ); - // Summary card content — shared between sidebar and mobile modal - const SummaryContent = () => ( + // Summary content, split into pieces so the mobile bottom-sheet can show a compact + // header + always-reachable Continue button by default, and only reveal the full + // passenger list (SummaryDetails) when the user explicitly expands it — otherwise it + // permanently covers most of the seat map on small screens. + const SummaryHeader = () => ( +
+

+ Selection Summary +

+ + {assignedCount}/{seatEligibleIndices.length} selected + +
+ ); + + const SummaryDetails = () => ( <> -
-

- Selection Summary -

- - {assignedCount}/{seatEligibleIndices.length} selected - -

{allSeatsAssigned ? "All seats selected — ready to continue" @@ -1910,27 +1923,49 @@ export default function SeatsPage() { ); })} + + ); - - + const SummaryContinueButton = () => ( + + ); + + const SummaryAutoAssignButton = () => ( + + ); + + const SummaryActions = () => ( + <> + + + + ); + + // Full summary card — used as-is in the desktop sidebar, which has room to show + // everything at once. + const SummaryContent = () => ( + <> + + + ); @@ -1998,15 +2033,41 @@ export default function SeatsPage() { )} - {/* Mobile summary bottom-sheet — always visible so the active passenger is clear */} + {/* Mobile summary bottom-sheet — collapsed to a single slim row by default so it + barely dents the seat map; tap it to expand the full passenger list + auto-assign. + The Continue button always stays visible either way. */}

-
- + + + {mobileSummaryExpanded && ( +
+ + +
+ )} + +
+ +
@@ -2326,8 +2387,9 @@ export default function SeatsPage() {
- {/* Mobile spacer so bottom-sheet doesn't cover last seat */} -
+ {/* Mobile spacer so the bottom-sheet doesn't cover the last seat — sized to + match the sheet's current (collapsed/expanded) height. */} +