-
Notifications
You must be signed in to change notification settings - Fork 0
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
getMenteesForMentor endpoint (control, model, routes), awaiting Mongo #18
Conversation
api/src/server.ts
Outdated
@@ -19,6 +20,8 @@ app.use("/workshop", routes.workshop); | |||
|
|||
connectDB(); | |||
|
|||
app.use("/api", router); // connect routes (prefix as api), what is the lin 7 logic for?: import * as routes from "./routes/index" |
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.
It imports modules user and workshop on line 18 in this file
Updated mentorController with retrieval logic aligned with user object structure, abstracted user model so that scope wasn't limited to user route, resolved path import that was preventing npm run dev, included necessary nodemon dependencies? Tested w/ Mongo Implementation & Postman: This request: GET - http://localhost:8000/api/mentor/674d179b846a2c3069b9cfbb/mentees Added the following objects to Mongo: Mentee 1 Mentee 2 Must ensure that mentees in the 'menteeInfo' field array are represented by ObjectIDs rather than normal strings. |
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.
looks good to me -- resolve merge conflicts and it can be merged!
I resolved conflicts in the web editor but could someone take a second look at api/src/server.ts, specifically the app.use statements? previously there was a "app.use("/api", routes.workshop)" which might interfere with "app.use("/api", router)". Instead I added "app.use("/workshop", routes.workshop)" and kept the /user and /api statements, for which the backend worked on my local repo |
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.
i think it should work! you can merge
No description provided.