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

CiviCRM Scheduled Reminders, Effective Start Date and Effective End Date are incorrectly evaluated if these fields contain a value '0000-00-00 00:00:00' #21250

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

agileware-justin
Copy link
Contributor

@agileware-justin agileware-justin commented Aug 25, 2021

CiviCRM Scheduled Reminders, Effective Start Date and Effective End Date are incorrectly evaluated if these fields contain a value '0000-00-00 00:00:00'. See Gitlab, https://lab.civicrm.org/dev/core/-/issues/2787

Overview

See Gitlab, https://lab.civicrm.org/dev/core/-/issues/2787

Before

Scheduled Reminder is NOT sent when either the Effective Start Date or Effective End Date have a value of '0000-00-00 00:00:00'.

After

Scheduled Reminder IS sent when either the Effective Start Date or Effective End Date have a value of '0000-00-00 00:00:00'.

Technical Details

Comments

This problem relates to this PR #19973

Agileware Ref: CIVICRM-1820

@civibot
Copy link

civibot bot commented Aug 25, 2021

(Standard links)

@civibot civibot bot added the master label Aug 25, 2021
@eileenmcnaughton
Copy link
Contributor

@agileware-justin should we be targetting the rc / porting this to 5.40? I can see the expecation was that it would be NULL unless deliberately set but perhaps that is not the case on all mysql/mariadb versions

@agileware-justin
Copy link
Contributor Author

agileware-justin commented Aug 25, 2021

@eileenmcnaughton I think the impact of this issue is fairly serious. Scheduled Reminders will fail to execute, which is a silent fail. And CiviCRM administrators are only made aware of the issue when business processes start failing.

It's not difficult to identify if a CiviCRM site is affected by querying the civicrm_action_schedule table and effective_start_date, effective_end_date columns for values '0000-00-00 00:00:00'. If there's rows, then those Scheduled Reminders will simply not work.

I did try to reproduce creating this invalid data thru the UI, but couldn't repeat which was very annoying. My next guess was that it may have been created during the CiviCRM database upgrade - I didn't test that explicitly.

TL:DR Silent failures, very problematic.

@eileenmcnaughton
Copy link
Contributor

@agileware-justin ok - are you able to switch this to being against the rc - 5.41 & we can merge & then port to 5.40

@agileware-justin agileware-justin changed the base branch from master to 5.41 August 25, 2021 23:45
@civibot civibot bot added 5.41 and removed master labels Aug 25, 2021
…fective End Date are incorrectly evaluated if these fields contain a value '0000-00-00 00:00:00'
$startDateClauses[] = "'{$actionSchedule->effective_start_date}' <= {$date}";
}
if (!empty($actionSchedule->effective_end_date)) {
if (!empty($actionSchedule->effective_end_date) && $actionSchedule->effective_end_date !== '0000-00-00 00:00:00') {
$startDateClauses[] = "'{$actionSchedule->effective_end_date}' > {$date}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, I've tried various paths to reproduce the situation where the value is 0000-00-00 00:00:00 and had no success. e.g.

  • Upgrade 5.39.latest => 5.40.latest on MySQL 5.7.23
  • Upgrade 5.33.latest => 5.40.latest on MySQL 5.7.23
  • Upgrade 5.33.latest => 5.40.0 and then use the admin UI, alternately setting/saving/clearing the value. (There had been some bug in saving this field circa5.40.0-5.40.3; so an early 5.40.x might've had the problem)

Without that, it's hard to affirmatively describe the proper resolution. But, as a sort of work-around, this seems safe enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@totten thanks for that - I think the issue depends on mysql config - (NO_ZERO_IN_DATE,NO_ZERO_DATE or something like that)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who knows how, but have seen two sites with this issue so far and that's enough to warrant a fix IMHO. Thanks for checking.

Dunno

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.

4 participants