Ibrahim Saad - #9
Open
Ibrahim5aad wants to merge 11 commits into
Open
Conversation
…ed error model hierarchy Introduce ResourceError base error/exception class with ResourceType/ResourceId Replace controllers-level try/catch with centerlized ExceptionHandlingMiddleware Configure structured logging and replace Console.WriteLine with proper logging
Reservation/booking endpoint has FluentValidation for request validation (dates, email, room number format), room existence check, and implicit guest creation. Validation errors tunnled through exception handled middleware. Room numbers stored as TEXT with proper format validation.
Scoped DB connections/abstractions No point in using WAL mode with one singleton connection = sqlite serlializes concurrent reentrance
Add JWT authentication for staff using shared access code Reservations endpoint supports filtering by date range, room, and email UI includes login dialog, staff reservations table with filters Add 401 interceptor for auto-logout on token expiry
Dapper takes care of the connection for other queries, but the ADO transaction we added need to get opened if closed. Integration tests were passing desbite this becuase of the test factory opens connections eagerly on creation.
Add check-in endpoint that validates guest email, reservation dates and marks room as occupied. Staff UI shows check-in status badges and check-in button for reservations.
Add check-out flow (API endpoint, repository, UI), room state PATCH endpoint, and housekeeping controls (mark clean/dirty)
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. Pull Request Developer GuidanceAction Required: Please review and fix any Critical or High severity findings identified above.
Need help or have questions? Reach out to the Security team on Slack: #rnd-wiz |
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.
Ibrahim Saad
Overview
All tasks are completed. First three (RE-001, RE-002, RE-003) are done within the stated 3 hours time frame (+ breaks).
The rest of tasks (RE-004, RE-005, RE-006) are done outside that time frame.
Task Report
Commits per task
1502032feat(api): implement guest room booking with validatione143a99feat(ui): implement booking flow with validation errors and confirmation dialogdb6a734feat(api): guarding against reservation conflicts/overlaps04e277cfix(api): concurrent overlapping booking requests64c3bb1fix(api): open create reservation transactions if closed84980e1feat: staff login with JWT auth and reservations view3dcc345feat: staff can check in guests0027eb1feat: staff room management with CSV import and room listing UIfe0047bfeat: add check-out, room state management, and housekeepingTasks overview
The commit log is a rough overview of the time spent, some breaks were took between tasks.
- Initial easy wins / refactor
Some initial easy wins that I thought will help lay down some proper foundation for any coming work, mainly exception handling (dont need to care about it afterwards or handle exceptions in larger surface area), logging and proper API error responses. Captured in commits
87c48fbandc668075.Expected behavior: Exceptions handling responsibility are offloaded from repositories and controllers, centralized in
ExceptionHandlingMiddlewarefor convenience (one place to catch our custom errors, handle, set informative API error responses). Custom erorr hierarchy were added as well.(RE-001) Guest Booking
Around an hour spent in this task. The work is divided between two commits;
1502032for API-related work, ande143a99for UI-related work.Expected behavior: Guests can book their room by selecting a room card, entering their email and date range in a modal, and submitting. The API validates all inputs via FluentValidation (email format with domain, room number format
###, start before end, 1-30 day duration, no past dates) and checks that the room exists. On success, a confirmation dialog shows the reservation details. On failure, validation errors are shown as toast notifications.(RE-002) Booking Validations
Around 30 minutes spent. The work is divided between two commits;
db6a734for the overlap detection query, and04e277cfor handling concurrent booking race conditions. A small follow-up fix in64c3bb1.Expected behavior: Double bookings are prevented by checking for overlapping reservations on the same room before inserting. A serializable transaction wraps the multistep booking operation to prevent race conditions from concurrent requests.
(RE-003) Staff Login
Around an hour spent in
84980e1,.Expected behavior: Staff authenticate with a shared access code via a login dialog. The backend validates the code against configuration, issues a JWT token, and the frontend stores it in sessionStorage. Authenticated staff see navigation links to Reservations. The reservations list defaults to showing today and future reservations, with filters for date range, room, and guest email.
(RE-004) Check In
Captured in commit
3dcc345.Expected behavior: Staff can check in a guest for any reservation that covers today. Check-in requires email confirmation matching the reservation. The backend validates that the the reservation is not already checked in, and today falls within the reservation period, and the room is not dirty. On success, the reservation is marked as checked in and the room state is set to Occupied. Both updates are wrapped in a transaction for atomicity.
(RE-005) CSV Import
Captured in commit
0027eb1.Expected behavior: Staff can import rooms via a CSV file (up to 500 rooms) through a drag-and-drop dialog. The import result shows counts of imported and failed rows, with a detailed error table listing line numbers and failure reasons. Valid rooms are atomically inserted within a transaction.
(RE-006) Housekeeping
Captured in commit
fe0047b.Expected behavior: Staff can mark rooms as clean or dirty from the Rooms page. Check-out marks the room as dirty. Staff cannot check in a guest to a dirty room (the check-in button is disabled and the backend also enforces this). The Rooms page supports filtering by status (Ready/Occupied/Dirty) and by floor, with pagination.
Notes
Key decisions:
ExceptionHandlingMiddleware+ exception/error hierarchyEncountered along the way:
Future considerations: