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

Enhanced Measurement Created Despite Disabled #8409

Closed
4 tasks
wpdarren opened this issue Mar 18, 2024 · 4 comments
Closed
4 tasks

Enhanced Measurement Created Despite Disabled #8409

wpdarren opened this issue Mar 18, 2024 · 4 comments
Labels
Module: Analytics Google Analytics module related issues P0 High priority Squad 1 (Team S) Issues for Squad 1 Type: Bug Something isn't working

Comments

@wpdarren
Copy link
Collaborator

wpdarren commented Mar 18, 2024

Bug Description

While completing testing on the Singular Analytics Module, I noticed that when setting up Analytics, the user creates a new property/web data stream and disables Enhanced Measurement; when the new property is created, the Enhanced Measurement toggle is changed and enabled. This is not a regression, as it occurs on the latest release.

You can see it in action on this screencast.

sam-em.1.mp4

Steps to reproduce

  1. Activate Analytics Module.
  2. During setup, select and existing account but chose to create a New property.
  3. Toggle the enhanced measurement switch off.
  4. Continue setup.
  5. See that the enhance measurement for the newly created property/data stream is enabled anyways.

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • Creating a new property (or new measurement ID) during Analytics setup should honor the "Enable enhanced measurement toggle" and only enable enhanced measurement for the newly created web data stream associated with newly created property when the toggle is "On".

Implementation Brief

It seems we do not have an implementation for passing the enhanced measurement settings when new web data stream is created from settings, unlike how it is done in account create flow.

  • Go to the assets/js/modules/analytics-4/datastore/webdatastreams.js and update the createWebDataStream action
    • Collect the value from the CORE_FORMS where enhanced measurement switch component will store the current value
      const isEnhancedMeasurementEnabled = useSelect( ( select ) =>
      select( CORE_FORMS ).getValue( formName, ENHANCED_MEASUREMENT_ENABLED )
      );
    • Pass it as a third parameter to the fetchCreateWebDataStream function , as enhancedMeasurementStreamEnabled
      yield fetchCreateWebDataStreamStore.actions.fetchCreateWebDataStream(
      propertyID,
      displayName
      );
      • And include it in
        const fetchCreateWebDataStreamStore = createFetchStore( {
        baseName: 'createWebDataStream',
        controlCallback( { propertyID, displayName } ) {
        return API.set( 'modules', 'analytics-4', 'create-webdatastream', {
        propertyID,
        displayName,
        } );
        },
        reducerCallback( state, webDataStream, { propertyID } ) {
        return {
        ...state,
        webdatastreams: {
        ...state.webdatastreams,
        [ propertyID ]: [
        ...( state.webdatastreams[ propertyID ] || [] ),
        webDataStream,
        ],
        },
        };
        },
        argsToParams( propertyID, displayName ) {
        return { propertyID, displayName };
        },
        validateParams( { propertyID, displayName } = {} ) {
        invariant(
        isValidPropertyID( propertyID ),
        'A valid GA4 propertyID is required.'
        );
        invariant(
        isValidWebDataStreamName( displayName ),
        'A valid web data stream name is required.'
        );
        },
        } );
  • Update Google\Site_Kit\Modules\Analytics_4 class
    • Modify the POST:create-webdatastream endpoint in create_data_request method
      case 'POST:create-webdatastream':
      if ( ! isset( $data['propertyID'] ) ) {
      return new WP_Error(
      'missing_required_param',
      /* translators: %s: Missing parameter name */
      sprintf( __( 'Request parameter is empty: %s.', 'google-site-kit' ), 'propertyID' ),
      array( 'status' => 400 )
      );
      }
      $options = array(
      'displayName' => $data['displayName'],
      );
      return $this->create_webdatastream( $data['propertyID'], $options );
      to check for newly added enhancedMeasurementStreamEnabled parameter, and pass it to the create_webdatastream method. Make it as an optional parameter
    • In create_webdatastream method
      • Instead of returning the response straight, return it to a variable, and if enhancedMeasurementStreamEnabled parameter is available send the request to the enhanced-measurement-settings endpoint
        $this->set_data(
        'enhanced-measurement-settings',
        array(
        'propertyID' => $property->_id,
        'webDataStreamID' => $web_datastream->_id,
        'enhancedMeasurementSettings' => array(
        // We can hardcode this to `true` here due to the conditional invocation.
        'streamEnabled' => true,
        ),
        )
        );
        }
        with the needed parameters. You can acquire web data stream ID from response’s _id property
      • Return the web data stream creation response.

Test Coverage

  • No update needed

QA Brief

  • Go through Steps to reproduce section
  • Note the importance of the need to grant the additional edit permission to create a new property/datastream in reproduce the issue (going through OAuth and coming back and the related automatic creation afterwards)
  • Verify enhanced measurement setting is keeping the value selected during new property/web data stream creation.

Additional testing points

  • Note that behaviour for the switch differes per Settings/Setup module screens:
    • On setup module screen, switch should be "pushy", it should be enabled by default, even if on the selected property you disabled enhanced measurement switch should be enabled by default, as well as whenever you switch to different/new property or/and web data stream switch should reset to on by default
    • On Settings page, the switch should be less pushy, and if you disabled enhanced measurement for selected property, it should show as off by default or when switching between account/property/web data stream dropdown.
  • Do some general smoke testing on this behaviour as well

Changelog entry

  • Fix a bug where the Enhanced Measurement toggle may not have the intended effect when creating a new property during Analytics module setup.
@wpdarren wpdarren added Type: Bug Something isn't working Module: Analytics Google Analytics module related issues labels Mar 18, 2024
@kuasha420
Copy link
Collaborator

kuasha420 commented Mar 22, 2024

I was doing release testing on main and created new everything (account, property, data steam) and disabled EM. All went well.

Even the EM banner showed up right after.

Screenshot_20240322_110527
Screenshot_20240322_110551

However, I can reproduce this by selecting an existing account and creating a new Property on it. Note that is only happens in setup, and not when creating a new property in Settings.

Moving this forward. 👍

@kuasha420 kuasha420 added the P0 High priority label Mar 22, 2024
@kuasha420 kuasha420 assigned kuasha420 and unassigned kuasha420 Mar 22, 2024
@eugene-manuilov eugene-manuilov self-assigned this Mar 22, 2024
@eugene-manuilov
Copy link
Collaborator

AC ✔️

@eugene-manuilov eugene-manuilov removed their assignment Mar 22, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Mar 28, 2024
@tofumatt tofumatt self-assigned this Mar 28, 2024
@tofumatt
Copy link
Collaborator

IB ✅

@tofumatt tofumatt removed their assignment Mar 28, 2024
@ivonac4 ivonac4 added the Squad 2 (Team M) Issues for Squad 2 label Apr 1, 2024
@bethanylang bethanylang added Squad 1 (Team S) Issues for Squad 1 and removed Squad 2 (Team M) Issues for Squad 2 labels Apr 3, 2024
@zutigrm zutigrm self-assigned this Apr 4, 2024
@zutigrm zutigrm removed their assignment Apr 9, 2024
@aaemnnosttv aaemnnosttv assigned aaemnnosttv and zutigrm and unassigned aaemnnosttv Apr 9, 2024
@zutigrm zutigrm assigned aaemnnosttv and unassigned zutigrm Apr 11, 2024
@aaemnnosttv aaemnnosttv assigned zutigrm and unassigned aaemnnosttv Apr 11, 2024
@zutigrm zutigrm assigned aaemnnosttv and unassigned zutigrm Apr 12, 2024
@aaemnnosttv aaemnnosttv removed their assignment Apr 15, 2024
@wpdarren wpdarren self-assigned this Apr 16, 2024
@wpdarren
Copy link
Collaborator Author

QA Update: ✅

Verified:

Noted the need to grant the additional edit permission to create a new property/datastream to reproduce the issue:

  • Went through OAuth and came back and the automatic creation afterward. See screencast.
  • The enhanced measurement setting keeps the value selected while creating a new property/web data stream.
  • On the setup module screen, the switch is enabled by default, even if on the selected property, you disabled enhanced measurement switch is enabled by default, as well as whenever you switch to different/new property or/and the web data stream switch is reset to on by default
  • On the Settings page, the switch is less pushy. If the enhanced measurement for a selected property is disabled, it shows as off by default or when switching between the account/property/web data stream dropdown.

I ran through some testing of enhanced measurement in its entirety to ensure everything worked as expected. Including testing the banner on the dashboard when the feature is disabled. Enabling and disabling the feature within settings

Note: I did notice a bug, but not a regression related to the banner on the dashboard. I will create a ticket for this.

em01.mp4
Screenshots

image
image
image
image

@wpdarren wpdarren removed their assignment Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Analytics Google Analytics module related issues P0 High priority Squad 1 (Team S) Issues for Squad 1 Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants