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

Handle all from and to relative dates in API4 #24663

Conversation

larssandergreen
Copy link
Contributor

Overview

API4 does not work correctly with relative dates that are open ended (e.g. From start of current calendar year). The open ended date is set to NULL here and then that is somewhere converted to a 0 timestamp, so one side of the comparison is Jan 1, 1970. That works half the time and doesn't work the other half of the time (when the To date is 1970).

Before

The unspecified date was set to 0.

After

Open ended relative date API calls with =, !=, <>, LIKE or NOT LIKE are converted to >= or <= as appropriate.
Nothing is changed for BETWEEN or NOT BETWEEN calls.

Technical Details

It is possible (for example in Search Kit) to specify a BETWEEN with relative date ranges as the From and To dates. That works fine in general, but won't work with half of the open ended relative dates. Such a query is pretty nonsensical and I don't think we need to worry about what it returns.

It would have been nice if BETWEEN date and NULL was equivalent to >= date in SQL, but it isn't.

@civibot
Copy link

civibot bot commented Oct 2, 2022

(Standard links)

@civibot civibot bot added the master label Oct 2, 2022
@colemanw
Copy link
Member

colemanw commented Oct 2, 2022

@larssandergreen this makes a lot of sense but really needs a unit test. You could just add a few more cases to \api\v4\Action\DateTest.

@larssandergreen
Copy link
Contributor Author

@colemanw Tests added.

@eileenmcnaughton eileenmcnaughton merged commit dfb7448 into civicrm:master Oct 2, 2022
@larssandergreen larssandergreen deleted the handle-between-with-missing-values-in-api4 branch November 5, 2022 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants