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.

Sign up for GitHub

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

CRM: Fix dashboard contact graph #38316

Merged
merged 5 commits into from
Nov 1, 2024
Merged

CRM: Fix dashboard contact graph #38316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0649121
Stop relying on MySQLs timestamp conversion for months, weeks and days
gogdzl Jul 12, 2024
7535b25
changelog
gogdzl Jul 12, 2024
e92a96f
Merge branch 'trunk' into fix/crm/3468-dashboard-contact-graph-fix
gogdzl Oct 25, 2024
7d0721a
Add timezone offset to MySQL
gogdzl Oct 25, 2024
f7f4035
Escape SQL
gogdzl Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions projects/plugins/crm/admin/dashboard/dashboard.ajax.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,20 @@ function jetpackcrm_dash_refresh() {
);
}

$tz_offset = esc_sql( (string) jpcrm_get_wp_timezone_offset() );
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

$wpdb->query( 'SET time_zone = "' . $tz_offset . '";' );

// next we want the contact chart which is total contacts between the dates grouped by day, week, month, year
$sql = $wpdb->prepare( '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 );

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

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

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

$zeros = jetpackcrm_create_zeros_array( $start_date, $end_date );
Expand All @@ -89,19 +92,20 @@ function jetpackcrm_dash_refresh() {
$zeros['year'][ $v->year ] = $v->count;
}

// convert the monthly array into a zero padded one
foreach ( $monthly as $v ) {
$the_month = gmdate( 'M y', $v->ts );
$datetime_month = DateTime::createFromFormat( '!m Y', $v->month . ' ' . $v->year );
$the_month = $datetime_month->format( 'M y' );
Comment on lines +96 to +97
Copy link
Contributor

Choose a reason for hiding this comment

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

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

$zeros['month'][ $the_month ] = $v->count;
}

foreach ( $weekly as $v ) {
$the_week = gmdate( 'W Y', $v->ts );
$the_week = str_pad( $v->week, 2, '0', STR_PAD_LEFT ) . ' ' . $v->year;
$zeros['week'][ $the_week ] = $v->count;
}

foreach ( $daily as $v ) {
$the_day = gmdate( 'd M y', $v->ts );
$datetime_day = DateTime::createFromFormat( 'Y-m-d', $v->year . '-' . $v->month . '-' . $v->day );
$the_day = $datetime_day->format( 'd M y' );
$zeros['day'][ $the_day ] = $v->count;
}

Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/crm/changelog/fix-crm-3468-dashboard-contact-graph-fix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Dashboard: Fixed an issue where the CRM contacts graph displayed incorrect values in some instances