Skip to content
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

added session update api and on_update callback #16

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ const confirmService = require('@services/apis/confirm')
const selectService = require('@services/apis/select')
const statusService = require('@services/apis/status')
const cancelService = require('@services/apis/cancel')
const sessionService = require('@services/session')
const searchService = require('@services/apis/search')
const initService = require('@services/apis/init')


exports.search = async (req, res) => {
try {
await res.status(200).send(responses.success_ack)
Expand Down Expand Up @@ -60,3 +62,13 @@ exports.status = async (req, res) => {
console.log(err)
}
}

exports.sessionUpdate = async (req, res) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

try {
console.debug(JSON.stringify(req.body, null, '\t'))
res.status(200).send(responses.success_ack)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

await sessionService.session(req.body)
} catch (err) {
console.log(err)
}
}
8 changes: 8 additions & 0 deletions src/database/storage/bap/queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,11 @@ exports.findOrCreate = async ({ where = {}, defaults = {} }) => {
throw err
}
}

exports.findByIds = async (ids) => {
try {
return await Bap.find({ _id: { $in: ids } }).lean()
} catch (err) {
console.log(err)
}
}
7 changes: 7 additions & 0 deletions src/database/storage/sessionAttendance/queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,10 @@ exports.setStatusAsCancelledById = async (id, { reasonId, reasonDesc }) => {
console.log('SessionAttendance.findByOrderId: ', err)
}
}
exports.findBySessionId = async (sessionId) => {
try {
return await SessionAttendance.find({ sessionId: sessionId })
} catch (err) {
console.log('SessionAttendance.findBySessionId: ', err)
}
}
8 changes: 8 additions & 0 deletions src/database/storage/user/queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,11 @@ exports.findOrCreate = async ({ where = {}, defaults = {} }) => {
throw err
}
}

exports.findByIds = async (ids) => {
try {
return await User.find({ _id: { $in: ids } }).lean()
} catch (err) {
console.log(err)
}
}
15 changes: 15 additions & 0 deletions src/dtos/onUpdateRequest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict'

exports.onUpdateRequestDTO = async (context, statusBody, orderId, status) => {
return {
context,
message: {
order: {
id: orderId,
state: status,
type: 'DEFAULT',
provider: statusBody,
},
},
}
}
1 change: 1 addition & 0 deletions src/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ router.post('/init', bppController.init)
router.post('/confirm', bppController.confirm)
router.post('/cancel', bppController.cancel)
router.post('/status', bppController.status)
router.post('/sessionUpdate', bppController.sessionUpdate)

module.exports = router
4 changes: 3 additions & 1 deletion src/services/protocolCallbacks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ const { onConfirm } = require('./onConfirm')
const { onSelect } = require('./onSelect')
const { onStatus } = require('./onStatus')
const { onCancel } = require('./onCancel')
const { onUpdate } = require('./onUpdate')
const { onSearch } = require('./onSearch')
const { onInit } = require('./onInit')
const protocolCallbacks = { onConfirm, onSelect, onStatus, onCancel, onSearch, onInit }

const protocolCallbacks = { onConfirm, onSelect, onStatus, onCancel, onSearch, onInit, onUpdate }
module.exports = protocolCallbacks
37 changes: 37 additions & 0 deletions src/services/protocolCallbacks/onUpdate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict'

const { contextBuilder } = require('@utils/contextBuilder')
const { onUpdateRequestDTO } = require('@dtos/onUpdateRequest')
const { externalRequests } = require('@helpers/requests')
const { internalRequests } = require('@helpers/requests')

exports.onUpdate = async (callbackData) => {
try {
const context = await contextBuilder(
callbackData.transactionId,
callbackData.messageId,
process.env.ON_UPDATE_ACTION
)
const response = await internalRequests.catalogGET({
route: process.env.CATALOG_GET_STATUS_BODY_ROUTE,
pathParams: {
sessionId: callbackData.sessionId,
fulfillmentId: callbackData.fulfillmentId,
},
})
const statusBody = response.statusBody
const onUpdateRequest = await onUpdateRequestDTO(
context,
statusBody.providers[0],
callbackData.orderId,
callbackData.status
)
await externalRequests.callbackPOST({
baseURL: callbackData.bapUri,
route: process.env.ON_UPDATE_ROUTE,
body: onUpdateRequest,
})
} catch (err) {
console.log('OnUpdate.ProtocolCallbacks.services: ', err)
}
}
50 changes: 50 additions & 0 deletions src/services/session.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use strict'

const protocolCallbacks = require('@services/protocolCallbacks/')
const sessionAttendanceQueries = require('@database/storage/sessionAttendance/queries')
const userQueries = require('@database/storage/user/queries')
const bapQueries = require('@database/storage/bap/queries')
const crypto = require('crypto')

exports.session = async (requestBody) => {
try {
const sessionId = requestBody.sessionId
const sessionAttendance = await sessionAttendanceQueries.findBySessionId(sessionId)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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).

const users = await userQueries.findByIds(userIds)
if (!users) {
return console.log('Users Not Found')
}
const bapIds = users.map((user) => user.bapId)
const baps = await bapQueries.findByIds(bapIds)
if (!baps) {
return console.log('BAP Not Found')
}
let usersWithBapAndAttendance = users.map((user) => {
const bapInfo = baps.find((f) => f._id.toString() === user.bapId.toString())
const sessionAttendanceInfo = sessionAttendance.find((f) => f.userId.toString() === user._id.toString())
return {
...user,
bapInfo,
sessionAttendanceInfo,
}
})
usersWithBapAndAttendance.map(async (user) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

await protocolCallbacks.onUpdate({
transactionId: crypto.randomUUID(),
messageId: crypto.randomUUID(),
bapId: user.bapInfo.bapId,
bapUri: user.bapInfo.bapUri,
status: user.sessionAttendanceInfo.statusText,
sessionId: sessionId.toString(),
fulfillmentId: user.sessionAttendanceInfo.fulfillmentId.toString(),
orderId: user.sessionAttendanceInfo.orderId.toString(),
})
})
} catch (err) {
console.log(err)
}
}