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

Create conversion-tracking.js Datastore #8615

Closed
3 tasks done
10upsimon opened this issue Apr 25, 2024 · 13 comments
Closed
3 tasks done

Create conversion-tracking.js Datastore #8615

10upsimon opened this issue Apr 25, 2024 · 13 comments
Labels
javascript Pull requests that update Javascript code P0 High priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@10upsimon
Copy link
Collaborator

10upsimon commented Apr 25, 2024

Feature Description

With Conversion Tracking settings and associated REST controller being defined and complete (see #8612, #8613 & #8614), the appropriate datastore should be defined within the main Site Kit React application. This datastore should define the necessary actions and selectors required to save and retrieve applicable Conversion Tracking settings.


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

Acceptance criteria

  • New datastore partial for conversion tracking settings should be added
  • It should handle: fetching, and updating/saving settings leveraging the REST endpoint added in Add REST_Conversion_Tracking_Controller Class #8613
  • When Ads or Analytics settings are saved, the conversion tracking setting is saved as well.

Implementation Brief

  • Add assets/js/googlesitekit/datastore/site/conversion-tracking.js
    • You can use assets/js/googlesitekit/datastore/site/consent-mode.js as a starting point
    • Create a fetch store for sourcing the settings data, say getConversionTrackingSettings, it should use core/site/data/conversion-tracking endpoint for that
    • Create a fetch store for saving settings, say saveConversionTrackingSettings, targeting the same endpoint
    • For the baseInitialState include conversionTrackingSettings.enabled (dot annotates nesting), and savedConversionTrackingSettings: undefined
    • Add saveConversionTrackingSettings action that will use the fetch store to save the settings
    • Add setConversionTrackingEnabled action, that will yield passed boolean value to the reducer. Reducer should update state.conversionTrackingSettings.enabled value
    • Add getConversionTrackingSettings resolver that will use getConversionTrackingSettings fetch store to source the settings data
    • Add selectors for getting the conversion tracking settings (state.conversionTrackingSettings) value from the state, and individual selector for retrieving only nested enabled value.
    • Add hasConversionTrackingSettingChanged selector that compares the difference between state.conversionTrackingSettings and state.savedConversionTrackingSettings objects, and returns true if no difference is found.
  • Include new datastore partial to the assets/js/googlesitekit/datastore/site/index.js main datastore
  • Update assets/js/modules/ads/datastore/settings.js and assets/js/modules/analytics-4/datastore/settings.js
    • In submitChanges function
      • After saveSettings is successfuly done (no errors), check if conversion tracking settings changed (hasConversionTrackingSettingChanged) to invoke saveConversionTrackingSettings action on CORE_SITE store to save conversion tracking setting

Test Coverage

  • Add tests for the datastore, testing the actions/resolvers/selectors

QA Brief

  • Setup Site Kit with conversionInfra feature flag
  • Go to dashboard or setting screen and paste this snippet: googlesitekit.data.select('core/site').getConversionTrackingSettings()
    • Firs it will return undefined, you can either check in network tab that response returns default settings value, or paste same snippet again in console which will output the settings
  • Paste googlesitekit.data.dispatch('core/site').setConversionTrackingEnabled(true) to change the enabled value, you can use true or false
    • You can verify the state change by using the first snippet, which will output the state
  • Paste googlesitekit.data.dispatch('core/site').saveConversionTrackingSettings()
    • Verify in the network tab that request is sent to the server with properly set enabled value
  • Refresh the page, use the first snippet again, verify that default value is now true
  • Paste googlesitekit.data.select('core/site').haveConversionTrackingSettingsChanged()
    • It should output false as no change happened yet
  • Use googlesitekit.data.dispatch('core/site').setConversionTrackingEnabled(false) to change the value, and use the snippet from previous step again, it should output true as settings were updated in the state
  • At any point you can also use googlesitekit.data.select('core/site').isConversionTrackingEnabled() to verify the current value in the state. When value is set to true it should output it as true, otherwise it should be false

Changelog entry

  • Implement the new conversion tracking partial datastore.
@10upsimon 10upsimon added javascript Pull requests that update Javascript code Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature labels Apr 25, 2024
@10upsimon 10upsimon changed the title Create conversion_tracking.js Datastore Create conversion-tracking.js Datastore Apr 25, 2024
@eclarke1 eclarke1 added the Next Up Issues to prioritize for definition label Apr 30, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm Apr 30, 2024
@10upsimon 10upsimon self-assigned this May 3, 2024
@10upsimon
Copy link
Collaborator Author

AC LGTM here, but noting that during IB we may want to consider a selector to check if conversion tracking is enable via a selector such as isConversionTrackingEnabled() or similar.

@10upsimon 10upsimon removed their assignment May 3, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm May 6, 2024
@mxbclang mxbclang added the P0 High priority label May 6, 2024
@eugene-manuilov eugene-manuilov self-assigned this May 7, 2024
@eugene-manuilov
Copy link
Collaborator

Thanks, @zutigrm. IB technically looks good to me, but I think we need to add one more thing to it. We need to make sure that when Ads or Analytics settings are saved, the conversion events tracking setting is saved as well. In order to do that, we need to call the save action from within the appropriate submit changes functions for both modules when main settings for the module are saved.

@zutigrm
Copy link
Collaborator

zutigrm commented May 7, 2024

@eugene-manuilov thanks for the comment. Basically we won't need to worry about it here, or in the settings forms, the saving should be handled in the the toggle itlself #8616, like what a consent mode switch is doing

@eugene-manuilov
Copy link
Collaborator

  • Before or after saveSettings is called, invoke saveConversionTrackingSettings action on CORE_SITE store to save conversion tracking setting

@zutigrm this should happen after settings have been successfully saved and only if the conversion tracking setting has changed.

@zutigrm zutigrm assigned eugene-manuilov and unassigned zutigrm May 9, 2024
@eugene-manuilov
Copy link
Collaborator

Thanks, almost looks good. The last thing let's rename canSubmitConversionTrackingSettings to hasConversionTrackingSettingChanged since that is the more relevant name for that selector.

@zutigrm
Copy link
Collaborator

zutigrm commented May 13, 2024

@eugene-manuilov IB updated. Thanks

@eugene-manuilov
Copy link
Collaborator

Thanks, IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment May 13, 2024
@zutigrm zutigrm self-assigned this May 13, 2024
@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label May 13, 2024
@zutigrm zutigrm removed their assignment May 15, 2024
@zutigrm zutigrm assigned eugene-manuilov and unassigned zutigrm May 16, 2024
@eugene-manuilov eugene-manuilov removed their assignment May 16, 2024
@mohitwp mohitwp self-assigned this May 20, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented May 22, 2024

QA Update ⚠️

@zutigrm As per AC - When Ads or Analytics settings are saved, the conversion tracking setting is saved as well. But, I've noticed that if analytics or ads module both are not connected then also I'm getting the snippet response and able to save conversion tracking settings. Is this expected ?

PASS CASES

  • Tested on dev environment.
  • Verified on both dashboard and settings page.
  • Verified when 'conversionInfra' feature flag is enabled.
  • Verified when 'conversionInfra' feature flag is not enabled.
  • Verified when Ads or Analytics module are not connected.
  1. Go to dashboard or setting screen and paste this snippet: googlesitekit.data.select('core/site').getConversionTrackingSettings()

image

  1. Paste googlesitekit.data.dispatch('core/site').setConversionTrackingEnabled(true) to change the enabled value, you can use true or false

image

True State:

image

False State:

image

  1. Paste googlesitekit.data.dispatch('core/site').saveConversionTrackingSettings()
    Verify in the network tab that request is sent to the server with properly set enabled value

image

image

  1. aste googlesitekit.data.dispatch('core/site').saveConversionTrackingSettings()
    Verify in the network tab that request is sent to the server with properly set enabled value
Recording.982.mp4
  1. Paste googlesitekit.data.select('core/site').haveConversionTrackingSettingsChanged()
    It should output false as no change happened yet

image

  1. Use googlesitekit.data.dispatch('core/site').setConversionTrackingEnabled(false) to change the value, and use the snippet from previous step again, it should output true as settings were updated in the state

image

image

  1. Current value state-

image

When 'ConversionInfra' feature flaf is not enabled

image

When both 'Analytics' and Ads module not connected

image

@zutigrm
Copy link
Collaborator

zutigrm commented May 22, 2024

@mohitwp Thanks for the observations. Can you provide me more details, on how are you saving the settings/getting the snippet response? The implementation is tied to the Analytics and Ads module, so saving the conversion tracking is tied to only get triggered from the settings on these modules.

@mohitwp
Copy link
Collaborator

mohitwp commented May 22, 2024

@zutigrm

  • I created a new site on dev environment.
  • Setup Site Kit without Analytics.
  • Also, not setup Ads module.
  • On dashboard or settings page I pasted the snippet as per QAB.
  • Changed the state from 'False' to 'True' and it's getting save.
  • I'm getting response as per QAB without setting up Analytics or Ads module.

Note : It's a fresh site on which site kit was never setup.

image

image

image

@zutigrm
Copy link
Collaborator

zutigrm commented May 22, 2024

@mohitwp That's fine since you are triggering it manually. In real usage the setting is saved only once button to save settings changes is hit on the settings screen of those 2 modules. Conversion event is independent and shared between these two modules, so manually invoking settings will still work independently of whether the modules are active or not

@mohitwp
Copy link
Collaborator

mohitwp commented May 22, 2024

QA Update ✅

Thanks @zutigrm !

  • Tested on dev environment.
  • Verified on both dashboard and settings page.
  • Verified when 'conversionInfra' feature flag is enabled.
  • Verified when 'conversionInfra' feature flag is not enabled.
  • Verified when Ads or Analytics module are not connected.
  1. Go to dashboard or setting screen and paste this snippet: googlesitekit.data.select('core/site').getConversionTrackingSettings()

image

  1. Paste googlesitekit.data.dispatch('core/site').setConversionTrackingEnabled(true) to change the enabled value, you can use true or false

image

True State:

image

False State:

image

  1. Paste googlesitekit.data.dispatch('core/site').saveConversionTrackingSettings()
    Verify in the network tab that request is sent to the server with properly set enabled value

image

image

  1. aste googlesitekit.data.dispatch('core/site').saveConversionTrackingSettings()
    Verify in the network tab that request is sent to the server with properly set enabled value
Recording.982.mp4
  1. Paste googlesitekit.data.select('core/site').haveConversionTrackingSettingsChanged()
    It should output false as no change happened yet

image

  1. Use googlesitekit.data.dispatch('core/site').setConversionTrackingEnabled(false) to change the value, and use the snippet from previous step again, it should output true as settings were updated in the state

image

image

  1. Current value state-

image

When 'ConversionInfra' feature flaf is not enabled

image

When both 'Analytics' and Ads module not connected

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code P0 High priority Team S Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants