Francisco Rosa Herrero - #16
Open
apkouk wants to merge 38 commits into
Open
Conversation
- Implemented create, update, and delete endpoints for guests with validation and error handling. - Added booking validation logic and exception classes. - Updated reservation creation to validate input and check guest/room existence. - Modified database schema to include guest surname. - Enhanced Room model with room number validation. - Updated repositories for new guest and reservation logic. - Switched frontend booking to real API call. - Added comprehensive unit tests for booking validation. - Bumped package version and improved .gitignore.
Implement conflict detection for overlapping room reservations. Return HTTP 409 Conflict when a reservation overlaps. Add `ReservationConflictValidator` for validation logic and refactor `ReservationDb` for clarity.
Introduce CreateReservationTests to verify ReservationRepository's CreateReservation logic, including conflict detection and edge cases. Add Microsoft.Data.Sqlite package for in-memory database testing.
Implement staff login and upcoming reservations endpoints on the backend. Add StaffLoginPage and StaffReservationsPage with routing and API hooks on the frontend. Update LandingPage to link to staff login. Enable staff to view and filter upcoming reservations.
Replaced staffAccessCode in config files with a placeholder and added UserSecretsId to the project. Updated README with instructions for setting the access code using dotnet user-secrets. Explicitly set AllowedHosts in development settings.
Implemented backend and frontend support for staff to check in guests. Added a new API endpoint to handle check-in with guest email validation, updated reservation and room state logic, and introduced a dialog in the staff UI for confirming check-in.
Introduce housekeeping section for staff to view and update room states (Ready, Occupied, Dirty). Add RoomStateDialog for state changes. Backend now prevents check-in to dirty rooms and provides an endpoint to set room state. Improve guest and room repository validation. Change repository DI from singleton to scoped. Update frontend to fetch and manage room states.
There was a problem hiding this comment.
Pull request overview
Implements a staff-facing workflow for hotel reservations (login, upcoming reservations list, check-in) and housekeeping (room state updates), alongside backend validation for bookings and reservation conflicts, plus initial NUnit coverage for validators/repository behavior.
Changes:
- Add staff UI routes/pages (login + reservations/housekeeping) and wire UI booking to real API calls.
- Implement reservation conflict detection + booking validation, plus guest CRUD enhancements and upcoming reservations query.
- Introduce staff endpoints + check-in flow + room-state update endpoint, and add NUnit test project with core validation/conflict tests.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/staff/api.ts | Staff/housekeeping ky + react-query hooks and Zod schemas |
| ui/src/staff/StaffReservationsPage.tsx | Staff reservations table, check-in dialog, and housekeeping room state UI |
| ui/src/staff/StaffLoginPage.tsx | Staff login form and navigation to staff area |
| ui/src/router.tsx | Adds /staff/login and /staff/reservations routes |
| ui/src/reservations/api.ts | Hooks booking flow to real POST endpoint (replaces stub) |
| ui/src/LandingPage.tsx | Adds navigation to staff login from landing page |
| ui/package-lock.json | Bumps UI package version in lockfile |
| readme.md | Documents local secret setup for staff access code |
| api/appsettings.json | Removes plaintext staff code from committed settings (placeholder value) |
| api/appsettings.Development.json | Aligns dev settings and placeholders for staff access code |
| api/api.csproj | Adds UserSecretsId for local secrets support |
| api/Validators/ReservationConflictValidator.cs | Centralizes reservation conflict exception behavior |
| api/Validators/BookingValidator.cs | Adds booking validation (room number, email, date range) |
| api/Repositories/RoomRepository.cs | Adds DB update for room state |
| api/Repositories/ReservationRepository.cs | Implements create + conflict check, upcoming query, and check-in persistence |
| api/Repositories/GuestRepository.cs | Adds guest existence check, conflict on create, and update behavior |
| api/Program.cs | Switches DB/repositories to scoped lifetime; API remains mounted at /api |
| api/Models/Room.cs | Adds room-number validation helper |
| api/Models/Errors/InvalidBooking.cs | New exception type for booking validation failures |
| api/Models/Errors/ConflictException.cs | New exception type for conflicts (e.g., overlaps, duplicates) |
| api/Db/Setup.cs | Adds Surname column support and fixes typos in setup |
| api/Controllers/StaffController.cs | Adds staff reservations endpoint; adjusts login behavior/cookie settings |
| api/Controllers/RoomController.cs | Adds endpoint to set room state |
| api/Controllers/ReservationController.cs | Adds booking validation + check-in endpoint and related checks |
| api/Controllers/GuestController.cs | Adds create/update/delete guest endpoints with validation |
| api.Tests/api.Tests.csproj | Introduces NUnit test project |
| api.Tests/CreateReservationTests.cs | Tests reservation creation and overlap detection against in-memory SQLite |
| api.Tests/BookingValidatorTests.cs | Tests booking validator behavior (room/email/dates) |
| .gitignore | Ignores Copilot snapshots and .NET build artifacts |
Files not reviewed (1)
- ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Previously, rooms were marked as Dirty after check-in. Now, the room state is set to Occupied to better reflect its current status during the check-in process.
Changed response from BadRequest (400) to NotFound (404) when a NotFoundException is caught, ensuring the API returns a more accurate HTTP status for missing resources.
Move IsNotStaff method to new StaffAuth static class for centralized staff authentication. Update RoomController and StaffController to use StaffAuth.IsNotStaff for consistent access checks based on the "access" cookie.
Throw NotFoundException in SetRoomState if no rows are updated, ensuring callers are notified when a specified room does not exist. This enhances error reporting and robustness.
Implemented authentication and "StaffOnly" authorization policy using a custom handler that checks the "access" cookie. Applied [Authorize(Policy = "StaffOnly")] to Guest, Reservation, and Room controllers, and the StaffController's /reservations endpoint. Removed manual staff checks and updated Program.cs to configure authentication/authorization middleware. Improved cookie security by setting the Secure flag based on HTTPS.
Added a validation in ReservationRepository.cs to throw an InvalidOperationException when attempting to check in a reservation that has already been checked out, ensuring reservation state consistency.
Introduced CheckInTests to validate reservation check-in behavior and room state persistence using an in-memory SQLite database. Tests cover successful check-ins (with case-insensitive email), error scenarios, unknown reservation handling, and room state updates. Setup and teardown ensure isolated test environments.
Expanded the check-in condition to block check-in for any room state other than 'Ready', not just 'Dirty'. Updated the error message to match the new logic.
Changed API URLs in api.ts to include a leading slash, ensuring requests are sent to the correct absolute server paths for reservations and rooms.
Replaced direct length checks with Room.IsValidRoomNumber in GetRoom, CreateRoom, DeleteRoom, and SetRoomState actions. This centralizes and standardizes room number validation logic across endpoints, and adds validation to CreateRoom.
Updated CookieOptions in StaffController.cs to specify the Path property as "/api", restricting the cookie to API requests only.
Replaced exception-based check for existing "Surname" column in the "Guests" table with a schema query using `pragma_table_info`. Now, the column is only added if it does not already exist.
Agent-Logs-Url: https://github.com/apkouk/reservations-interview/sessions/da694a2d-2187-4e80-96cd-1ce6ac7a7535 Co-authored-by: apkouk <9098732+apkouk@users.noreply.github.com>
Improve Test Coverage Across Repositories, Validators, and Models
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated 10 comments.
Files not reviewed (1)
- ui/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
api/Db/Setup.cs:15
Setup.EnsureDbis declared asasync void, which means it can’t be awaited and exceptions thrown after the firstawaitwon’t be caught by the caller. With the new migration logic (extra awaits), startup can race with DB initialization and failures may go unnoticed. Change the signature toTaskand ensure the caller awaits it (and disposes the created scope).
public static async void EnsureDb(IServiceScope scope)
{
using var db = scope.ServiceProvider.GetRequiredService<SqliteConnection>();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add CheckForConflictTests for comprehensive overlap testing - Refactor ReservationRepositoryTests with parameterized cases - Store reservation dates as ISO TEXT for correct comparisons - Clarify room number validation error messages - Add GuestRepository.GetOrCreateGuest for on-demand guest creation - Use CreatedAtAction in ReservationController for Location header - Remove obsolete StaffAuth.cs; use attribute-based auth - Update authorization: allow anonymous booking, restrict staff actions - Remove unused staffAccessCode from appsettings files
Refactor reservation check-in to atomically update both reservation and room state in a single repository call. Change EnsureDb to async Task and properly await it during app startup, ensuring database setup completes before continuing. Adjust middleware order for correct CORS and authentication handling.
Implemented a generic useSortableTable React hook for client-side sorting. Updated StaffReservationsPage to use sortable columns with clickable headers and sort indicators. Ensured stable sorting for reservation status. Added a database connection check before transactions in ReservationRepository.cs.
Implement signed staff authentication using ASP.NET Core Data Protection to prevent cookie forgery. Add global error handling in the frontend with TanStack Query, a new /error route, and a user-friendly ErrorPage component. Update router for 404 and error states. Add comments to AllowedHosts in appsettings.
- Change NoOpAuthenticationHandler challenge status from 403 to 401 - Rename GetRoom to GetReservation in ReservationController - Return 500 with generic message for reservation errors - Clarify InvalidRoomNumber exception message - Optimize GuestExists with COUNT query in GuestRepository - Fix NotFoundException message in ReservationRepository
Correct CreatedAtAction usage in ReservationController to use GetReservation. Add and configure forwarded headers middleware in Program.cs for reverse proxy support.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 43 changed files in this pull request and generated 8 comments.
Files not reviewed (1)
- ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Added validation to ensure RoomNumber and GuestEmail are not null, empty, or whitespace in BookingValidator.Validate. Throws InvalidBooking with a clear message if validation fails.
Ensure Reservations_new temp table is dropped before migration to prevent conflicts. Update migration to cast Start/End columns to TEXT directly, since Dapper stored ISO-8601 strings, eliminating the need for epoch conversion.
Updated the GuestController to build the Location header using Request.PathBase and URI-escaped email addresses. This change ensures the generated URLs are correctly formatted and safe for use in HTTP headers.
Removed the comment warning about arbitrary Host headers from appsettings.Development.json and appsettings.json. The AllowedHosts configuration remains unchanged.
Limit trusted proxies to 127.0.0.1 and ::1, ensuring only local reverse proxy (e.g., Caddy) forwarded headers are accepted. This prevents external spoofing of X-Forwarded-For and X-Forwarded-Proto, improving security.
Enhanced validateSearch to ensure status is a valid HTTP code (100-599) and message is a string, defaulting to 500 and undefined otherwise. This increases robustness against invalid query parameters.
Refactored ReservationDb class for clarity and consistency. Added error handling to ensure room state updates during check-in affect exactly one room. Introduced DeleteReservation method to support reservation deletion by ID.
Convert reservation room number to integer before updating room state in the database. Add check to ensure exactly one room is updated, throwing an exception if the update fails.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 43 changed files in this pull request and generated 6 comments.
Files not reviewed (1)
- ui/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Return 201 without Location header for anonymous reservations - StaffController returns 500 if access code is misconfigured - Standardize service variable naming and clarify proxy comments - Ensure transaction rollback on check-in failure in repository - Add type-safe compareValues for improved table sorting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Francisco Rosa Herrero
Task Report
This is a table containing all task completed and time spent on each one:
The full implementation took approximately 7.25 hours (430minutes). R001 and R004 required the most time, as both involved to fix unexpected errors (FK constraints and compilation issues) rather than just implementation work.
Task Description
This PR implements a hotel reservations management system covering guest management, booking logic, staff operations, and housekeeping built with ASP.NET Core and SQLite.
Note: R005 was intentionally skipped as it was considered out of scope for this implementation.
Solution Setup
Nothing remarkable, I just needed to install Caddy
R001
Guest CRUD endpoints with booking validation. A guest FK constraint required implementing guest creation before reservations could be added. Input validations (duplicate email checks, update conflicts) are functional but flagged for deeper review.
R002
Logic to detect and reject overlapping reservations. Test coverage added for reservation creation using an in-memory SQLite database.
R003
Staff-facing UI and API for managing reservations. Sensitive configuration (staff access code) moved to .NET user secrets.
R004
Staff can check guests in against existing reservations.
R006
Housekeeping interface with room state tracking (clean/dirty status).
Setup
Run the following commands from the api folder:
dotnet user-secrets init
dotnet user-secrets set "staffAccessCode" "pass"
The current staff access code is pass. See Known Limitations for notes on how this should be handled in production.
Security
A Semgrep OSS SAST scan was run across all 63 tracked files using 249 rules — 0 findings. Note this covers first-party code vulnerabilities; a full scan including Supply Chain (SCA) dependency analysis would require Semgrep Code.
Notes
To make the app run execute these two commands in the api folder:
Now the right pass is pass (really creative). This should be in an Azure KeyVault or any other service to keep secrets secure.
Known Limitations & Future Improvements
These are intentional shortcuts made for scope — in a production system I would address all of the following:
Solution structure: Migrate to a .sln file to enable proper debugging and multi-project support in Visual Studio and Rider.
Authentication: Replace the plaintext access code with a proper identity server using JWT. On successful login, dispatch an event via ServiceBus or an Outbox pattern to send a one-time code by email, then validate it through a dedicated endpoint and apply role-based authorization via middleware.
Database: Migrate from SQLite to Azure SQL Database for persistence and easier management.
Secret management: Move the staff access code to Azure Key Vault.
Observability: Add Serilog or Application Insights for structured logging and error monitoring.
API hardening: Review status codes and response messages across all endpoints, add CORS restrictions, and apply rate limiting.
Deployment: Containerize with a Dockerfile and deploy to a Linux App Service on Azure.
UI/UX: The current frontend is functional but intentionally minimal. The overall design could be more user-friendly, and a proper navigation menu including a logout option should be added.