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

Add GET:report datapoint for GA4. #6379

Merged
merged 57 commits into from
Jan 13, 2023
Merged

Add GET:report datapoint for GA4. #6379

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
54b16e1
Add initial Analytics-4 GET:report implementation.
techanvil Jan 5, 2023
45e9dde
Add metric aggregations to match UA reports.
techanvil Jan 5, 2023
384d626
Update return type.
techanvil Jan 5, 2023
065681e
Fix typo.
techanvil Jan 5, 2023
ad4f5df
Add a reminder to remove metrics/dimensions for subsequent allow-list…
techanvil Jan 5, 2023
423a820
Update error codes for GA4.
techanvil Jan 5, 2023
a0974d6
Update metric validation.
techanvil Jan 5, 2023
f56e24d
Update metric parsing to reflect requirement for `name`.
techanvil Jan 5, 2023
88a47bc
Update comment.
techanvil Jan 5, 2023
b9f041d
Fix test.
techanvil Jan 5, 2023
695a5ec
Add PHPUnit test for GET:report.
techanvil Jan 5, 2023
4c0b1a9
Remove metrics and dimensions from their allow-lists.
techanvil Jan 6, 2023
717bf16
Flesh out GET:report test coverage.
techanvil Jan 6, 2023
c1a6229
Re-order report parameter validation.
techanvil Jan 6, 2023
857f745
Fix metric validation.
techanvil Jan 6, 2023
05db15a
Add tests for report parameter validation.
techanvil Jan 6, 2023
6e2da29
Remove comment.
techanvil Jan 6, 2023
43df5f9
Update parameter description.
techanvil Jan 6, 2023
7ed6d1f
Simplify test slightly.
techanvil Jan 6, 2023
7056c14
Fix comment.
techanvil Jan 6, 2023
2b0bfee
Add missing @param doc line.
techanvil Jan 6, 2023
b714579
Fix comment alignment.
techanvil Jan 6, 2023
98243e0
Trim unused imports.
techanvil Jan 6, 2023
b586ad2
Extend test coverage to all GET:report input parameters.
techanvil Jan 9, 2023
13334bf
Add notes about required/optional parameter usage.
techanvil Jan 9, 2023
74e7b4d
Retrieve property ID from module settings within GET:report handler.
techanvil Jan 10, 2023
cb54367
Use the single-report runReport rather than batchRunReports.
techanvil Jan 10, 2023
ef6d6c7
Remove unnecessary fake HTTP client.
techanvil Jan 10, 2023
00c9fdd
Change request_handler_calls to be an instance variable.
techanvil Jan 10, 2023
f78b1f7
Move various object creation into test setup method.
techanvil Jan 10, 2023
3be82f8
Extract fake HTTP client creation and refactor a bit.
techanvil Jan 10, 2023
945e79b
Remove get_report helper method.
techanvil Jan 10, 2023
b5e7e32
Replace setup_report with enable_shared_credentials helper method.
techanvil Jan 10, 2023
6236f38
Only enable dashboardSharing for those tests which are relevant.
techanvil Jan 10, 2023
850fb82
Order services by name.
techanvil Jan 10, 2023
790d8c3
Create helper methods for setting shared metrics and dimensions.
techanvil Jan 10, 2023
15be316
Add authenticated user variants for all relevant report tests.
techanvil Jan 10, 2023
1be6cb4
Remove @since from test function.
techanvil Jan 10, 2023
e6a969d
Remove unused import.
techanvil Jan 12, 2023
eca7c7e
Return error if no connected property and move calls to normalize_pro…
techanvil Jan 12, 2023
6407fd5
Don't fake HTTP client for tests which don't need it.
techanvil Jan 12, 2023
4093e6f
Add test for insufficient permissions.
techanvil Jan 12, 2023
a61350a
Add test for default date range.
techanvil Jan 12, 2023
203fbe0
Remove legacy date params.
techanvil Jan 12, 2023
4037108
Use permute_site_hosts rather than mapping result of permute_site_url.
techanvil Jan 12, 2023
82e3567
Reuse setup_user_authentication.
techanvil Jan 12, 2023
732f9be
Set new admin as owner of Analytics 4 module for test.
techanvil Jan 12, 2023
70e4d78
Set shared roled from current user, and use default management: owner…
techanvil Jan 12, 2023
93a6b27
Extract setting user access token to a new trait.
techanvil Jan 12, 2023
e9c0a41
Merge branch 'develop' into enhancement/6172-analytics-4-get-report.
aaemnnosttv Jan 13, 2023
38f8695
Change status code to 400 for metrics and dimensions validation errors.
techanvil Jan 13, 2023
80476e8
Change the "no property ID setting" error to be a 500.
techanvil Jan 13, 2023
f85fc3e
Move call to set granted scopes into the test cases.
techanvil Jan 13, 2023
06d6faf
Add missing import.
aaemnnosttv Jan 13, 2023
615f0ad
Remove HTTP status from DS validation.
aaemnnosttv Jan 13, 2023
a3183f3
Require property ID after required params.
aaemnnosttv Jan 13, 2023
01c24e5
Update formatting of returned errors.
aaemnnosttv Jan 13, 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
15 changes: 9 additions & 6 deletions includes/Modules/Analytics_4.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,10 +876,6 @@ protected function create_report_request( Data_Request $data ) {

$option = $this->get_settings()->get();

if ( empty( $option['propertyID'] ) ) {
return new WP_Error( 'missing_required_setting', __( 'No connected Google Analytics 4 property ID.', 'google-site-kit' ), array( 'status' => 400 ) );
}

if ( empty( $data['metrics'] ) ) {
/* translators: %s: Missing parameter name */
return new WP_Error( 'missing_required_param', sprintf( __( 'Request parameter is empty: %s.', 'google-site-kit' ), 'metrics' ), array( 'status' => 400 ) );
Expand Down Expand Up @@ -926,7 +922,8 @@ function ( $dimension_def ) {
} catch ( Invalid_Report_Dimensions_Exception $exception ) {
return new WP_Error(
'invalid_analytics_4_report_dimensions',
$exception->getMessage()
$exception->getMessage(),
array( 'status' => 400 )
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can see why you used the status here as well but let's not do that because it would imply we should make the same changes in the other modules that also implement this check. Aside from that, I think the default status is okay here because the client is not making an invalid request – the validation is applied due to internal state/conditions related to dashboard sharing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No worries and thanks for changing it back 👍

);
}

Expand Down Expand Up @@ -1034,7 +1031,8 @@ function ( $metric_def ) {
} catch ( Invalid_Report_Metrics_Exception $exception ) {
return new WP_Error(
'invalid_analytics_4_report_metrics',
$exception->getMessage()
$exception->getMessage(),
array( 'status' => 400 )
);
}

Expand All @@ -1057,6 +1055,11 @@ function ( $metric_def ) {
)
);

if ( empty( $option['propertyID'] ) ) {
// Only return a 500 Internal Server error once we've checked for all 400 Bad Request errors above.
return new WP_Error( 'missing_required_setting', __( 'No connected Google Analytics 4 property ID.', 'google-site-kit' ), array( 'status' => 500 ) );
}

return $this->get_analyticsdata_service()->properties->runReport( self::normalize_property_id( $option['propertyID'] ), $request );
}

Expand Down
98 changes: 66 additions & 32 deletions tests/phpunit/integration/Modules/Analytics_4Test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,11 @@ public function test_get_report( $access_token ) {
)
);

// Grant scopes so request doesn't fail.
$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$http_client = $this->create_fake_http_client( $property_id );
$this->analytics->get_client()->setHttpClient( $http_client );
$this->analytics->register();
Expand Down Expand Up @@ -491,6 +496,11 @@ public function test_get_report__default_date_range( $access_token ) {
)
);

// Grant scopes so request doesn't fail.
$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$http_client = $this->create_fake_http_client( $property_id );
$this->analytics->get_client()->setHttpClient( $http_client );
$this->analytics->register();
Expand Down Expand Up @@ -537,6 +547,11 @@ public function test_get_report__metrics_as_string( $access_token ) {
)
);

// Grant scopes so request doesn't fail.
$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$http_client = $this->create_fake_http_client( $property_id );
$this->analytics->get_client()->setHttpClient( $http_client );
$this->analytics->register();
Expand Down Expand Up @@ -582,6 +597,11 @@ public function test_get_report__metrics_as_single_object( $access_token ) {
)
);

// Grant scopes so request doesn't fail.
$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$http_client = $this->create_fake_http_client( $property_id );
$this->analytics->get_client()->setHttpClient( $http_client );
$this->analytics->register();
Expand Down Expand Up @@ -628,6 +648,11 @@ public function test_get_report__dimensions_as_string( $access_token ) {
)
);

// Grant scopes so request doesn't fail.
$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$http_client = $this->create_fake_http_client( $property_id );
$this->analytics->get_client()->setHttpClient( $http_client );
$this->analytics->register();
Expand Down Expand Up @@ -674,6 +699,11 @@ public function test_get_report__dimensions_as_single_object( $access_token ) {
)
);

// Grant scopes so request doesn't fail.
$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$http_client = $this->create_fake_http_client( $property_id );
$this->analytics->get_client()->setHttpClient( $http_client );
$this->analytics->register();
Expand Down Expand Up @@ -714,25 +744,7 @@ public function test_report__insufficient_permissions( $access_token ) {
$data = $this->analytics->get_data( 'report', array() );

$this->assertWPErrorWithMessage( 'Site Kit can’t access the relevant data from Analytics 4 because you haven’t granted all permissions requested during setup.', $data );
}

/**
* @dataProvider data_access_token
*
* When an access token is provided, the user will be authenticated for the test.
*
* @param string $access_token Access token, or empty string if none.
*/
public function test_report__no_property_id( $access_token ) {
$this->setup_user_authentication( $access_token );

$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$data = $this->analytics->get_data( 'report', array() );

$this->assertWPErrorWithMessage( 'No connected Google Analytics 4 property ID.', $data );
$this->assertEquals( 'missing_required_scopes', $data->get_error_code() );
}

/**
Expand Down Expand Up @@ -760,6 +772,8 @@ public function test_report__no_metrics( $access_token ) {
$data = $this->analytics->get_data( 'report', array() );

$this->assertWPErrorWithMessage( 'Request parameter is empty: metrics.', $data );
$this->assertEquals( 'missing_required_param', $data->get_error_code() );
$this->assertEquals( array( 'status' => 400 ), $data->get_error_data( 'missing_required_param' ) );
}

public function test_report__metric_validation() {
Expand All @@ -786,10 +800,6 @@ public function test_report__metric_validation() {
)
);

$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$this->set_shareable_metrics( 'sessions', 'totalUsers' );

$this->enable_shared_credentials();
Expand All @@ -808,6 +818,8 @@ public function test_report__metric_validation() {
);

$this->assertWPErrorWithMessage( 'Unsupported metrics requested: invalidMetric, anotherInvalidMetric', $data );
$this->assertEquals( 'invalid_analytics_4_report_metrics', $data->get_error_code() );
$this->assertEquals( array( 'status' => 400 ), $data->get_error_data( 'invalid_analytics_4_report_metrics' ) );
}

public function test_report__dimension_validation() {
Expand All @@ -834,10 +846,6 @@ public function test_report__dimension_validation() {
)
);

$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$this->set_shareable_metrics( 'sessions' );
$this->set_shareable_dimensions( 'date', 'pageTitle' );

Expand All @@ -855,6 +863,37 @@ public function test_report__dimension_validation() {
);

$this->assertWPErrorWithMessage( 'Unsupported dimensions requested: invalidDimension, anotherInvalidDimension', $data );
$this->assertEquals( 'invalid_analytics_4_report_dimensions', $data->get_error_code() );
$this->assertEquals( array( 'status' => 400 ), $data->get_error_data( 'invalid_analytics_4_report_dimensions' ) );
}

/**
* @dataProvider data_access_token
*
* When an access token is provided, the user will be authenticated for the test.
*
* @param string $access_token Access token, or empty string if none.
*/
public function test_report__no_property_id( $access_token ) {
$this->setup_user_authentication( $access_token );

$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$data = $this->analytics->get_data(
'report',
array(
// Note, metrics is a required parameter.
'metrics' => array(
array( 'name' => 'sessions' ),
),
)
);

$this->assertWPErrorWithMessage( 'No connected Google Analytics 4 property ID.', $data );
$this->assertEquals( 'missing_required_setting', $data->get_error_code() );
$this->assertEquals( array( 'status' => 500 ), $data->get_error_data( 'missing_required_setting' ) );
}

/**
Expand Down Expand Up @@ -904,11 +943,6 @@ function() use ( $dimensions ) {
protected function create_fake_http_client( $property_id ) {
$this->request_handler_calls = array();

// Grant scopes so request doesn't fail.
$this->authentication->get_oauth_client()->set_granted_scopes(
$this->analytics->get_scopes()
);

$http_client = new FakeHttpClient();
$http_client->set_request_handler(
function ( Request $request ) use ( $property_id ) {
Expand Down