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

Don't allow due dates in the past (was: time parsing error) #239

Open
bsoule opened this issue Jul 30, 2018 · 6 comments
Open

Don't allow due dates in the past (was: time parsing error) #239

bsoule opened this issue Jul 30, 2018 · 6 comments
Labels
BUG help wanted UVI User-Visible Improvement

Comments

@bsoule
Copy link
Contributor

bsoule commented Jul 30, 2018

On http://bee.commits.to/check-habitica-dailies-thread-w-crystal-before-midnight

The due date was parsed as 1 January 1900.

(is it ok if I correct the due date? or will it be useful for me to leave it for debugging?)

@dreeves dreeves added BUG UVI User-Visible Improvement labels Jul 30, 2018
@dreeves
Copy link
Contributor

dreeves commented Jul 30, 2018

Here's a snapshot of the row of the database so @bsoule can fix her due date!

bee/check-habitica-dailies-thread-w-crystal-before-midnight |   | check-habitica-dailies-thread-w-crystal-before-midnight | check-habitica-dailies-thread-w-crystal | Check habitica dailies thread w crystal | FALSE | FALSE | 0.005259171042928253 | 2018-07-30 17:02:04.429+00 | 1900-01-01 08:00:00+00 |   | 1 | 3 |   | 2018-07-30 17:02:04.43+00 | 2018-07-30 19:20:01.962+00 | 8 | 76.105.255.250 | "{"isAuthoritative":true,"isChrome":true,"isDesktop":true,"isMac":true,"browser":"Chrome","version":"67.0.3396.79","os":"macOS Sierra","platform":"Apple Mac","geoIp":{},"source":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.79 Safari/537.36"}" | America/Los_Angeles

@chrisbutler
Copy link
Member

this looks pretty similar to #192, the fix for which was implemented in sherlock itself... hmmm...

@dreeves
Copy link
Contributor

dreeves commented Aug 3, 2018

@dreeves
Copy link
Contributor

dreeves commented Aug 22, 2018

Proposal: Add a special case to say if it parses to more than a year in the past then pretend it didn’t find a date at all. In the status quo that means treating it as if it said "in a week" which is particularly wrong in this case. But if we do #260 then it means the human will have to pick a date, which is a good choice when Sherlock is clearly hopelessly confused.

@dreeves dreeves changed the title time parsing error Don't allow due dates 100 years in the past (was: time parsing error) Aug 22, 2018
@chrisbutler
Copy link
Member

@dreeves why do we pick a year? isn't any date in the past going to be 'wrong' by this logic?

@dreeves
Copy link
Contributor

dreeves commented Sep 7, 2018

I kinda half-imagined use cases where you remember after the fact that you said you'd do something by last week and want to log that. A year is way too much though, now that you mention it. And even for my imagined use case, having to manually adjust the due date in such rare cases totally fine. So, yeah, I agree, no due dates in the past!

@dreeves dreeves changed the title Don't allow due dates 100 years in the past (was: time parsing error) Don't allow due dates in the past (was: time parsing error) Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG help wanted UVI User-Visible Improvement
Projects
None yet
Development

No branches or pull requests

3 participants