Sanity check on reconcileStamp #53
Conversation
…recover it. Also ensure that `_fuzzing` doesn’t nullify it!
index.js
Outdated
| if (self.state.properties.days) self.state.reconcileStamp = self.state.properties.days * msecs.day | ||
| if ((self.state.reconcileStamp === null) || (isNaN(self.state.reconcileStamp))) self.state.reconcileStamp = 14 * msecs.day | ||
| self._log('sync', { reconcileStamp: self.state.reconcileStamp }) | ||
| return self.setTimeUntilReconcile(null, callback) |
There was a problem hiding this comment.
shouldn't this be return self.setTimeUntilReconcile(self.state.reconcileStamp, callback)
There was a problem hiding this comment.
because if we send null back, reconcile will be pushed back for 30 days
There was a problem hiding this comment.
s/ @diracdeltas / @NejcZdovc / in description above
There was a problem hiding this comment.
@NejcZdovc - you raise a good point. please let me know if the latest revision addresses it. thanks!
index.js
Outdated
| @@ -115,6 +115,13 @@ Client.prototype.sync = function (callback) { | |||
| if (typeof callback !== 'function') throw new Error('sync missing callback parameter') | |||
|
|
|||
| if (!self.state.properties) self.state.properties = {} | |||
| if ((self.state.reconcileStamp === null) || (isNaN(self.state.reconcileStamp))) { | |||
| if (self.state.properties.reconcileDate) self.state.reconcileStamp = new Date(self.state.properties.reconcileDate).getTime() | |||
| if (self.state.properties.days) self.state.reconcileStamp = self.state.properties.days * msecs.day | |||
There was a problem hiding this comment.
do you actually want self.state.properties.days to override self.state.properties.reconcileDate, or should this be else if instead of if?
|
please add some unit tests for what this function is expected to do |
After thinking about the comment from @NejcZdovc, I have concluded that simpler is better in this recovery case.
5f75f07 to
4463a59
Compare
|
@diracdeltas tests added for code change. Also added tests for |
|
apparently this broke the integration tests (my bad, i didn't know they existed so i only checked unit tests). #49 seems important for preventing this in the future. |
No description provided.