dev/core#434: Check If 'absolute_date' Is Used In 'prepareRepetitionEndFilter' To Prevent SQL Error

[vinuvarshith]

Overview

When adding a scheduled reminder with an 'absolute_date' initially and then enabling 'Repeat'and adding conditions results in an SQL error when executed.

Before

SQL systax error because of empty datefield
Produces SQL with condition something like the following (notice empty value in DATE_ADD)
...AND ("20181219180831" <= DATE_ADD(, INTERVAL 9 day))

After

SQL is fixed by checking if datefield is empty and replacing it with absolute_date

Technical Details

A function in Civi\ActionSchedule\RecipientBuilder.php, 'prepareRepetitionEndFilter' expects a date field which would be empty when using an 'absolute_date'. This causes the SQL query to be constructed with an empty string inside thus creating a syntax error.
Edit:
Its more appropriate to handle this in CRM/Event/ActionMapping.php createQuery() method and make sure 'casDateField' param is not empty. PR updated.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@vinuvarshith We'd really need to get a unit test with this one to get it merged since it touches some pretty critical & pretty deep stuff - have you written tests for CiviCRM before?

[seamuslee001]

ping @jtwyman i think this might be the issue we are hitting with our reminders

[vinuvarshith]

On looking into this a bit more, the issue is that 'casDateField' param is empty when start_date is not given (but absolute_date is given) on CRM/Event/ActionMapping.php under createQuery() method.
I have update the PR to handle this.
@eileenmcnaughton Can you pls take another look?
Do you still want me to add tests? (as we are not touching critical areas)

[seamuslee001]

Given that this creates a hard fail and that we have had some user testing of this PR according to another ticket https://lab.civicrm.org/dev/core/issues/637#note_12388. This code looks pretty safe now i'm going to add merge on pass to this

[seamuslee001]

Jenkins re test this please

[seamuslee001]

I'm going to merge this as it fixes a fatal and i think the code is pretty safe. Also Eileen was ok with merging if i was