CRM: Fix dashboard contact graph

[gogdzl]

Fixes https://github.com/Automattic/zero-bs-crm/issues/3468

Proposed changes:

• This PR fixes the dashboard contact graph which shows wrong values in some instances.

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

https://github.com/Automattic/zero-bs-crm/issues/3468

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

• Create one contact in the last day of the month
• Change your timezone to a high enough value that would cause the day to overflow, when changing timezones
• Verify that the graph gets shifted
• Apply this PR's code to your CRM installation
• Verify that the error is gone

Alternatively just test the new code and check if the behavior is consistently the same as the previous, and that the new code looks good enough.

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• 🔴 Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Choose a review path based on your changes:
   • A. Team Review: add the "[Status] Needs Team Review" label
     • For most changes, including minor cross-team impacts.
     • Example: Updating a team-specific component or a small change to a shared library.
   • B. Crew Review: add the "[Status] Needs Review" label
     • For significant changes to core functionality.
     • Example: Major updates to a shared library or complex features.
   • C. Both: Start with Team, then request Crew
     • For complex changes or when you need extra confidence.
     • Example: Refactor affecting multiple systems.
3. Get at least one approval before merging.

Still unsure? Reach out in #jetpack-developers for guidance!

[coder-karen]

Nice, thanks for working on this.

What would be the best way to change the timezone? In my case I created a contact and set the creation date for June 30th UTC+0, evening. Changed my timezone in WordPress to UTC+13 which changed the contact creation date to July 1st. The dashboard graph still shows the contact entry for June, both with and without the PR.

[gogdzl]

What would be the best way to change the timezone? In my case I created a contact and set the creation date for June 30th UTC+0, evening. Changed my timezone in WordPress to UTC+13 which changed the contact creation date to July 1st. The dashboard graph still shows the contact entry for June, both with and without the PR.

That's a great question. That should've bugged the dashboard's graph. Adjust inside your database zbsc_created so MONTH(FROM_UNIXTIME(zbsc_created)) as month returns June on your machine's timezone.

[coder-karen]

Yes that's how I adjusted the zbsc_created initially for the contact, the timestamp there (in the database, for zbsc_created) is for June 30th, unless I'm misunderstanding 🤔

[cleacos]

To test this way:

We could use the WP installation timezone configured:

$datetime_month = new DateTime( '@', $v->ts );
$datetime_month->setTimezone( new DateTimeZone('the_UTC_time_zone') );
$the_month = $datetime_month->format( 'M y' );

[coder-karen]

We could use the WP installation timezone configured:

@cleacos Do you mean the existing code could be replaced by the suggested code there? Not sure I'm following on that, but I tried doing so using the 'Europe/London' DateTimeZone, and encountered an issue because of the '@' - uncaught Exception: Failed to parse time string (@) at position 0 (@):. Removing that, I'm also encountering Undefined property: stdClass::$ts, as ts isn't a property on $v.
So again I'm probably misunderstanding what the intention is here :)

[tbradsha]

👋 Indeed, this is a timezone-related bug! However, this PR wouldn't fix it.

1. Removing the ts column from the SELECT: not a problem; it was probably an arbitrary column used to debug.
2. While I haven't tested the other changes, I'm positive they don't address the core issue. By the time you're past the queries, the grouping and counts have already been pulled and no amount of shifting timezones or whatever will affect that.

There are a few ways to handle this. My recommendation is to NOT rely on gmdate or the raw DateTime object. There are several functions in includes/jpcrm-localisation.php that are timezone-friendly. In this case, you'll probably want jpcrm_get_wp_timezone_offset() (jpcrm_get_wp_timezone_offset_in_seconds() could be used too, but in this instance would result in much messier queries):

$tz_offset = jpcrm_get_wp_timezone_offset();

My preferred solution would be to set the MySQL timezone to jpcrm_get_wp_timezone_offset(). I'm not sure if that's a possibility with $wpdb. Note that in whatever solution you use, timezone codes (e.g. 'UTC') are not portable, but +5:00 should work consistently.

If there's not an easy way to adjust the timezone globally, you can try something like this:

$sql    = $wpdb->prepare( 'SET time_zone = "' . $tz_offset . '"; SELECT count(ID) as count, YEAR(FROM_UNIXTIME(zbsc_created)) as year FROM ' . $ZBSCRM_t['contacts'] . ' WHERE zbsc_created > %d AND zbsc_created < %d GROUP BY year ORDER BY year', $start_date, $end_date ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
$yearly = $wpdb->get_results( $sql );

However, I'm not sure if $wpdb->prepare() allows multiple queries in this manner. If not, this would work well enough:

$sql    = $wpdb->prepare( 'SELECT count(ID) as count, YEAR(CONVERT_TZ(FROM_UNIXTIME(zbsc_created),"SYSTEM","' . $tz_offset . '")) as year FROM ' . $ZBSCRM_t['contacts'] . ' WHERE zbsc_created > %d AND zbsc_created < %d GROUP BY year ORDER BY year', $start_date, $end_date ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
$yearly = $wpdb->get_results( $sql );

Technically FROM_UNIXTIME() is lossy, but that shouldn't matter in this case.

---

For testing, I'd recommend using a creation date of 1830297600 (1 January 2028, 0:00:00 UTC) and 1830301200 (1 January 2028, 1:00:00 UTC). This'll make it easy to switch the WP timezone from -1:00 to 0:00 to +1:00 and spot the chart change in year and month.

---

As an aside, there might be another place that is affected in the same way:

jetpack/projects/plugins/crm/includes/ZeroBSCRM.DAL3.Obj.Transactions.php

Line 602 in dee7678

$sql = $wpdb->prepare( 'SELECT SUM(' . $column_prefix . 'total) as total, MONTH(FROM_UNIXTIME(' . $column_prefix . 'date)) as month, YEAR(FROM_UNIXTIME(' . $column_prefix . 'date)) as year FROM ' . $ZBSCRM_t['transactions'] . ' WHERE ' . $column_prefix . 'date > %d AND ' . $column_prefix . 'date < %d' . $transStatusQueryAdd . ' GROUP BY month, year ORDER BY year, month', $paidAfter, $paidBefore );

[tbradsha]

Per this comment in documentation, there may be an undocumented time_zone property on $wpdb:

https://developer.wordpress.org/reference/classes/wpdb/

[gogdzl]

Per this comment in documentation, there may be an undocumented time_zone property on $wpdb:

https://developer.wordpress.org/reference/classes/wpdb/

Tried, didn't work.

[gogdzl]

If there's not an easy way to adjust the timezone globally, you can try something like this:

It works! Thanks, @tbradsha!

[coder-karen]

Thanks for the changes!

I tested again, and this time I got what I believe is the expected result (different to last time), but want to make sure that's expected:

• I set the contact creation date to late in the day UTC+0 Sep 30th. The contact shows in September in the dashboard. I then change the site timezone to UTC+13 - the contact now shows in October in the dashboard.
• Same results following the suggestion here - I changed the creation date to 1st Jan (but 2024, not 2028), 0:00:00 and 1:00:00 UTC. The dashboard continues to show the contact in Jan regardless of timezone change without the PR, and with the PR if the timezone change switches the creation date to the previous month the dashboard now reflects that.

[tbradsha]

want to make sure that's expected

Based on your description, it sounds like it's doing what it should: showing the creation date based on the currently-selected WP timezone.

[tbradsha]

Not a blocker, but the cast should be unnecessary, as wp_timezone_string() should always return a valid string, and everything else uses built-ins.

[gogdzl]

I'm escaping this because we are not calling wp_timezone_string directly.

[tbradsha]

Yep, I was referring to the cast. :^)

[tbradsha]

I still think the timestamp way is more readable than this way, though that'd require keeping that column in the database.

[tbradsha]

Seems to work.