-
Notifications
You must be signed in to change notification settings - Fork 292
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
Create API endpoints and Redux store infrastructure for "Enable enhanced measurement" feature. #7458
Comments
Thanks @techanvil ! There are a few inconsistencies in the naming
For selectors returning a boolean, these are usually starting with Perhaps more importantly, we should think about which of the sub-settings are important to us if not "all" since EM can be enabled while all of the additional measurements it has can be disabled which would be silly but possible. Similarly, what if some of the sub-settings are turned off? Does "enabled" for us mean everything is enabled or are there certain events that are important?
We tend to keep the naming for these kinds of actions close to the underlying endpoint, so it might be more consistent to use |
Thanks for taking a look at this @aaemnnosttv.
Both good points, thanks! I've updated accordingly.
Based on #7459 and the edge-case handling issues currently in Triage (#7475, #7476 and #7477, see below) I think that to start with we'll just be needing to check two values: one, the simple state of To provide a streamlined API in keeping with other similar cases where we provide individual getter/setter selectors and actions for an underlying collection (e.g. various settings, site info etc), I've renamed My thinking is that we should also spec a
Thanks for pointing this out, you've made me realise there are some edge cases that we need to determine the approach for. I've re-read the related discussion on Asana, and given it some further thought. As a result, and as mentioned above I've created three additional issues which are currently in Triage where we can flesh out the approach to these cases.
Sounds good - I did take a look to see if I could spot anything analagous naming wise, but nothing stood out at the time so I went with the common get/save combination. However, as a principle that certainly SGTM and I've made the change to |
This looks good, and I know there's an existing proof-of-concept PR here, but this is a pretty involved issue so I'm gonna up the estimate a bit just to be safe. IB ✅ |
Hi @hussain-t, as discussed, I've assigned this back to you because while working on #7459, having branched off your branch, I've noticed an issue that I/we didn't spot during the PoC, IB or initial execution. The problem is that the As can be seen in the version of Although you've implemented this issue as per the spec, we should address this missing aspect here in order to be able to QA this properly, as this inconsistent model can otherwise cause problems when testing. As per our conversation, the solution should be quite straightforward, we simply need to use our own model implementation that's essentially a copy of As an aside, thanks for jumping on a call so late to discuss this, definitely above and beyond the call of duty but certainly appreciated :) |
Thanks, @techanvil. The above issue has been addressed. |
…asurement Infrastructure/#7458 - Create API endpoints and Redux store infrastructure for "Enable enhanced measurement" feature
QA Update: ✅Verified: Checked with Hussain via Slack - this is to be tested outside of the
|
Feature Description
API endpoints and Redux store infrastructure should be created as required to get/set enhanced measurement settings and thus toggle the enhanced measurement enabled state for a given web data stream. It should be possible to update the local settings state, check if settings have changed locally and persist or reset the changes.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
API Endpoints
GET
/POST
datapoints namedenhanced-measurement-settings
should be created in theanalytics-4
module.GET
endpoint:propertyID: string
andwebDataStreamID: string
.getEnhancedMeasurementSettings
.POST
endpoint:propertyID: string
,webDataStreamID: string
, andenhancedMeasurementSettings
which is a JSON object as defined by theEnhancedMeasurementSettings
type.updateEnhancedMeasurementSettings
.EnhancedMeasurementSettings
JSON object.Redux store
analytics-4
module. Additional store infrastructure should be added as necessary to support these top-level requirements:setEnhancedMeasurementStreamEnabled( propertyID: string, webDataStreamID: string, enabled: boolean )
streamEnabled
setting) for the given web data stream.updateEnhancedMeasurementSettings( propertyID: string, webDataStreamID: string )
POST
to theenhanced-measurement-settings
endpoint to persist the changes for the given web data stream.resetEnhancedMeasurementSettings()
isEnhancedMeasurementStreamEnabled( propertyID: string, webDataStreamID: string ): boolean
streamEnabled
setting) for the given web data stream.GET
to theenhanced-measurement-settings
endpoint to populate the initial state.haveEnhancedMeasurementSettingsChanged( propertyID: string, webDataStreamID: string ): boolean
Implementation Brief
IB Note: A PoC has been created, notably resolving some of the more obscure aspects of how to call the GA4 API methods using the
google-api-php-client-services
PHP package.API endpoints
Note: Although the
google-api-php-client-services
PHP package did at one point provide support for calling the GA4getEnhancedMeasurementSettings
andupdateEnhancedMeasurementSettings
methods, it was removed back in version 0.221, and since then the package has gone on to support the v1beta version of the API which doesn't provide these methods at all.Therefore, in order to provide support for these methods, we need to reimplement some code from
v0.220.0
of the package. Fortunately, although support was removed from theGoogleAnalyticsAdmin
service andPropertiesWebDataStreams
resource classes, the package does still provide the underlyingGoogleAnalyticsAdminV1alphaEnhancedMeasurementSettings
model class, which we can utilise in the following implementation.Restore support for calling the GA4 API methods
Google\Site_Kit\Modules\Analytics_4\GoogleAnalyticsAdmin
:PropertiesEnhancedMeasurementResource
which extendsGoogle\Site_Kit_Dependencies\Google\Service\Resource
.AccountsResource
for an example of a class where we extendResource
.getEnhancedMeasurementSettings()
andupdateEnhancedMeasurementSettings()
methods into this class fromv0.220.0
ofPropertiesWebDataStreams
.PropertiesEnhancedMeasurementService
which extendsGoogle\Site_Kit_Dependencies\Google\Service\GoogleAnalyticsAdmin
.AccountProvisioningService
for an example of a class where we extendGoogleAnalyticsAdmin
.v0.220.0
ofGoogleAnalyticsAdmin
to provide support forgetEnhancedMeasurementSettings
andupdateEnhancedMeasurementSettings
- but using our ownPropertiesEnhancedMeasurementResource
in place ofPropertiesWebDataStreams
.PropertiesEnhancedMeasurementsResource
->PropertiesEnhancedMeasurementResource
etc.)Add our own REST endpoints
Analytics_4
class:setup_services()
, add a new service calledanalyticsenhancedmeasurement
which is an instance ofPropertiesEnhancedMeasurementService
.get_datapoint_definitions()
, addGET
andPOST
datapoints calledenhanced-measurement-settings
.analyticsenhancedmeasurement
service.POST
datapoint should require thehttps://www.googleapis.com/auth/analytics.edit
scope.create_data_request()
, add cases for the newGET
andPOST
datapoints.getEnhancedMeasurementSettings()
andupdateEnhancedMeasurementSettings()
methods provided via thePropertiesEnhancedMeasurementResource
instance of theanalyticsenhancedmeasurement
service to make requests to the GA4 API.POST
endpoint, theupdateMask
option should have the valuestreamEnabled
, ensuring that we can't update any other fields for now. See https://developers.google.com/analytics/devguides/config/admin/v1/rest/v1alpha/properties.dataStreams/updateEnhancedMeasurementSettings#query-parameters.Redux store
assets/js/modules/analytics-4/datastore/enhanced-measurement.js
.assets/js/modules/analytics-4/datastore/index.js
.savedSettings
may be omitted if the store hasn't been populated via a call to theGET
endpoint). SeeEnhancedMeasurementSettings
.GET
andPOST
endpoints, with respective basenamesgetEnhancedMeasurementSettings
andupdateEnhancedMeasurementSettings
.settings
andsavedSettings
to be theEnhancedMeasurementSettings
JSON object returned by the endpoint.Actions
setEnhancedMeasurementSettings( propertyID: string, webDataStreamID: string, settings Partial<EnhancedMeasurementSettings> )
settings
state for the givenpropertyID
andwebDataStreamID
being updated with the provided object. The given object should entirely replace the current state.streamEnabled
) without first retrieving the current settings from the backend, and it would not make sense to attempt to provide all of the settings in this case as we wouldn't have a meaningful value for them.setEnhancedMeasurementStreamEnabled( propertyID: string, webDataStreamID: string, enabled: boolean )
streamEnabled
setting for the data stream, via a retrieval of the current settings, inversion of thestreamEnabled
flag, and subsequent dispatch of thesetEnhancedMeasurementSettings()
action.updateEnhancedMeasurementSettings( propertyID: string, webDataStreamID: string )
fetchUpdateEnhancedMeasurementSettings()
with the currentsettings
state for the givenpropertyID
andwebDataStreamID
, thus updating the settings via the GA4 API.resetEnhancedMeasurementSettings()
settings
state for all web data streams being reverted to the correspondingsavedSettings
state. IfsavedSettings
is not defined, the state for the given web data stream should be entirely removed.Selectors
getEnhancedMeasurementSettings( propertyID: string, webDataStreamID: string ): EnhancedMeasurementSettings
settings
state for the givenpropertyID
andwebDataStreamID
.propertyID
andwebDataStreamID
is not yet defined, the fetch store generated actionfetchGetEnhancedMeasurementSettings()
is dispatched to populate the settings from the GA4 API.isEnhancedMeasurementStreamEnabled( propertyID: string, webDataStreamID: string ): boolean
settings
state viagetEnhancedMeasurementSettings()
and returns thestreamEnabled
property.haveEnhancedMeasurementSettingsChanged( propertyID: string, webDataStreamID: string ): boolean
settings
andsavedSettings
state for the givenpropertyID
andwebDataStreamID
.Again, some of the Redux implementation has been fleshed out in the PoC.
Test Coverage
QA Brief
GoogleTagIDMismatchNotification
banner and ensure no regression occurred. This is due to the changes made to thegetAnalyticsConfigByMeasurementIDs
selector used in the banner.Actions
Action:
setEnhancedMeasurementStreamEnabled( propertyID: string, webDataStreamID: string, enabled: boolean )
Execute the action by running the following command:
Replace
YOUR_PROPERTY_ID
andYOUR_WEBDATASTREAM_ID
with actual values.Verify that the action has updated the client-side state correctly.
Run the selector to double-check:
It should return
true
.Action:
updateEnhancedMeasurementSettings( propertyID: string, webDataStreamID: string )
Execute the following selector to get the current settings:
Execute the action by running the following command:
Check the network tab for a
POST
request toenhanced-measurement-settings
and verify that it contains the correct data and that the response is a 200 OK.Action:
resetEnhancedMeasurementSettings()
Execute the action by running the following command:
Use the selector to check if the settings have indeed been reset.
It should return the initial state.
Selectors
Selector:
getEnhancedMeasurementSettings( propertyID: string, webDataStreamID: string ): EnhancedMeasurementSettings
Execute the selector:
Verify that it either returns the correct EnhancedMeasurementSettings object or triggers a
GET
request toenhanced-measurement-settings
if the state is not yet defined.Selector:
isEnhancedMeasurementStreamEnabled( propertyID: string, webDataStreamID: string ): boolean
Execute the selector:
Verify that it either returns the correct boolean value.
Selector:
haveEnhancedMeasurementSettingsChanged( propertyID: string, webDataStreamID: string ): boolean
Execute the selector:
Verify that it returns
true
if there are unsaved changes andfalse
otherwise.Changelog entry
The text was updated successfully, but these errors were encountered: