-
Notifications
You must be signed in to change notification settings - Fork 339
Enhancement/11092 refactor reporting key events datapoints #12391
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
Open
abdelmalekkkkk
wants to merge
8
commits into
develop
Choose a base branch
from
enhancement/11092-refactor-reporting-key-events-datapoints
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
c14b6df
Extract get report datapoint.
abdelmalekkkkk 55bb1cc
Add get report datapoint tests.
abdelmalekkkkk 5aedaf3
Extract has property access datapoint.
abdelmalekkkkk c830bea
Add tests for has property access datapoint.
abdelmalekkkkk 7929c6b
Extract get key events datapoint.
abdelmalekkkkk 79f4954
Improve import name.
abdelmalekkkkk d38443a
Fix is_shared_datapoint_request logic.
abdelmalekkkkk 5ab1bf9
Address review feedback.
abdelmalekkkkk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
77 changes: 77 additions & 0 deletions
77
includes/Modules/Analytics_4/Datapoints/Get_Has_Property_Access.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| <?php | ||
| /** | ||
| * Class Google\Site_Kit\Modules\Analytics_4\Datapoints\Get_Has_Property_Access | ||
| * | ||
| * @package Google\Site_Kit\Modules\Analytics_4\Datapoints | ||
| * @copyright 2026 Google LLC | ||
| * @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0 | ||
| * @link https://sitekit.withgoogle.com | ||
| */ | ||
|
|
||
| namespace Google\Site_Kit\Modules\Analytics_4\Datapoints; | ||
|
|
||
| use Google\Site_Kit\Core\Modules\Datapoint; | ||
| use Google\Site_Kit\Core\Modules\Executable_Datapoint; | ||
| use Google\Site_Kit\Core\REST_API\Data_Request; | ||
| use Google\Site_Kit\Core\REST_API\Exception\Missing_Required_Param_Exception; | ||
| use Google\Site_Kit_Dependencies\Google\Service\AnalyticsData\RunReportRequest as Google_Service_AnalyticsData_RunReportRequest; | ||
| use Google\Site_Kit_Dependencies\Google\Service\AnalyticsData\DateRange as Google_Service_AnalyticsData_DateRange; | ||
| use Google\Site_Kit_Dependencies\Google\Service\AnalyticsData\Dimension as Google_Service_AnalyticsData_Dimension; | ||
| use Google\Site_Kit_Dependencies\Google\Service\AnalyticsData\Metric as Google_Service_AnalyticsData_Metric; | ||
|
|
||
| /** | ||
| * Has property access datapoint. | ||
| * | ||
| * @since n.e.x.t | ||
| * @access private | ||
| * @ignore | ||
| */ | ||
| class Get_Has_Property_Access extends Datapoint implements Executable_Datapoint { | ||
|
|
||
| /** | ||
| * Creates a request object. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param Data_Request $data Data request object. | ||
| * @return mixed Request object on success, or WP_Error on failure. | ||
| * @throws Missing_Required_Param_Exception Thrown if a required parameter is missing. | ||
| */ | ||
| public function create_request( Data_Request $data ) { | ||
| if ( ! isset( $data['propertyID'] ) ) { | ||
| throw new Missing_Required_Param_Exception( 'propertyID' ); | ||
| } | ||
|
|
||
| // A simple way to check for property access is to attempt a minimal report request. | ||
| // If the user does not have access, this will return a 403 error. | ||
| $request = new Google_Service_AnalyticsData_RunReportRequest(); | ||
| $request->setDimensions( array( new Google_Service_AnalyticsData_Dimension( array( 'name' => 'date' ) ) ) ); | ||
| $request->setMetrics( array( new Google_Service_AnalyticsData_Metric( array( 'name' => 'sessions' ) ) ) ); | ||
| $request->setDateRanges( | ||
| array( | ||
| new Google_Service_AnalyticsData_DateRange( | ||
| array( | ||
| 'start_date' => 'yesterday', | ||
| 'end_date' => 'today', | ||
| ) | ||
| ), | ||
| ) | ||
| ); | ||
| $request->setLimit( 0 ); | ||
|
|
||
| return $this->get_service()->properties->runReport( $data['propertyID'], $request ); | ||
| } | ||
|
|
||
| /** | ||
| * Parses a response. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param mixed $response Request response. | ||
| * @param Data_Request $data Data request object. | ||
| * @return mixed The original response without any modifications. | ||
| */ | ||
| public function parse_response( $response, Data_Request $data ) { | ||
| return $response; | ||
| } | ||
| } |
88 changes: 88 additions & 0 deletions
88
includes/Modules/Analytics_4/Datapoints/Get_Key_Events.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| <?php | ||
| /** | ||
| * Class Google\Site_Kit\Modules\Analytics_4\Datapoints\Get_Key_Events | ||
| * | ||
| * @package Google\Site_Kit\Modules\Analytics_4\Datapoints | ||
| * @copyright 2026 Google LLC | ||
| * @license https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0 | ||
| * @link https://sitekit.withgoogle.com | ||
| */ | ||
|
|
||
| namespace Google\Site_Kit\Modules\Analytics_4\Datapoints; | ||
|
|
||
| use Google\Site_Kit\Core\Modules\Executable_Datapoint; | ||
| use Google\Site_Kit\Core\Modules\Shareable_Datapoint; | ||
| use Google\Site_Kit\Core\REST_API\Data_Request; | ||
| use Google\Site_Kit\Modules\Analytics_4; | ||
| use Google\Site_Kit_Dependencies\Google\Service\GoogleAnalyticsAdmin\GoogleAnalyticsAdminV1betaKeyEvent; | ||
| use Google\Site_Kit\Modules\Analytics_4\Settings; | ||
| use WP_Error; | ||
|
|
||
| /** | ||
| * Get key events datapoint class. | ||
| * | ||
| * @since n.e.x.t | ||
| * @access private | ||
| * @ignore | ||
| */ | ||
| class Get_Key_Events extends Shareable_Datapoint implements Executable_Datapoint { | ||
|
|
||
| /** | ||
| * Module settings instance. | ||
| * | ||
| * @since n.e.x.t | ||
| * @var Settings | ||
| */ | ||
| private $settings; | ||
|
|
||
| /** | ||
| * Constructor. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param array $definition Definition fields. | ||
| */ | ||
| public function __construct( array $definition ) { | ||
| parent::__construct( $definition ); | ||
| $this->settings = $definition['settings']; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a request object. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param Data_Request $data Data request object. | ||
| * @return mixed Request object on success, or WP_Error on failure. | ||
| */ | ||
| public function create_request( Data_Request $data ) { | ||
| $settings = $this->settings->get(); | ||
|
|
||
| if ( empty( $settings['propertyID'] ) ) { | ||
| return new WP_Error( | ||
| 'missing_required_setting', | ||
| __( 'No connected Google Analytics property ID.', 'google-site-kit' ), | ||
| array( 'status' => 500 ) | ||
| ); | ||
| } | ||
|
|
||
| $property_id = Analytics_4::normalize_property_id( $settings['propertyID'] ); | ||
|
|
||
| return $this->get_service() | ||
| ->properties_keyEvents // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase | ||
| ->listPropertiesKeyEvents( $property_id ); | ||
| } | ||
|
|
||
| /** | ||
| * Parses a response. | ||
| * | ||
| * @since n.e.x.t | ||
| * | ||
| * @param mixed $response Request response. | ||
| * @param Data_Request $data Data request object. | ||
| * @return GoogleAnalyticsAdminV1betaKeyEvent[] Array of key events. | ||
| */ | ||
| public function parse_response( $response, Data_Request $data ) { | ||
| return (array) $response->getKeyEvents(); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refactored
is_shared_data_request()no longer accounts for the scope validation fallback inget_oauth_client_for_datapoint().Previously, if all sharing conditions were met but the owner's scopes failed validation,
get_oauth_client_for_datapoint()would fall back to the default client, andis_shared_data_request()would returnfalse(since the clients matched). Now it returnstrueregardless of scope validation.This means shared-data restrictions (e.g. date range limits in
GET:batch-report, metric/dimension validation in AdSense) could be applied to requests that are actually using the current user's own credentials — potentially restricting data the user should have full access to.Should this method preserve the old scope-aware behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out. I thought checking the clients was just a quick way to determine if the request was shared. I reverted some of the changes and only left the new
is_shared_datapoint_request. Please take another look.