Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #181 +/- ##
===========================================
- Coverage 84.00% 10.76% -73.24%
===========================================
Files 7 94 +87
Lines 50 3224 +3174
Branches 24 24
===========================================
+ Hits 42 347 +305
- Misses 8 2872 +2864
- Partials 0 5 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dao-Ho
left a comment
Dao-Ho
left a comment
There was a problem hiding this comment.
BANG 🔥 fire ships! Just some cmts and you're good to go
| floors = append(floors, floor) | ||
| } | ||
| return floors, nil | ||
| } |
There was a problem hiding this comment.
Left this cmt on a your other pr but might be relevant here
I'm missing some context on what you're using this for so I'll just be adding additional info and let you use your best judgement.
If your page only requires to filter by floor, this is ok for now. But typically, filtering should be done using a post endpoint, since it's not really ideal to filter a lot of data using a query parameter. The query body is a much better use case for passing more complicated objects to the backend, and so filtering typically use query bodies to pass their objects/lists instead.
Also note that if you only want to retrieve bookings for rooms 1-50, your query would be very long.
Overall, if it's just fetching this to display a fixed amount that you know is small, than this is fine. Otherwise, I'd advise swapping over to use a POST to pass more complicated objects and data in the body.
There was a problem hiding this comment.
forgot to delete this on this pr too, but this is not needed and dont think it will be in general, so I will delete, if it is needed later on we have to refactor anyway
| func (h *GuestsHandler) GetGuestWithStays(c *fiber.Ctx) error { | ||
| id := c.Params("id") | ||
| _, err := uuid.Parse(id) | ||
| if err != nil { |
| var departureDate *string | ||
| var arrivalDate *time.Time | ||
| var departureDate *time.Time | ||
| var roomNumber *int |
| )} | ||
| </View> | ||
| ); | ||
| } |
There was a problem hiding this comment.
While it's not too readable, I don't expect this to be touched too much since it's a component. You can move some divs into its own component if you'd like, but this is optional.
I would use the color vars here tho
There was a problem hiding this comment.
refactored colors and component
Dao-Ho
left a comment
Dao-Ho
left a comment
There was a problem hiding this comment.
LGTM, this is such a big ship, great work Manuel! 🐐 🚀 You're on fire 🔥 🔥 🔥 🔥
3 participants
Description
Type of Change
Testing & Validation
How this was tested
Screenshots/Recordings
https://drive.google.com/file/d/19mld677bPCtk2kogElLd0MGSdnFn4VKR/view?usp=sharing
Unfinished Work & Known Issues
Notes & Nuances
Pre-Merge Checklist
Code Quality
Testing & CI
Documentation