Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion angular.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"src/vendors.scss",
"src/app/lib/ngm-dev-blocks/styles/ngm-dev-blocks-styles.scss"
],
"allowedCommonJsDependencies": ["lottie-web"],
"allowedCommonJsDependencies": ["lottie-web", "qrcode"],
"outputPath": "./dist"
},
"configurations": {
Expand Down
8 changes: 8 additions & 0 deletions openapi-spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,14 @@
"format": "date-time",
"nullable": true
},
"depositAmount": {
"type": "number",
"format": "double"
},
"totalAmount": {
"type": "number",
"format": "double"
},
"cancelReason": {
"type": "string",
"nullable": true
Expand Down
2 changes: 1 addition & 1 deletion specs/001-staff-booking-flow/quickstart.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

1. Ensure dependencies are installed with `pnpm install`.
2. Start the application locally via `pnpm start` and navigate to `http://localhost:4200/staff/bookings` using a staff account.
3. Open a booking that is deposit-paid and pending approval, then click **Tiếp tục xử lý** to reach `/staff/bookings/:bookingId/fulfillment`.
3. Open a booking that is deposit-paid and pending approval, then click **Tiếp tục xử lý** to reach `/staff/fulfillment/:bookingId`.

## Required API Sequence

Expand Down
2 changes: 1 addition & 1 deletion specs/001-staff-booking-flow/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ With the rental package in place, the staff agent secures both renter and staff

### Functional Requirements

- **FR-001**: Provide a dedicated fulfillment route (`/staff/bookings/{bookingId}/fulfillment`) available only from the staff bookings workspace for bookings that are deposit-paid and awaiting manual processing; the existing dashboard component must remain unchanged.
- **FR-001**: Provide a dedicated fulfillment route (`/staff/fulfillment/{bookingId}`) available only from the staff bookings workspace for bookings that are deposit-paid and awaiting manual processing; the existing dashboard component must remain unchanged.
- **FR-002**: Display booking, renter, vehicle, and rental summaries on the fulfillment route using read-only data from existing staff booking sources so agents can confirm they are working on the correct record.
- **FR-003**: Present the six required milestones as a sequential checklist that enforces the order `check-in → create rental → create contract → upload inspection → capture renter signature → capture staff signature → confirm vehicle receipt`, unlocking each item only when its predecessors succeed.
- **FR-004**: When the agent confirms booking check-in, call `/booking/POST/api/Booking/checkin` with `bookingId` and `BookingStatus.Approved`, persist completion metadata, and prevent duplicate submissions while awaiting the response.
Expand Down
106 changes: 61 additions & 45 deletions src/app/core-logic/auth/auth.interceptor.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,61 @@
import { HttpErrorResponse, HttpEvent, HttpHandlerFn, HttpRequest } from '@angular/common/http';
import { inject } from '@angular/core';
import { Router } from '@angular/router';
import { Observable, catchError, finalize, take, throwError } from 'rxjs';
import {
Observable,
catchError,
finalize,
of,
shareReplay,
switchMap,
take,
throwError,
} from 'rxjs';
import { AuthService } from './auth.service';
import { TokenService } from '../token/token.service';

let refreshInFlight$: Observable<unknown> | null = null;

Copilot AI Nov 14, 2025

Copy link

Choose a reason for hiding this comment

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

Module-level mutable state (refreshInFlight$) creates a shared singleton that persists across multiple injector contexts and can cause race conditions or stale state. Consider moving this into a service with proper lifecycle management or using a BehaviorSubject to ensure proper cleanup and testability.

Copilot uses AI. Check for mistakes.

const ensureFreshAccessToken = (
authService: AuthService,
tokenService: TokenService,
): Observable<unknown> => {
const needsRefresh = tokenService.isAccessTokenExpiration();
if (!needsRefresh) {
return of(null);
}

const hasRefreshToken = !!tokenService.refreshToken.token;
if (!hasRefreshToken) {
return throwError(() => new Error('Missing refresh token.'));
}

if (!refreshInFlight$) {
refreshInFlight$ = authService.invokeAccessTokenExpiration().pipe(
finalize(() => {
refreshInFlight$ = null;
}),
shareReplay({ bufferSize: 1, refCount: true }),
);
}

return refreshInFlight$;
};

const attachAuthorizationHeader = (
request: HttpRequest<unknown>,
tokenService: TokenService,
): HttpRequest<unknown> => {
const accessToken = tokenService.accessToken.token;
if (!accessToken) {
return request;
}

return request.clone({
headers: request.headers.set('Authorization', `Bearer ${accessToken}`),
});
};

/**
* Intercept
*
Expand All @@ -18,63 +69,28 @@ export const authInterceptor = (
const authService = inject(AuthService);
const tokenService = inject(TokenService);
const router = inject(Router);
// Clone the request object
let newReq = req.clone();

// Request
//Check if the user is authenticated
if (!authService.isAuthenticated) {
return next(req);
}
// Check if the access token is expiration
if (tokenService.isAccessTokenExpiration()) {
// Invoke access token expiration
authService.invokeAccessTokenExpiration().subscribe({
next: () => {
newReq = req.clone({
headers: req.headers.set('Authorization', 'Bearer ' + tokenService.accessToken.token),
});
},
error: () => {
return throwError(() => new Error('Failed to invoke access token expiration.'));
},
});
} else {
// If the access token exists, add the Authorization header.
newReq = req.clone({
headers: req.headers.set('Authorization', 'Bearer ' + tokenService.accessToken.token),
});
}

// Response
return next(newReq).pipe(
return ensureFreshAccessToken(authService, tokenService).pipe(
switchMap(() => next(attachAuthorizationHeader(req, tokenService))),
catchError((error) => {
// Catch "401 Unauthorized" responses
if (error instanceof HttpErrorResponse && error.status === 401) {
//Show alert message about 401 Unauthorized
// alertService.show('401 Unauthorized');
// Run when refresh token is expiration
// Sign out
authService
.signOut()
.pipe(
take(1),
finalize(() => {
// Reload the app
location.reload();
}),
)
.subscribe({
next: () => {
return next(newReq);
},
error: () => {
return throwError(() => error);
},
complete: () => {
return next(newReq);
},
});
return authService.signOut().pipe(
take(1),
finalize(() => {
location.reload();
}),
switchMap(() => throwError(() => error)),
);
}
// Catch other errors
if (error instanceof HttpErrorResponse) {
Expand Down
166 changes: 111 additions & 55 deletions src/app/core-logic/bookings/bookings.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
export interface StaffBookingRecord {
readonly bookingId: string;
readonly renterId?: string;
readonly renterUserId?: string;
readonly status?: BookingStatus;
readonly verificationStatus?: BookingVerificationStatus;
readonly bookingCreatedAt?: string;
Expand Down Expand Up @@ -183,67 +184,82 @@ export class BookingsService {

const bookingIdentifier = this._normalizeString(booking.bookingId) ?? bookingId;
const renterId = this._normalizeString(booking.renterId);
const userIdFromBooking = this._extractUserId(booking);

const renter$ = renterId
? this._bookingService.apiBookingRenterProfileGet(renterId).pipe(
map((response: RenterProfileDtoApiResponse) => response.data ?? undefined),
catchError(() => of(undefined)),
)
: of(undefined);

const rental$ = renterId
? this._rentalService.apiRentalByRenterGet(renterId).pipe(
map((response) => response.data?.items ?? []),
map((items) =>
items.find((item) => this._normalizeString(item.bookingId) === bookingIdentifier),
),
switchMap((match) => {
const rentalId = this._normalizeString(match?.rentalId);
if (!rentalId) {
return of(undefined);
}

return this._rentalService.apiRentalRentalIdGet(rentalId).pipe(
map((response) => response.data ?? undefined),
const userId$ = userIdFromBooking
? of(userIdFromBooking)
: this._resolveRenterUserId$(renterId);

return userId$.pipe(
switchMap((resolvedUserId) => {
const renterLookupId = resolvedUserId ?? renterId;

const renter$ = renterLookupId
? this._bookingService.apiBookingRenterProfileGet(renterLookupId).pipe(
map((response: RenterProfileDtoApiResponse) => response.data ?? undefined),
catchError(() => of(undefined)),
);
}),
catchError(() => of(undefined)),
)
: of(undefined);

return forkJoin({ renter: renter$, rental: rental$ }).pipe(
switchMap(({ renter, rental }) => {
const vehicleId = this._normalizeString(
rental?.vehicle?.vehicleId ?? rental?.vehicleId,
);
const vehicle$ = vehicleId
? this._bookingService.apiBookingVehiclesVehicleIdGet(vehicleId).pipe(
map((response: VehicleDetailsDtoApiResponse) => response.data ?? undefined),
)
: of(undefined);

const rental$ = renterId
? this._rentalService.apiRentalByRenterGet(renterId).pipe(
map((response) => response.data?.items ?? []),
map((items) =>
items.find(
(item) => this._normalizeString(item.bookingId) === bookingIdentifier,
),
),
switchMap((match) => {
const rentalId = this._normalizeString(match?.rentalId);
if (!rentalId) {
return of(undefined);
}

return this._rentalService.apiRentalRentalIdGet(rentalId).pipe(
map((response) => response.data ?? undefined),
catchError(() => of(undefined)),
);
}),
catchError(() => of(undefined)),
)
: of(undefined);

return vehicle$.pipe(
map(
(vehicle) =>
({
bookingId: bookingIdentifier,
renterId,
status: booking.status,
verificationStatus: booking.verificationStatus,
bookingCreatedAt: this._normalizeString(booking.bookingCreatedAt),
startTime: this._normalizeString(booking.startTime),
endTime: this._normalizeString(booking.endTime),
vehicleAtStationId: this._normalizeString(booking.vehicleAtStationId),
verifiedAt: this._normalizeString(booking.verifiedAt),
verifiedByStaffId: this._normalizeString(booking.verifiedByStaffId),
cancelReason: this._normalizeString(booking.cancelReason),
renterProfile: renter,
rental,
vehicleDetails: vehicle,
}) satisfies StaffBookingRecord,
),
return forkJoin({ renter: renter$, rental: rental$ }).pipe(
switchMap(({ renter, rental }) => {
const vehicleId = this._normalizeString(
rental?.vehicle?.vehicleId ?? rental?.vehicleId,
);
const vehicle$ = vehicleId
? this._bookingService.apiBookingVehiclesVehicleIdGet(vehicleId).pipe(
map((response: VehicleDetailsDtoApiResponse) => response.data ?? undefined),
catchError(() => of(undefined)),
)
: of(undefined);

return vehicle$.pipe(
map(
(vehicle) =>
({
bookingId: bookingIdentifier,
renterId,
renterUserId:
resolvedUserId ?? userIdFromBooking ?? this._extractUserId(renter),
status: booking.status,
verificationStatus: booking.verificationStatus,
bookingCreatedAt: this._normalizeString(booking.bookingCreatedAt),
startTime: this._normalizeString(booking.startTime),
endTime: this._normalizeString(booking.endTime),
vehicleAtStationId: this._normalizeString(booking.vehicleAtStationId),
verifiedAt: this._normalizeString(booking.verifiedAt),
verifiedByStaffId: this._normalizeString(booking.verifiedByStaffId),
cancelReason: this._normalizeString(booking.cancelReason),
renterProfile: renter,
rental,
vehicleDetails: vehicle,
}) satisfies StaffBookingRecord,
),
);
}),
);
}),
);
Expand Down Expand Up @@ -343,11 +359,13 @@ export class BookingsService {
const bookingId = booking.bookingId;
const rental = rentalByBookingId.get(bookingId);
const renterProfile = booking.renterId ? renterById.get(booking.renterId) : undefined;
const renterUserId = this._extractUserId(booking) ?? this._extractUserId(renterProfile);
const vehicleDetails = this._resolveVehicleDetails(rental, vehicleDetailsMap);

records.push({
bookingId,
renterId: this._normalizeString(booking.renterId),
renterUserId,
status: booking.status,
verificationStatus: booking.verificationStatus,
bookingCreatedAt: this._normalizeString(booking.bookingCreatedAt),
Expand Down Expand Up @@ -553,6 +571,44 @@ export class BookingsService {
return trimmed.length > 0 ? trimmed : undefined;
}

private _extractUserId(source: unknown): string | undefined {
if (!source || typeof source !== 'object') {
return undefined;
}

const record = source as Record<string, unknown>;
const candidateKeys: readonly string[] = ['userId', 'userID', 'identityId', 'accountId'];

for (const key of candidateKeys) {
const candidate = record[key];
if (typeof candidate !== 'string') {
continue;
}

const normalized = this._normalizeString(candidate);
if (normalized) {
return normalized;
}
}

return undefined;
}

private _resolveRenterUserId$(renterId: string | undefined): Observable<string | undefined> {
if (!renterId) {
return of(undefined);
}

return this._staffService.apiStaffRentersGet().pipe(
map((response: RenterProfileDtoListApiResponse) => response.data ?? []),
map((renters) =>
renters.find((renter) => this._normalizeString(renter?.renterId) === renterId),
),
map((match) => this._extractUserId(match)),
catchError(() => of(undefined)),
);
Comment on lines +602 to +609

Copilot AI Nov 14, 2025

Copy link

Choose a reason for hiding this comment

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

The _resolveRenterUserId$ method fetches the entire renter list on every call to find a single userId. This creates an N+1 query pattern when loading multiple bookings. Consider caching the renter list, using a dedicated API endpoint for userId lookup, or resolving all userIds in batch before processing individual bookings.

Copilot uses AI. Check for mistakes.
Comment on lines +597 to +609

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid refetching renter list for every booking

When building staff booking records the new _resolveRenterUserId$ helper now calls apiStaffRentersGet() for each booking that lacks a userId. The staff dashboard can easily contain dozens of bookings, so this change issues dozens of identical requests that download the entire renter list repeatedly before any booking data resolves. This is a regression from the previous code path (which performed no additional calls) and significantly slows the dashboard and stresses the API. Consider caching the renter list or resolving all user IDs in a single request per refresh.

Useful? React with 👍 / 👎.

}

private _compareByDateDesc(first?: string, second?: string): number {
const firstTime = first ? Date.parse(first) : Number.NaN;
const secondTime = second ? Date.parse(second) : Number.NaN;
Expand Down
Loading
Loading