feat: add booking workflow detail page for staff by nnh53 · Pull Request #33 · Office-Hour/EV-Rental-FE

nnh53 · 2025-11-11T17:00:15Z

Summary

add a booking workflow detail route for staff bookings, backed by a workflow service that executes the required API sequence
implement the staff booking detail component with reactive forms, workflow status display, and dedicated styling
update the staff dashboard overlay and routes to link to the new workflow page

Testing

pnpm lint
pnpm build

Codex Task

naming-conventions-bot · 2025-11-11T17:00:19Z

Thank you for following naming conventions! 😻

cloudflare-workers-and-pages · 2025-11-11T17:01:55Z

Deploying ev-rental with Cloudflare Pages

Latest commit:	`b19d1f6`
Status:	✅ Deploy successful!
Preview URL:	https://611cee9b.ev-rental.pages.dev
Branch Preview URL:	https://codex-implement-booking-deta.ev-rental.pages.dev

View logs

Copilot

Pull Request Overview

This PR introduces a comprehensive booking workflow feature for staff, enabling them to execute the complete rental handoff process through a multi-step guided interface. The implementation includes a new detail page, a dedicated workflow service orchestrating API calls, and navigation enhancements to the staff dashboard.

Key Changes:

Added a reactive forms-based booking workflow detail page with step-by-step status tracking for rental handoff operations
Implemented BookingWorkflowService to orchestrate the sequential API workflow (check-in, rental creation, contract signing, inspection, vehicle receipt)
Enhanced staff dashboard with navigation to the new workflow page via a "Continue workflow" button

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
`staff.routes.ts`	Adds dynamic route for booking detail page (`bookings/:bookingId`)
`staff-dashboard.ts`	Implements navigation method `goToBookingWorkflow()` to route to workflow page
`staff-dashboard.html`	Adds "Continue workflow" button in detail panel footer
`staff-dashboard.scss`	Styles for detail panel action button with hover effects
`booking-detail.ts`	Core component with reactive forms, computed signals, and workflow orchestration logic
`booking-detail.html`	Template with booking summary, workflow steps display, and comprehensive signature/inspection forms
`booking-detail.scss`	Comprehensive styling for workflow UI including steps, forms, badges, and responsive layout
`booking-workflow.service.ts`	Service managing workflow state and executing sequential API calls with error handling

Copilot · 2025-11-11T17:06:12Z

+  constructor() {
+    this.workflowService.reset();
+  }


The constructor calls this.workflowService.reset() which could cause side effects if multiple instances of this component are created. Services are singletons in Angular, so this could reset workflow state for other potential uses of the service. Consider moving this to ngOnInit or using a local workflow state instead.

Copilot · 2025-11-11T17:06:13Z

+  transition: background 0.2s ease, transform 0.2s ease;
+}
+
+.details-panel__action:hover {


[nitpick] Using color-mix(in srgb, ...) for hover effects may not be supported in older browsers. While this is likely acceptable for a modern Angular 20 app, consider adding a fallback or documenting the minimum browser requirements if not already done.

Suggested change

.details-panel__action:hover {

.details-panel__action:hover {

/* Fallback for browsers that do not support color-mix */

background: var(--mat-sys-primary, #2563eb);

Copilot · 2025-11-11T17:06:13Z

+    this._result.set(null);
+  }
+
+  executeWorkflow(payload: BookingWorkflowPayload): Observable<BookingWorkflowSummary> {


The executeWorkflow method doesn't prevent concurrent executions. If a user clicks "Run workflow" multiple times rapidly, multiple workflow executions could start before the _running flag is set. Consider adding a guard at the beginning: if (this._running()) return throwError(...) to prevent concurrent workflow executions.

Suggested change

executeWorkflow(payload: BookingWorkflowPayload): Observable<BookingWorkflowSummary> {

executeWorkflow(payload: BookingWorkflowPayload): Observable<BookingWorkflowSummary> {

if (this._running()) {

return throwError(() => new Error('A workflow is already running. Please wait for it to complete before starting a new one.'));

}

Copilot · 2025-11-11T17:06:13Z

+  cursor: not-allowed;
+}
+
+.workflow__submit:hover:enabled {


[nitpick] Using color-mix(in srgb, ...) for hover effects may not be supported in older browsers. While this is likely acceptable for a modern Angular 20 app, consider adding a fallback or documenting the minimum browser requirements if not already done.

Suggested change

.workflow__submit:hover:enabled {

.workflow__submit:hover:enabled {

/* Fallback for browsers that do not support color-mix */

background: #3b82f6;

Copilot · 2025-11-11T17:06:14Z

+    if (batteryCapacity === null || batteryCapacity === undefined) {
+      return null;
+    }
+
+    return {
+      bookingId: record.bookingId,
+      staffId: staffId ?? undefined,
+      rentalStart: record.startTime ?? undefined,
+      rentalEnd: record.endTime ?? undefined,


The _buildPayload method only checks if batteryCapacity is null/undefined but doesn't validate other required fields like staffId, rentalStart, or rentalEnd. If these are undefined, the API calls in the workflow service may fail. Consider adding validation for all critical fields or documenting which fields are optional.

Suggested change

if (batteryCapacity === null || batteryCapacity === undefined) {

return null;

}

return {

bookingId: record.bookingId,

staffId: staffId ?? undefined,

rentalStart: record.startTime ?? undefined,

rentalEnd: record.endTime ?? undefined,

// Validate all required fields before building the payload

if (

batteryCapacity === null || batteryCapacity === undefined ||

staffId === null || staffId === undefined ||

record.startTime === null || record.startTime === undefined ||

record.endTime === null || record.endTime === undefined

) {

return null;

}

return {

bookingId: record.bookingId,

staffId: staffId,

rentalStart: record.startTime,

rentalEnd: record.endTime,

Copilot · 2025-11-11T17:06:16Z

+      return new Date().toISOString();
+    }
+
+    const parsed = new Date(value);
+    if (Number.isNaN(parsed.getTime())) {
+      return new Date().toISOString();


The _toIsoString method falls back to new Date().toISOString() when the input is invalid. This could lead to incorrect timestamps being sent to the API without the user knowing. Consider throwing an error or returning a validation error instead of silently using the current time as a fallback.

Suggested change

return new Date().toISOString();

}

const parsed = new Date(value);

if (Number.isNaN(parsed.getTime())) {

return new Date().toISOString();

throw new Error('Invalid date value: value is empty or undefined');

}

const parsed = new Date(value);

if (Number.isNaN(parsed.getTime())) {

throw new Error(`Invalid date value: "${value}" is not a valid date`);

Copilot · 2025-11-11T17:06:17Z

  }

+  goToBookingWorkflow(bookingId: string): void {
+    if (!bookingId) {


The goToBookingWorkflow method checks if bookingId is falsy but the parameter is typed as string, which could be an empty string. Consider checking for empty string explicitly: if (!bookingId || bookingId.trim().length === 0) to prevent navigation with empty booking IDs.

Suggested change

if (!bookingId) {

if (!bookingId || bookingId.trim().length === 0) {

Copilot · 2025-11-11T17:06:17Z

+  }
+
+  private _buildPayload(record: StaffBookingRecord): BookingWorkflowPayload | null {
+    const staffId = this.userService.user?.id ?? undefined;


The userService.user?.id access pattern could potentially return undefined, which is then assigned to staffId and used throughout the workflow. Consider adding validation or a fallback to ensure staffId is never undefined when it's required for the workflow. This could lead to API errors if staff identification is mandatory for workflow steps.

Suggested change

const staffId = this.userService.user?.id ?? undefined;

const staffId = this.userService.user?.id;

if (!staffId) {

return null;

}

Copilot · 2025-11-11T17:06:17Z

+            @if (
+              inspectionGroup.controls.batteryCapacity.invalid &&
+              inspectionGroup.controls.batteryCapacity.touched
+            ) {
+              <span class="form-error">Enter the current battery capacity.</span>
+            }


The error messages shown inline (e.g., "Enter the current battery capacity.") don't have proper ARIA attributes to associate them with their input fields. Add aria-describedby to the input fields and corresponding IDs to the error messages for better screen reader support. For example: <input aria-describedby="inspection-battery-error" ...> and <span id="inspection-battery-error" class="form-error" ...>.

Copilot · 2025-11-11T17:06:18Z

+    this.formBuilder.group<BookingWorkflowFormModel>({
+      inspection: this.formBuilder.group<InspectionFormGroup>({
+        batteryCapacity: this.formBuilder.nonNullable.control<number>(80, {
+          validators: [Validators.required, Validators.min(0)],


The battery capacity input has min="0" in the HTML and Validators.min(0) in the TypeScript, but there's no maximum validation. A battery capacity value could be set to an unrealistically high number (e.g., 999999). Consider adding Validators.max() with a reasonable upper limit based on your vehicle specifications.

Suggested change

validators: [Validators.required, Validators.min(0)],

validators: [Validators.required, Validators.min(0), Validators.max(150)],

Copilot AI review requested due to automatic review settings November 11, 2025 17:00

nnh53 added the codex label Nov 11, 2025 — with ChatGPT Codex Connector

Copilot started reviewing on behalf of nnh53 November 11, 2025 17:00 View session

Copilot finished reviewing on behalf of nnh53 November 11, 2025 17:03

Copilot AI reviewed Nov 11, 2025

View reviewed changes

feat(staff): add booking workflow detail route

b19d1f6

nnh53 force-pushed the codex/implement-booking-detail-routing-and-api-sequence branch from 14ecc24 to b19d1f6 Compare November 11, 2025 17:10

	if (!bookingId) {
	if (!bookingId \|\| bookingId.trim().length === 0) {

	validators: [Validators.required, Validators.min(0)],
	validators: [Validators.required, Validators.min(0), Validators.max(150)],

Conversation

nnh53 commented Nov 11, 2025

Summary

Testing

Uh oh!

naming-conventions-bot Bot commented Nov 11, 2025

Uh oh!

cloudflare-workers-and-pages Bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying ev-rental with Cloudflare Pages

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cloudflare-workers-and-pages Bot commented Nov 11, 2025 •

edited

Loading