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

Enhancement/7602-parsing-custom-dimensions-report #7697

Merged
merged 23 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d59ab88
Add custom dimension string constants.
jimmymadon Oct 13, 2023
dc956b1
Add helpers to convert user and category IDs to names.
jimmymadon Oct 13, 2023
50ba870
Add unit tests for user and category helpers.
jimmymadon Oct 13, 2023
aeca2f4
Convert custom dimension IDs to names in report response.
jimmymadon Oct 13, 2023
c72e6f7
Merge branch 'develop' into enhancement/7602-parsing-custom-dimension…
jimmymadon Oct 13, 2023
e5b753f
Fix potentially existent category in tests.
jimmymadon Oct 13, 2023
9abf2eb
Generalize entity helpers.
jimmymadon Oct 15, 2023
f2477e0
Fix potential null value in term name.
jimmymadon Oct 16, 2023
07e0e7f
Extract custom dimensions swapping method into its own class.
jimmymadon Oct 16, 2023
b2966ac
Remove outdated test.
jimmymadon Oct 16, 2023
259f120
Update references in Response to use new class.
jimmymadon Oct 16, 2023
df7e617
Add custom dimensions ID to name converter tests.
jimmymadon Oct 16, 2023
794f4f2
Refactor WP_Entities_Helpers into new parser class.
jimmymadon Oct 16, 2023
2f0d0e5
Remove redundant helper class and methods.
jimmymadon Oct 16, 2023
5fe120c
Add support for mixed number and string category ID values.
jimmymadon Oct 16, 2023
3a1b1d9
Update return type and dependency of the swap_custom_dimensions method.
jimmymadon Oct 16, 2023
5bf0282
Update unit tests for dependency changes and remove duplicate object …
jimmymadon Oct 16, 2023
f34d9ca
Clarify documentation around the customEvent prefix.
jimmymadon Oct 16, 2023
12d864c
Merge branch 'develop' into enhancement/7602-parsing-custom-dimension…
jimmymadon Oct 16, 2023
3380e3f
Update doc block.
aaemnnosttv Oct 16, 2023
93dadc5
Fix typo.
aaemnnosttv Oct 16, 2023
5511c1e
Make internal methods protected.
aaemnnosttv Oct 16, 2023
c7bb05b
Remove unnecessary variable.
aaemnnosttv Oct 16, 2023
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
75 changes: 75 additions & 0 deletions includes/Core/Util/WP_Entity_Helpers.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php
/**
* Class Google\Site_Kit\Core\Util\WP_Entity_Helpers
*
* @package Google\Site_Kit\Core\Util
* @copyright 2023 Google LLC
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0
* @link https://sitekit.withgoogle.com
*/

namespace Google\Site_Kit\Core\Util;

/**
* Utility class for fetching meta data for posts.
*
* @since n.e.x.t
* @access private
* @ignore
*/
class WP_Entity_Helpers {

/**
* Gets the display name for a given user ID.
*
* @since n.e.x.t
*
* @param int $user_id User ID of the user to get the display name of.
* @return string Display name of the user for the given user ID.
*/
public static function get_user_display_name( $user_id ) {
$user = get_userdata( $user_id );
return $user->display_name;
aaemnnosttv marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Gets the category names for a given array of category IDs.
*
* If no category is found for a given ID, the original ID is preserved in
* the returned array.
*
* @since n.e.x.t
*
* @param array $category_ids Array of IDs of categories to get names of.
* @return array Array of category names (or their original IDs if no name is found).
*/
public static function get_category_names( $category_ids ) {
aaemnnosttv marked this conversation as resolved.
Show resolved Hide resolved
$category_names = array();
foreach ( $category_ids as $category_id ) {
$category_name = get_cat_name( $category_id );
$category_names[] = empty( $category_name ) ? $category_id : $category_name;
}
return $category_names;
}

/**
* Converts a string list of category IDs to a stringified array of their
* category names.
*
* If no category is found for a given ID, the original ID is preserved in
* the returned string.
*
* @since n.e.x.t
*
* @param string $category_ids_string Comma separated string list of IDs of categories to get names of.
* @return string JSON encoded string of comma separated category names (or their original IDs if no name is found).
*/
public static function parse_category_names( $category_ids_string ) {
aaemnnosttv marked this conversation as resolved.
Show resolved Hide resolved
$category_ids = json_decode( '[' . $category_ids_string . ']', true );

$category_names = self::get_category_names( $category_ids );

return wp_json_encode( $category_names );
}

}
7 changes: 7 additions & 0 deletions includes/Modules/Analytics_4.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ final class Analytics_4 extends Module
*/
const MODULE_SLUG = 'analytics-4';

/**
* Custom dimensions tracked by Site Kit
*/
const CUSTOM_EVENT_PREFIX = 'customEvent:';
aaemnnosttv marked this conversation as resolved.
Show resolved Hide resolved
const CUSTOM_DIMENSION_POST_AUTHOR = 'googlesitekit_post_author';
const CUSTOM_DIMENSION_POST_CATEGORIES = 'googlesitekit_post_categories';

/**
* Registers functionality through WordPress hooks.
*
Expand Down
48 changes: 48 additions & 0 deletions includes/Modules/Analytics_4/Report/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace Google\Site_Kit\Modules\Analytics_4\Report;

use Google\Site_Kit\Core\REST_API\Data_Request;
use Google\Site_Kit\Modules\Analytics_4;
use Google\Site_Kit\Modules\Analytics_4\Report;
use Google\Site_Kit_Dependencies\Google\Service\AnalyticsData\DateRange as Google_Service_AnalyticsData_DateRange;
use Google\Site_Kit_Dependencies\Google\Service\AnalyticsData\Row as Google_Service_AnalyticsData_Row;
Expand Down Expand Up @@ -42,6 +43,8 @@ public function parse_response( Data_Request $data, $response ) {
return $response;
}

$response = $this->parse_custom_dimensions( $response );
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd suggest a different name for this method since parsing to me is more about extracting one or more values from the input (which is happening here) but more importantly, this is really transforming the response by mapping the IDs to display names. So the response is changing here, but the name doesn't quite reflect that.


// Get report dimensions and return early if there is either more than one dimension or
// the only dimension is not "date".
$dimensions = $this->parse_dimensions( $data );
Expand Down Expand Up @@ -117,6 +120,51 @@ function( $date ) use ( &$rows, $existing_rows, $ranges_count, $metric_headers,
return $response;
}

/**
* Converts the IDs of any custom dimensions within the response into their respective display names.
*
* @since n.e.x.t
*
* @param Google_Service_AnalyticsData_RunReportResponse $response Request response that may contain custom dimensions.
* @return Google_Service_AnalyticsData_RunReportResponse Response with any IDs of custom dimensions converted to their display names.
*/
protected function parse_custom_dimensions( $response ) {
if ( empty( $response['rows'] ) ) {
return $response;
}

// Create a map of any custom dimension to its equivalent parsing function to avoid
// looping through report rows multiple times below.
$custom_dimension_map = array();
foreach ( $response['dimensionHeaders'] as $dimension_key => $dimension ) {
if ( Analytics_4::CUSTOM_EVENT_PREFIX . Analytics_4::CUSTOM_DIMENSION_POST_AUTHOR === $dimension['name'] ) {
$custom_dimension_map[ $dimension_key ] = array( 'Google\\Site_Kit\\Core\\Util\\WP_Entity_Helpers', 'get_user_display_name' );
aaemnnosttv marked this conversation as resolved.
Show resolved Hide resolved
}

if ( Analytics_4::CUSTOM_EVENT_PREFIX . Analytics_4::CUSTOM_DIMENSION_POST_CATEGORIES === $dimension['name'] ) {
$custom_dimension_map[ $dimension_key ] = array( 'Google\\Site_Kit\\Core\\Util\\WP_Entity_Helpers', 'parse_category_names' );
}
}

if ( empty( $custom_dimension_map ) ) {
return $response;
}

foreach ( $response['rows'] as &$row ) {
foreach ( $custom_dimension_map as $dimension_key => $callable ) {
$dimension_value = $row['dimensionValues'][ $dimension_key ]->getValue();
if ( '(not set)' === $dimension_value ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be safer to check ! is_numeric here instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@aaemnnosttv I actually had that check and then wanted to be more "strict" in preventing any string to filter through. But actually, it makes sense to filter through strings which we can't do anything about.

continue;
}

$new_dimension_value = call_user_func( $callable, $dimension_value );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going to be rather inefficient to call these functions every time for every row that will likely have many duplicates throughout.

Let's update the process to go through the rows and build a list of all of the IDs for each entity without duplicates (i.e. a Set). Then we can map those to their display names, after which we can go through and do the actual replacements in the dimension values all at once rather than doing everything on every iteration.

$row['dimensionValues'][ $dimension_key ]->setValue( $new_dimension_value );
}
}

return $response;
}

/**
* Gets the response row key composed from the date and the date range index values.
*
Expand Down
45 changes: 45 additions & 0 deletions tests/phpunit/integration/Core/Util/WP_Entity_HelpersTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php
/**
* WP_Entity_HelpersTest
*
* @package Google\Site_Kit\Tests\Core\Util
* @copyright 2023 Google LLC
* @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0
* @link https://sitekit.withgoogle.com
*/

namespace Google\Site_Kit\Tests\Core\Util;

use Google\Site_Kit\Core\Util\WP_Entity_Helpers;
use Google\Site_Kit\Tests\TestCase;

/**
* @group Util
*/
class WP_Entity_HelpersTest extends TestCase {

public function test_get_user_display_name() {
$test_user_id = $this->factory()->user->create( array( 'display_name' => 'test display name' ) );
$this->assertEquals( 'test display name', WP_Entity_Helpers::get_user_display_name( $test_user_id ) );
}

public function test_parse_category_names__mixed_values() {
$category_with_number = $this->factory()->category->create( array( 'name' => '2' ) );
$category_with_commas = $this->factory()->category->create( array( 'name' => 'Category,with,commas' ) );
$normal_category = $this->factory()->category->create( array( 'name' => 'Normal Category' ) );
$category_ids_string = implode( ',', array( $category_with_number, $category_with_commas, 1955, $normal_category ) ); // 1955 would be a non-existent category

$this->assertEquals(
'["2","Category,with,commas",1955,"Normal Category"]',
WP_Entity_Helpers::parse_category_names( $category_ids_string )
);
}

public function test_parse_category_names__single_value() {
$this->assertEquals(
'["Uncategorized"]',
WP_Entity_Helpers::parse_category_names( '1' )
);
}

}
Loading