[PROJ-1523] fix(place/auto_release): fix logic so email sends with negative time_… #586
Conversation
…before value The conditions were previously mutually exclusive with a negative time_before value, so the email would never send. We need to allow for negative values so the email can send X minutes after the meeting time but before the booking is released.
|
hey @chillfox can we get your review as you're the main author of this driver? |
|
When the time_before is set to a negative value, the conditions for sending the email skip the exact moment at which the email would be sent. By changing the sign to <=, that moment is included and the email should send. 1st condition:
|
chillfox
left a comment
chillfox
left a comment
There was a problem hiding this comment.
mostly looks good.
Just remove the end, run crystal tool format drivers/place/auto_release.cr to ensure consistent formatting. Make sure it compiles crystal build drivers/place/auto_release.cr and run the specs in the test harness to see if any needs to be changed with the change in logic.
drivers/place/auto_release.cr
Outdated
| (booking.booking_start - Time.utc.to_unix <= @auto_release.time_before(booking.booking_type) * 60) && | ||
| (Time.utc.to_unix - booking.booking_start <= @auto_release.time_after(booking.booking_type) * 60) | ||
| logger.debug { "sending release email to #{booking.user_email} for booking #{booking.id} as it is within the time_before window" } | ||
| end |
There was a problem hiding this comment.
This end should not be there.
w-le
changed the title
|
OK, @chillfox I have removed the "end" and added a couple of test results to the spec file that it was expecting to see. It now passes the test harness. Please verify and merge if those changes are ok. |
chillfox
left a comment
chillfox
left a comment
There was a problem hiding this comment.
LGTM
Please run crystal tool format in the future, as it makes the diffs much easier to read for review.
KesterJJ
deleted the
fix(place/auto_release)-fix-logic-so-email-sends-with-negative-time_before-value
branch
The conditions were previously mutually exclusive with a negative time_before value, so the email would never send. We need to allow for negative values so the email can send X minutes after the meeting time but before the booking is released.