-
Notifications
You must be signed in to change notification settings - Fork 74
Addresses #249 with some testing already done #292
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: develop
Are you sure you want to change the base?
Conversation
@simar-rekhi, can you check the build fail? |
@mikehquan19, sure! |
tl; dr: both ubuntu and windows are building successfully now I think it was an issue related to Swagger Annotation. The documents we are essentially returning to the client had to be wrapped in an APIResponse Wrapper if they were to stay an {object}. |
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.
Hey @simar-rekhi, you are making great progress so far. There's one issue.
The route you've implemented is actually /events/:day/:building/:room
instead of /events/:day/:building/:room/sections
.
For /events/:day/:building/:room/sections
, you would actually have to replace the ID of the sections with the actual section object.
So my suggestion is:
- corrrect the endpoint of the route you have implemented
- implement another one that uses the aggregate pipeline (sorry to say that) to pull the sections from the ids.
Also, try to resolve the conflicts you have since I updated new stuff. Reach out to me if you have questions.
@mikehquan19, I'll try to finish this in today's meeting. |
Fixes #249