-
Notifications
You must be signed in to change notification settings - Fork 0
feat: added Get /requests endpoint for getting all requests #82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
danctila
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall fire first pass at the UI and endpoints - looking very clean and the structure is solid overall, a few things to fix are below.
also make sure to fill out the PR description since its helpful to have more context on what was implemented / intentionally pushed off, any design decisions, etc. just a few bullet points and a screen recording of the flow workout would go a long way 🙏 (also these are good to look back on bc you can explicitly see where you contributed in each PR on your github)
Must fixes / blocking:
- API endpoint mismatch b/t
/requestand/requests - Fix 500 error in POST - see the
completed_atproperty in the INSERT / GET statement in/repository/requests.go
Should fix:
- Remove mock data once API is working
- Setup proper query invalidation pattern (see the suggestion)
- Add error state handling in the UI
- Fix the long type annotation in the map function
Nice to have:
- Add TODO comment for hardcoded hotel ID / remove hardcoded data altogether
- Changed backend route from /request to /requests for consistency - Removed completed_at from GetAllRequests SELECT statement - Removed mock data from frontend - Added query invalidation pattern in useCreateRequest hook - Added error state with retry button in UI - Fixed type annotation in map function - Added TODO comment for hardcoded values
Description
Added GET /requests endpoint to retrieve all requests
Type of Change
Related Issue(s)
Closes #69
What Changed?
Testing & Validation
How this was tested
Screenshots/Recordings
Unfinished Work & Known Issues
Notes & Nuances
Pre-Merge Checklist
Code Quality
Testing & CI
Documentation
Reviewer Notes