Sb 875 notification service #103
Conversation
…ervice # Conflicts: # src/app/helpers/tenantHelper.js
| * If the payload is valid structure then it will returns the payload object else it will return the null | ||
| */ | ||
|
|
||
| get () { |
There was a problem hiding this comment.
this method can be removed.
| @@ -0,0 +1,80 @@ | |||
| let Joi = require('joi') | |||
There was a problem hiding this comment.
suggest renaming this file. this is not a service.
| * @return {boolean} If the playload is in valid structure then it will returns the true else false | ||
| */ | ||
| __validate (payload) { | ||
| let validation = Joi.validate(payload, Joi.object().keys({ |
There was a problem hiding this comment.
this validation configuration (Joi.object().keys(...)) should be an attribute of the class itself.
There was a problem hiding this comment.
also, the notification payload is being created in code -- it is not user-submitted. does it need to be validated?
| return status | ||
| } | ||
|
|
||
| __create () { |
There was a problem hiding this comment.
this method could also be removed.
| const _ = require('lodash') | ||
| let _defaultDateFormat = dateFormat(new Date(), 'yyyy-mm-dd HH:MM:ss:lo') | ||
|
|
||
| class notificationPayload { |
There was a problem hiding this comment.
class names should start with an upper-case letter.
| class notificationService { | ||
| constructor (config) { | ||
| this.suportedType = config.suportedType || _supportedType | ||
| this.userAccessToken = config.userAccessToken |
There was a problem hiding this comment.
this should not be managed here, it should be managed at the httpService level. please move out from this class.
| }); | ||
| } | ||
| }); | ||
| let config = { |
There was a problem hiding this comment.
let's do some pair-programming on this snippet tomorrow.
There was a problem hiding this comment.
Here's the gist from our session: https://gist.github.com/kochhar/7e1c27b88020ab053e7cb2fc56bf28a0
| * @param {object} options request object | ||
| * @return {object} response or error object | ||
| */ | ||
| __httpService (options) { |
There was a problem hiding this comment.
abstract out this http service. use a common one which is also being used by the object rest class.
| * @param {string} key User access token | ||
| * @return {object} | ||
| */ | ||
| __getRequestHeader (key) { |
There was a problem hiding this comment.
please move into the common http service container.
| payload: payload | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
take a look at this gist: I think it does what you need:
https://gist.github.com/kochhar/273438f0afbc3afd7a4f5cebcd10e44e
|
https://gist.github.com/kochhar/7e1c27b88020ab053e7cb2fc56bf28a0
|
| * | ||
| * let target = new NotificationTarget({geo:{ids:['432-5435-6566']}}) | ||
| */ | ||
| constructor({ |
There was a problem hiding this comment.
suggest changing the constructor to take args like geo
Issue #SB-26013 : feat : observation for teacher
@kochhar @gsbajaj72
This patch is related to notification service. Please review it and let me know if any changes are needed.