CRM-18062: Mailing processing ignores timezone, and sends mailings at wrong time

[monishdeb]

• CRM-18062: Mailing processing ignores timezone, and sends mailings at wrong time

[kcristiano]

This patch https://github.com/andy-walker/wp-cli-civicrm/pull/4/files from PalanteJon allows the timezone to be controlled via the wp-cli command. This would allow overriding a misconfigured server. Can you look at this and see if we can put these two together?

I'll test as soon as I get a chance

[monishdeb]

Done :)

[MegaphoneJon]

I didn't test this code, but I read through it and it looks substantially similar (but improved upon) my patch, which I use in production on multiple sites.

Also NB - there are lots of use cases for the option. In particular, I wrote my patch to handle a multi-site install where an organization has affiliates in different time zones. The cron job for each CiviCRM domain specifies the appropriate time zone. Thanks Kevin and Monish for getting this all in!

[eileenmcnaughton]

Some cross over with this civicrm/civicrm-core#6976 (although I'm not liking the approach in the other one)

[monishdeb]

As per the current fix I have used the timezone code already used in invoke function with an improvement to fetch timezone parameter as per https://github.com/andy-walker/wp-cli-civicrm/pull/4/files by PalentenJon . Not sure about #6976 as I was wondering on what scenario user don't want to synchronise the timezone with CMS.

[monishdeb]

@kcristiano @eileenmcnaughton @PalanteJon can you please verify my patch so that we can include the changes before next release ?

[kcristiano]

@monishdeb Patch is applied to our production site (ours not a client) and we'll test there and report back

[kcristiano]

@monishdeb we have tested this.

First set of tests using wp-cli cron running as:
/usr/local/bin/wp --user=cradmin --url=https://example.cc --path=/home/user/public_html civicrm api job.execute auth=0

The results are what I expected, but not what a user would suspect. WP has a quirk (I'd call it a bug but it's a 'wontfix' in WP Core) where if you set your Time Zone in WP to a UTC offset for Cron WP will use UTC.

When I set the TZ to 'Europe/Zurich' or 'America/New_York' the code works fine. But when I set the WP Timezone to 'UTC -3' the email went out at UTC not 'UTC-3'.

This option in WordPress is controlled in Settings--> General and stored in the options table as 'timezone_string'.

Next Set of tests is with the wp-cli cron running as:
/usr/local/bin/wp --user=cradmin --url=https://example.cc --path=/home/user/public_html --timezone=America/New_York civicrm api job.execute auth=0

With the wp-cli flag even though CiviCRM says that there is a TZ mismtach, cron runs at the time that wp-cli tells it - in this example America/New_York

I think we should merge this and document the change. Users and Admins will need to set their WP Time Zones to America/New_York not UTC-5 or they need to use the wp-cli fllag --timezone=America/New_York

[kcristiano]

Can we also tag this for the 4.6 branch?

[monishdeb]

@kcristiano as you can see in the patch we are using date_default_timezone_set() to set php timezone, accept only valid timezone identifier listed in http://php.net/manual/en/timezones.php. UTC-3 is not valid timezone identifier, as it return FALSE for default_timezone_set(UTC-3)
Ref. http://www.w3schools.com/php/func_date_default_timezone_set.asp

Need to test the issue on 4.6 branch as I believe its a regression related to refactoring in wordpress plugin against master.

[kcristiano]

@monishdeb I agree, but WordPress allows UTC-3 (and other offsets for backwards compatibility. Therefore, if any WP site uses an offset it will return False, reset to UTC and emails go out at the wrong time.

I did a good deal of the wp-cli work, I will try and look at the 4.6 branch.

[monishdeb]

I see .. well then we need to check the logic how WP sets UTC-n formatted timezone, will be nice to incorporate the same for setting civicrm timezone.

@kcristiano thanks a lot for your test report :) and please comment on https://issues.civicrm.org/jira/browse/CRM-18062 if you encounter the same issue for 4.6, will submit a backport PR

[kcristiano]

I should Clarify, if we set TZ to UTC -3 (or any invalid option) BUT use the wp-cli --timezone flag and set that correctly it works.

I do believe we need to backport to 4.6 as well. I'll get on that as soon as I can

[eileenmcnaughton]

Wouldn't that cause the timezone offset to be applied twice?

https://github.com/civicrm/civicrm-core/blob/master/CRM/Utils/System/WordPress.php#L693-693

  /**
   * Set timezone in mysql so that timestamp fields show the correct time.
   */
  public function setMySQLTimeZone() {
    $timeZoneOffset = $this->getTimeZoneOffset();
    if ($timeZoneOffset) {
      $sql = "SET time_zone = '$timeZoneOffset'";
      CRM_Core_DAO::executequery($sql);
    }
  }


  /**
   * Get timezone from CMS.
   *
   * @return string|false|null
   */
  public function getTimeZoneOffset() {
    $timezone = $this->getTimeZoneString();

[kcristiano]

@eileenmcnaughton The override seems to take care of that. I've found funny behavior if the WP timezones are set badly (meaning UTC offsets) but this has worked for me and @PalanteJon

I do understand the concern. However, right now any civiCRM with WP that does not have all 3 timezones (php, MySQL and WP) set properly will have emails sent out at seemingly wrong times. I've been using Jon's patch for a while and this improves on it

[eileenmcnaughton]

but, why doesn't calling CRM_Core_Config::singleton()->userSystem->setMySQLTimeZone(); straight after civicrm_initialize acheive the same thing?

[kcristiano]

Because WP's built in setting will override that. I cannot trace it (I've tried) but once the cron job fires the timezone is set by WP and if it's wrong - the scheduled jobs go out at the wrong time.

I'd love WP to fix their TimeZone settings, but due to backwards compatibility it's a wontfix

I can get around it by setting it properly in WP, but Jon has use cases where a WP Multi-Site has CiviCRM domains in different timezones so we need something to force it.

[eileenmcnaughton]

OK so I guess what's confusing me is this

date_default_timezone_set($wpUserTimezone);

doesn't need to be in the IF (but it probably doesn't matter that it is) - but the bit where it sets the php timezone does I guess.

[kcristiano]

Let me test and confirm. I think so.

[monishdeb]

I guess for the same reason invoke() already implement the same logic to forcibly set the timezone just after intialize() call https://github.com/civicrm/civicrm-wordpress/blob/master/civicrm.php#L1190

In addition current changes is more safer as it also restore the WP timezone after api call
if ($wpBaseTimezone) { date_default_timezone_set($wpBaseTimezone); }

[eileenmcnaughton]

hmm - we should probably put those lines into a function if we are doing the same thing from 2 places

[kcristiano]

I did a quick test and we need all the logic that exist. Keep in mind this is for wp-cli only, and wp-cli boots a bit differently as it uses a special wp-settings.php tailored for the command line.

[eileenmcnaughton]

ok - sounds pretty conclusive!

[monishdeb]

Ya thought of that earlier but then there is no parent class for civicrm.php and wp-cli/civicrm.php so there is little difference as later provide --timezone option and restore timezone after api call. And former only sets WP TZ

[monishdeb]

mentioned the changes in https://wiki.civicrm.org/confluence/display/CRMDOC/Managing+Scheduled+Jobs under wp-cli section