added sendScheduledPushes endpoint #6079
Conversation
|
@acinader @dplewis @davimacedo let me know if you have time to check out this PR. I'm happy to answer any questions about it. |
There was a problem hiding this comment.
@mrmarcsmith I'm taking a look. You should run spell check on it and remove commented out code, please.
|
|
||
| await reconfigureServer({ | ||
| scheduledPush: true, | ||
| appId: Parse.applicationId, |
There was a problem hiding this comment.
do you need to reset masterkey, appid, serverurl? isn't it already set if you don't do?
|
|
||
| // translate parseObject to body | ||
| const body = {}; | ||
| if (pushObject.has('pushTime')) body.push_time = pushObject.get('pushTime'); |
There was a problem hiding this comment.
please use {} with all control statements (if, while, etc.). I'll look at making eslint more proactive.
| } | ||
|
|
||
| const pushTime = PushController.getPushTime(body); | ||
| if (pushTime && pushTime.date !== 'undefined') { |
There was a problem hiding this comment.
you probably mean pushTime.date !=== undefined, you could cover this case in test?
There was a problem hiding this comment.
I grabbed this section of code from the current PushController.sendPush function, so we should investigate if that is a bug already released to the public. In the mean time, I'll write a test case testing this in my section of code and report back
| body['push_time'] = PushController.formatPushTime(pushTime); | ||
| } | ||
|
|
||
| // TODO: currently we increment the badge when we schedule the push. |
There was a problem hiding this comment.
My $.02: make the breaking change
| @@ -87,6 +87,7 @@ const defaultColumns: { [string]: SchemaFields } = Object.freeze({ | |||
| }, | |||
| _PushStatus: { | |||
| pushTime: { type: 'String' }, | |||
| pushDate: { type: 'Date' }, // to improve speed for tolling Scheduled Pushs | |||
|
|
||
| // always returns { result: true } | ||
| static handlePOST(req) { | ||
| if (req.auth.isReadOnly) { |
There was a problem hiding this comment.
A question for @dplewis and/or @davimacedo: why don't we need to check for the master key? I grepped through the routers to find cases and we don't ever check, i just don't know why.
There was a problem hiding this comment.
| let expDate; | ||
|
|
||
| if (pushObject.has('expiry')) { | ||
| // Has an expiration date |
| // Has an expiration date | ||
| expDate = pushObject.get('expiry'); | ||
| } else if (pushObject.has('expiration_interval')) { | ||
| // Has an expiration Interval |
| ScheduledPushRouter.sendPushFromPushStatus(pushObject, req); | ||
| } | ||
| }, | ||
| { useMasterKey: true } |
There was a problem hiding this comment.
maybe use req.auth.master here to enforce security?
| .sendScheduledPush(object, req.config, req.auth) | ||
| .catch(err => { | ||
| req.config.loggerController.error( | ||
| `_PushStatus : error while sending push`, |
There was a problem hiding this comment.
you don't need the ` here, you can use '
|
I haven’t forgotten about this, just been on Pat Leave. I’ll make these changes when I come back. |
|
@mrmarcsmith Is this branch still available? I can finish up this PR for you if you want. |
|
This PR resolves issue #6066. There are no foreseeable breaking changes.
It should be noted that this adds an additional field, "pushDate", to _PushStatus that is only populated when saving scheduled pushes. Regular pushes don't have this property set. The "pushDate" field is the "date" version of "pushTime" which is a "string". "pushDate" is used to greatly reduce the amount of processing that needs to be done to see if there are any pushes to be sent. without is we would need to fetch all scheduled pushes every time and parse their "string" type "pushTime" objects.