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/6623 ga4 widgets real data #6790

Merged
merged 31 commits into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
f18e950
Add the new parse_report_response method.
eugene-manuilov Mar 28, 2023
e349669
Add the parse_reporting_dimensions method.
eugene-manuilov Mar 28, 2023
0bed654
Add the parse_reporting_dateranges method.
eugene-manuilov Mar 28, 2023
797578c
Implement the parse_report_response method.
eugene-manuilov Mar 28, 2023
4c06396
Update rows count.
eugene-manuilov Mar 28, 2023
6ca7774
Merge remote-tracking branch 'origin/develop' into enhancement/6623-g…
eugene-manuilov Mar 29, 2023
680a723
Update the parse_report_response method to set default values to all …
eugene-manuilov Mar 29, 2023
2a2d97e
Fix formatting issues.
eugene-manuilov Mar 29, 2023
97e835a
Add tests for the new method.
eugene-manuilov Mar 29, 2023
e130cc9
Apply suggestions from code review
eugene-manuilov Mar 30, 2023
eb5e884
Fix typos.
eugene-manuilov Mar 30, 2023
4aa8fec
Addressed code review feedback.
eugene-manuilov Mar 30, 2023
182b2fe
Moved the report related functionality to its own class.
eugene-manuilov Mar 30, 2023
3b5a4c8
Fix formatting issues.
eugene-manuilov Mar 30, 2023
d71278a
Update the test_parse_response method to use expected dates array.
eugene-manuilov Mar 30, 2023
fd91b5a
Fixed ordering issues.
eugene-manuilov Mar 31, 2023
b458c4f
Merge remote-tracking branch 'origin/develop' into enhancement/6623-g…
eugene-manuilov Mar 31, 2023
1dc3c19
Refactor the Report class into Response and Request ones.
eugene-manuilov Mar 31, 2023
84cb586
Add the Row_Trait trait.
eugene-manuilov Mar 31, 2023
73384fa
Update the default value for all metrics to be '0'.
eugene-manuilov Mar 31, 2023
43f6d6e
Fix formatting issues.
eugene-manuilov Mar 31, 2023
ebe895f
Merge remote-tracking branch 'origin/develop' into enhancement/6623-g…
eugene-manuilov Apr 3, 2023
78387da
Fix date ranges issue and add more tests.
eugene-manuilov Apr 3, 2023
fb98422
Address code review feedback.
eugene-manuilov Apr 3, 2023
53f76e1
Fix typo.
eugene-manuilov Apr 3, 2023
1557f86
Add $existing_rows to use clause.
techanvil Apr 3, 2023
c2ec3cf
Fix typo in comment.
techanvil Apr 3, 2023
9ae6e2b
Update tests to check initial data as well.
eugene-manuilov Apr 4, 2023
da96ecf
Fix formatting issues.
eugene-manuilov Apr 4, 2023
054afa9
Merge remote-tracking branch 'origin/develop' into enhancement/6623-g…
eugene-manuilov Apr 4, 2023
ac2339a
Simplify test and make expectations more verbose.
techanvil Apr 4, 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
157 changes: 125 additions & 32 deletions includes/Modules/Analytics_4/Report/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

use Google\Site_Kit\Core\REST_API\Data_Request;
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;
use Google\Site_Kit_Dependencies\Google\Service\AnalyticsData\RunReportResponse as Google_Service_AnalyticsData_RunReportResponse;

/**
Expand Down Expand Up @@ -48,7 +50,7 @@ public function parse_response( Data_Request $data, $response ) {
}

// Get date ranges and return early if there are no date ranges for this report.
$date_ranges = $this->parse_dateranges( $data );
$date_ranges = $this->get_sorted_dateranges( $data );
if ( empty( $date_ranges ) ) {
return $response;
}
Expand All @@ -60,6 +62,12 @@ public function parse_response( Data_Request $data, $response ) {

$range = 'date_range_0';
if ( count( $dimension_values ) > 1 ) {
// Considering this code will only be run when we are requesting a single dimension, `date`,
// the implication is that the row will _only_ have an additional dimension when multiple
// date ranges are requested.
//
// In this scenario, the dimension at index 1 will have a value of `date_range_{i}`, where
// `i` is the zero-based index of the date range.
$range = $dimension_values[1]->getValue();
techanvil marked this conversation as resolved.
Show resolved Hide resolved
}

Expand All @@ -75,38 +83,19 @@ public function parse_response( Data_Request $data, $response ) {
$multiple_ranges = $ranges_count > 1;
$rows = array();

foreach ( $date_ranges as $date_range ) {
$start = strtotime( $date_range->getStartDate() );
$end = strtotime( $date_range->getEndDate() );

// Skip this date range if either start date or end date is corrupted.
if ( ! $start || ! $end ) {
continue;
}

// Loop through all days in the date range and check if there is a metric value
// for it. If the metric value is missing, we will need to add one with a zero value.
$now = $start;
do {
// Format the current time to a date string and add a day in seconds to the current date
// to shift to the next date.
$current_date = gmdate( 'Ymd', $now );
$now += DAY_IN_SECONDS;

// Add rows for the current date for each date range.
// Add rows for the current date for each date range.
self::iterate_date_ranges(
$date_ranges,
function( $date ) use ( &$rows, $ranges_count, $metric_headers, $multiple_ranges ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This use clause is missing $existing_rows, which means the existing row is never found, so we always return zero data for all rows. It's a small fix, I'll apply the change myself.

Suggested change
function( $date ) use ( &$rows, $ranges_count, $metric_headers, $multiple_ranges ) {
function( $date ) use ( &$rows, $existing_rows, $ranges_count, $metric_headers, $multiple_ranges ) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @eugene-manuilov - although I'd applied this fix myself, hoping to prevent an extra execution/review cycle, I've realised that the test coverage is lacking a check to ensure that the resulting rows which do have data retain this data and aren't overwritten with zeroes. Therefore, please can you update the tests to provide coverage for this aspect?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, tests are updated.

for ( $i = 0; $i < $ranges_count; $i++ ) {
// Copy the existing row if it is available, otherwise create a new zero-value row.
$current_date_key = self::get_response_row_key( $current_date, $i );
$rows[ $current_date_key ] = isset( $existing_rows[ $current_date_key ] )
? $existing_rows[ $current_date_key ]
: $this->create_report_row(
$metric_headers,
$current_date,
$multiple_ranges ? $i : false
);
$key = self::get_response_row_key( $date, $i );
$rows[ $key ] = isset( $existing_rows[ $key ] )
? $existing_rows[ $key ]
: $this->create_report_row( $metric_headers, $date, $multiple_ranges ? $i : false );
}
} while ( $now <= $end );
}
}
);

// If we have the same number of rows as in the response at the moment, then
// we can return the response without setting the new rows back into the response.
Expand All @@ -115,6 +104,12 @@ public function parse_response( Data_Request $data, $response ) {
return $response;
}

// If we have multiple date ranges, we need to sort rows to have them in
// the correct order.
if ( $multiple_ranges ) {
$rows = self::sort_response_rows( $rows, $date_ranges );
}

// Set updated rows back to the response object.
$response->setRows( array_values( $rows ) );
$response->setRowCount( $new_rows_count );
Expand All @@ -127,12 +122,110 @@ public function parse_response( Data_Request $data, $response ) {
*
* @since n.e.x.t
*
* @param string $date The date of the row to return key for.
* @param int $date_range_index The date range index.
* @param string $date The date of the row to return key for.
* @param int|bool $date_range_index The date range index, or FALSE if no index is available.
* @return string The row key.
*/
protected static function get_response_row_key( $date, $date_range_index ) {
return "{$date}_{$date_range_index}";
}

/**
* Returns sorted and filtered date ranges received in the request params. All corrupted date ranges
* are ignored and not included in the returning list.
*
* @since n.e.x.t
*
* @param Data_Request $data Data request object.
* @return Google_Service_AnalyticsData_DateRange[] An array of AnalyticsData DateRange objects.
*/
protected function get_sorted_dateranges( Data_Request $data ) {
$date_ranges = $this->parse_dateranges( $data );
if ( empty( $date_ranges ) ) {
return $date_ranges;
}

// Filter out all corrupted date ranges.
$date_ranges = array_filter(
$date_ranges,
function( $range ) {
$start = strtotime( $range->getStartDate() );
$end = strtotime( $range->getEndDate() );
return ! empty( $start ) && ! empty( $end );
}
);

// Sort date ranges preserving keys to have the oldest date range at the beginning and
// the latest date range at the end.
uasort(
$date_ranges,
function( $a, $b ) {
$a_start = strtotime( $a->getStartDate() );
$b_start = strtotime( $b->getStartDate() );
return $a_start - $b_start;
}
);

return $date_ranges;
}

/**
* Sorts response rows using the algorithm similar to the one that Analytics 4 uses internally
* and returns sorted rows.
*
* @since n.e.x.t
*
* @param Google_Service_AnalyticsData_Row[] $rows The current report rows.
* @param Google_Service_AnalyticsData_DateRange[] $ranges The report date ranges.
* @return Google_Service_AnalyticsData_Row[] Sorted rows.
*/
protected static function sort_response_rows( $rows, $date_ranges ) {
$sorted_rows = array();
$ranges_count = count( $date_ranges );

self::iterate_date_ranges(
$date_ranges,
function( $date, $range_index ) use ( &$sorted_rows, $ranges_count, $rows ) {
// First take the main date range row.
$key = self::get_response_row_key( $date, $range_index );
$sorted_rows[ $key ] = $rows[ $key ];

// Then take all remaining rows.
for ( $i = 0; $i < $ranges_count; $i++ ) {
if ( $i !== $range_index ) {
$key = self::get_response_row_key( $date, $i );
$sorted_rows[ $key ] = $rows[ $key ];
}
}
}
);

return $sorted_rows;
}

/**
* Iterates over the date ranges and calls callback for each date in each range.
*
* @since n.e.x.t
*
* @param Google_Service_AnalyticsData_DateRange[] $ranges The report date ranges.
* @param callable $callback The callback to execute fot each date.
techanvil marked this conversation as resolved.
Show resolved Hide resolved
*/
protected static function iterate_date_ranges( $date_ranges, $callback ) {
foreach ( $date_ranges as $date_range_index => $date_range ) {
$now = strtotime( $date_range->getStartDate() );
$end = strtotime( $date_range->getEndDate() );

do {
call_user_func(
$callback,
gmdate( 'Ymd', $now ),
$date_range_index
);

$now += DAY_IN_SECONDS;
} while ( $now <= $end );
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -91,26 +91,26 @@ public function data_report_args() {
'multiple ranges' => array(
array(
'report_args' => array(
'startDate' => '2023-02-01',
'endDate' => '2023-02-03',
'compareStartDate' => '2023-01-01',
'compareEndDate' => '2023-01-03',
'startDate' => '2022-12-05',
'endDate' => '2022-12-07',
'compareStartDate' => '2022-12-02',
'compareEndDate' => '2022-12-04',
'dimensions' => 'date',
),
'initial_data' => array(),
'expected_dates_and_ranges' => array(
array( '20230201', 'date_range_0' ),
array( '20230201', 'date_range_1' ),
array( '20230202', 'date_range_0' ),
array( '20230202', 'date_range_1' ),
array( '20230203', 'date_range_0' ),
array( '20230203', 'date_range_1' ),
array( '20230101', 'date_range_0' ),
array( '20230101', 'date_range_1' ),
array( '20230102', 'date_range_0' ),
array( '20230102', 'date_range_1' ),
array( '20230103', 'date_range_0' ),
array( '20230103', 'date_range_1' ),
array( '20221202', 'date_range_1' ),
array( '20221202', 'date_range_0' ),
array( '20221203', 'date_range_1' ),
array( '20221203', 'date_range_0' ),
array( '20221204', 'date_range_1' ),
array( '20221204', 'date_range_0' ),
array( '20221205', 'date_range_0' ),
array( '20221205', 'date_range_1' ),
array( '20221206', 'date_range_0' ),
array( '20221206', 'date_range_1' ),
array( '20221207', 'date_range_0' ),
array( '20221207', 'date_range_1' ),
),
'expected_dimension_values' => 2,
),
Expand All @@ -134,8 +134,8 @@ public function data_report_args() {
array( '20230103', 'date_range_1' ),
array( '20230104', 'date_range_0' ),
array( '20230104', 'date_range_1' ),
array( '20230105', 'date_range_0' ),
array( '20230105', 'date_range_1' ),
array( '20230105', 'date_range_0' ),
),
'expected_dimension_values' => 2,
),
Expand Down Expand Up @@ -175,18 +175,18 @@ public function data_report_args() {
array( '20230203', 0 ),
),
'expected_dates_and_ranges' => array(
array( '20230101', 'date_range_1' ),
array( '20230101', 'date_range_0' ),
array( '20230102', 'date_range_1' ),
array( '20230102', 'date_range_0' ),
array( '20230103', 'date_range_1' ),
array( '20230103', 'date_range_0' ),
array( '20230201', 'date_range_0' ),
array( '20230201', 'date_range_1' ),
array( '20230202', 'date_range_0' ),
array( '20230202', 'date_range_1' ),
array( '20230203', 'date_range_0' ),
array( '20230203', 'date_range_1' ),
array( '20230101', 'date_range_0' ),
array( '20230101', 'date_range_1' ),
array( '20230102', 'date_range_0' ),
array( '20230102', 'date_range_1' ),
array( '20230103', 'date_range_0' ),
array( '20230103', 'date_range_1' ),
),
'expected_dimension_values' => 2,
),
Expand Down