Issue 239 stop old due dates #269
Conversation
|
Code Climate has analyzed commit 7982bdf and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 50.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 21.1% (0.1% change). View more on Code Climate. |
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| let { eventTitle, isAllDay, startDate } = parseSherlock({ text, timezone }) | ||
|
|
||
| if ( moment(startDate).isBefore(moment().subtract(1, 'years')) ) { | ||
| log.debug('Caught and fixed promise with old parsed start date of:', startDate) |
There was a problem hiding this comment.
Line 36 exceeds the maximum line length of 80.
There was a problem hiding this comment.
good idea to log this, we should probably have it as a warn with a more succinct message like 'parsePromise rejected past due date:'
|
|
||
| if ( moment(startDate).isBefore(moment().subtract(1, 'years')) ) { | ||
| log.debug('Caught and fixed promise with old parsed start date of:', startDate) | ||
| startDate = null; // clear startDate if more than a year ago |
Codecov Report
@@ Coverage Diff @@
## master #269 +/- ##
==========================================
+ Coverage 21.06% 21.12% +0.05%
==========================================
Files 28 28
Lines 674 677 +3
==========================================
+ Hits 142 143 +1
- Misses 532 534 +2
Continue to review full report at Codecov.
|
chrisbutler
force-pushed
the
issue_239_stop_old_due_dates
branch
from
7982bdf to
9cc7fb7
Compare
| let { eventTitle, isAllDay, startDate } = parseSherlock({ text, timezone }) | ||
|
|
||
| if ( moment(startDate).isBefore(moment().subtract(1, 'years')) ) { | ||
| log.debug('Caught and fixed promise with old parsed start date of:', startDate) |
There was a problem hiding this comment.
good idea to log this, we should probably have it as a warn with a more succinct message like 'parsePromise rejected past due date:'
| const { eventTitle, isAllDay, startDate } = parseSherlock({ text, timezone }) | ||
| let { eventTitle, isAllDay, startDate } = parseSherlock({ text, timezone }) | ||
|
|
||
| if ( moment(startDate).isBefore(moment().subtract(1, 'years')) ) { |
There was a problem hiding this comment.
so, i think that this check/override should only happen when we create the the promise, so that this function still gives us what we expect
also, this will need to be a timezone aware check because we're going to change the criteria to reject any date that is in the past i.e. < now (before, at a 1 year scale, being within 24 hours wouldn't really matter)
this should happen before that closeOfBusiness check so that the default will be applied (and you can look at that function for timezone handling)
|
|
||
| if ( moment(startDate).isBefore(moment().subtract(1, 'years')) ) { | ||
| log.debug('Caught and fixed promise with old parsed start date of:', startDate) | ||
| startDate = null; // clear startDate if more than a year ago |
Also added package-lock.json to .gitignore
…vemeister/commits.to into issue_239_stop_old_due_dates
Short change to check the date that has been parsed out of a new promise by Sherlock as the date the commitment is to be completed by. If the date is "too old" (currently coded as more than a year ago), the date is rejected, and we carry on as if no date were parsed out.