Allow comparing date/datetimes to ISO8601 date/datetime strings #188
Conversation
|
This pull request has been linked to Shortcut Story #30024: Add datetime comparison operators. |
| @@ -177,7 +177,7 @@ defmodule Expression.Eval do | |||
| def op(:==, a, b) when is_struct(a, DateTime) and is_struct(b, DateTime), | |||
| do: DateTime.compare(a, b) == :eq | |||
|
|
|||
| def op(:=, a, b) when is_struct(a, Date) and is_struct(b, Date), | |||
| def op(:=, a, b) when is_struct(a, DateTime) and is_struct(b, DateTime), | |||
There was a problem hiding this comment.
I think this was a typo, as this function clause was appearing twice.
I think this one was supposed to be for DateTime, not for Date: the one for Date already exists down below.
There was a problem hiding this comment.
@smn could you confirm?
|
@smn @santiagocardo any ideas on how to fix the failing dialyzer check in CI? It passes locally. It's saying something about an invalid PLT? Maybe an invalid cache? |
| end | ||
|
|
||
| case {operator, comparison_result} do | ||
| {:=, :eq} -> true |
There was a problem hiding this comment.
are we intentionally supporting the match operator as an equal operator?
There was a problem hiding this comment.
yes, this was already the case in the existing code. The FLOIP standard supports both = and == (or at least RapidPro does).
lib/expression/eval.ex
Outdated
| def op(operator, a, b) | ||
| when operator in [:=, :==, :!=, :<, :<=, :>, :>=] and | ||
| (is_struct(a, Date) or is_struct(a, DateTime)) and | ||
| is_binary(b) do |
There was a problem hiding this comment.
@fedme are we handling a scenario where a is a binary and b is a struct? Do we need to?
There was a problem hiding this comment.
In this PR I am only handling the case where the 1st argument is a Date/DateTime and the 2nd argument is a String. That's all the Canvas needs at the moment but I am happy to expand it to support all combinations
There was a problem hiding this comment.
Happy with this change, just curious about when a is a binary and b is a struct and if we should handle that. I can see how that could be expected from an expression authoring perspective if we're already supporting the scenario we're testing against in this PR.
If we don't, we should document this behaviour well.
…github.com:turnhub/expression into feature/sc-30024/add-datetime-comparison-operators
This PR adds overloads for the
:=, :==, :!=, :<, :<=, :>, :>=boolean operators to allows users to compareDateorDateTimevalues with ISO8601 string representations of dates and datetimes.Adding support for this kind of comparisons makes it much easier to create Triggers/Conditionals in Turn that compare a Contact's datetime field to a specific datetime value (expressed as an ISO8601 string).