added session update api and on_update callback #16
Conversation
| @@ -60,3 +62,13 @@ exports.status = async (req, res) => { | |||
| console.log(err) | |||
| } | |||
| } | |||
|
|
|||
| exports.sessionUpdate = async (req, res) => { | |||
There was a problem hiding this comment.
Please move this to a session route and a session controller with the controller name being something like "update". So /session/update calling sessionController.update which in-turn calls sessionService.update.
| exports.sessionUpdate = async (req, res) => { | ||
| try { | ||
| console.debug(JSON.stringify(req.body, null, '\t')) | ||
| res.status(200).send(responses.success_ack) |
There was a problem hiding this comment.
This idea of the controller immediately responding with a success acknowledgement doesn't apply to non-dsep or internal calls. Since this is an internal call from mentoring service to bpp, we are no longer dealing with DSEP protocol here. Hence no need to give early response. Here use the regular response flow where success or failure responses are given based on the execution of controller or service logic.
| exports.session = async (requestBody) => { | ||
| try { | ||
| const sessionId = requestBody.sessionId | ||
| const sessionAttendance = await sessionAttendanceQueries.findBySessionId(sessionId) |
There was a problem hiding this comment.
"sessionAttendances" would be more apt here since you are fetching multiple sessionAttendances with common sessionId.
| if (!sessionAttendance) { | ||
| return console.log('SessionAttendance Not Found') | ||
| } | ||
| const userIds = sessionAttendance.map((sessionAttendee) => sessionAttendee.userId) |
There was a problem hiding this comment.
"sessionAttendee" can be a bit confusing here since we already have a sessionAttendeeId in the model. And sessionAttendee is a mentoring service side concept. Just "attendance" would be better than "sessionAttendee" since that doesn't change the idea of what that object represent (which is an attendance, and not attendee).
| sessionAttendanceInfo, | ||
| } | ||
| }) | ||
| usersWithBapAndAttendance.map(async (user) => { |
There was a problem hiding this comment.
When you are switching back to regular responses, a question arises about when to give back a success response to mentoring. Do we do that when all the requests being generated was successful or can we give a success response as soon as we trigger these requests (but without considering if any of them were in-fact successful). Please bring this up as a discussion point with the team.
From my perspective, we shouldn't be checking each and every request to be successful (Feels fragile). If team agrees, you can use promise.race to split the execution and give back the control to controller while requests are still being generated.
No description provided.