-
Notifications
You must be signed in to change notification settings - Fork 2
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
Correctly unsnooze snoozed trips #274
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good and no issues with time zones in the unit test.
Did it unsnooze a trip if you let it run? On mine it didn't, I'll investigate. |
Never mind, the previous try might have been a flake. |
As mentioned in the team meeting, it is the wrong check (if you snooze a monitored trip on a day the trip does not happen, it will not unsnooze), so I'll take it back. |
@br648 and @JymDyerIBI Ready for review again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one nit, but all good.
} | ||
|
||
private static Stream<Arguments> createCanUnsnoozeTripCases() { | ||
// (Trips from above response above starts on Tuesday, June 9, 2020 at 8:40am and ends at 8:58am.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. "Trips from above response start on Tuesday".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Checklist
dev
before they can be merged tomaster
)Description
This PR fixes an issue where monitoring on snoozed trip did not resume automatically the next calendar day.