site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Add new resolver to the conversion reporting datastore partial to retrieve detected and lost events from inline module data

Open zutigrm opened this issue 1 year ago • 2 comments

Feature Description

Conversion reporting datastore partial should be updated to include new resolver which would retrieve detected and lost events from inline module data (implemented in #9342), and pass them to an action which should receive passed data and store them to the state.

See Implementation - Datastore Partial section of the design doc


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

Acceptance criteria

  • New resolver getConversionReportingEventsChange is added to the conversion-reporting datastore partial
    • It retrieves inline data implemented in #9342
    • It passes the new and lost events as an object to the receiveConversionReportingInlineData action
  • New action receiveConversionReportingInlineData is implemented, which yields passed object to the reducer
    • Reducer should store this data in the state under detectedEventsChange property
  • New selectors hasNewConversionReportingEvents and hasLostConversionReportingEvents are added
    • They should extract their respective events from detectedEventsChange state property by using getConversionReportingEventsChange selector

Implementation Brief

  • [ ] Update assets/js/modules/analytics-4/datastore/conversion-reporting.js
    • Add the initial state object, containing detectedEventsChange, undefined by default
    • Add getConversionReportingEventsChange resolver
    • Retrieve the values from newEvents and lostEvents from Analytics inline module data like global._googlesitekitModulesData?.[ 'analytics-4' ]?.newEvents, etc.
    • Group them under one object and pass it to the receiveConversionReportingInlineData action
    • You can check an example in assets/js/googlesitekit/datastore/site/info.js, or any other datastore partial resolving inline data.
    • Add receiveConversionReportingInlineData action
      • It should extract newEvents and lostEvents from the passed argument, and yield them as an object to the reducer. Then in the reducer, update detectedEventsChange object in the state to hold newEvents and lostEvents as properties
    • Add getConversionReportingEventsChange selector
      • It should return value from the state state.detectedEventsChange
    • Include hasNewConversionReportingEvents and hasLostConversionReportingEvents selectors
      • They should use getConversionReportingEventsChange selector to retrieve the inline data (or resolve it if not yet available), and then extract their respective properties from the returned value. You can add a helper function to extract the correct property, like for example, how it is used in info datastore https://github.com/google/site-kit-wp/blob/d0f81d38c89d89c1b0d39e861a56ca67d40c238f/assets/js/googlesitekit/datastore/site/info.js#L38-L43 and then invoke it here in each selector passing either newEvents or lostEvents for the argument

Test Coverage

  • Update assets/js/modules/analytics-4/datastore/conversion-reporting.test.js to include test coverage for new resolver, action and selectors

QA Brief

  • Setup Site Kit with Analytics module and conversionReporting feature flag enabled
  • Paste this snippet in the console await googlesitekit.data.select('modules/analytics-4').getConversionReportingEventsChange()
    • Initially it will output undefined
    • On the second attempt it will resolve the data and output {newEvents: Array(0), lostEvents: Array(0)}
    • Note: when feature flag is disabled, this selector will still be accessible but will return undefined for both newEvents and lostEvents
  • Paste this snippet in the console await googlesitekit.data.select('modules/analytics-4').hasNewConversionReportingEvents()
    • Same as above, firs call will output undefined, afterwards should show empty array []
    • Also same applies regarding the feature flag, when disabled selector will still work, but always return undefined value
  • Paste the snippet await googlesitekit.data.select('modules/analytics-4').hasLostConversionReportingEvents()
    • It should behave same as await googlesitekit.data.select('modules/analytics-4').hasNewConversionReportingEvents() above
  • Paste this snippet: await googlesitekit.data.dispatch('modules/analytics-4').receiveConversionReportingInlineData({newEvents: ['contact'], lostEvents: ['purchase']})
    • Then afterwards, without reloading, test the previous three snippets:
      • await googlesitekit.data.select('modules/analytics-4').getConversionReportingEventsChange() should return whole object passed above: {lostEvents: ['purchase'], newEvents: ['contact']}
      • await googlesitekit.data.select('modules/analytics-4').hasNewConversionReportingEvents() should return only newEvents value, in this case ['contact']
      • await googlesitekit.data.select('modules/analytics-4').hasLostConversionReportingEvents() should return only lostEvents value, in this case: ['purchase']

Changelog entry

zutigrm avatar Sep 19 '24 16:09 zutigrm

AC ✔️

eugene-manuilov avatar Sep 20 '24 14:09 eugene-manuilov

@zutigrm IB ✅ , moving to EB.

10upsimon avatar Oct 01 '24 14:10 10upsimon

Assigning this to myself to get this moved through and unblock other issues 🙂

tofumatt avatar Nov 04 '24 13:11 tofumatt

QA Update ⚠

Going through the QAB, I have a couple of things to highlight:

ITEM 1:

  • Pasted the snippet await googlesitekit.data.select('modules/analytics-4').hasNewConversionReportingEvents()

    First call outputs undefined, afterwards we are expecting it to show empty array [] However, instead of showing empty array, it's returning as 'False' ❌

    Image

    When feature flag is off, it always returned undefined value, this is expected. Image


ITEM 2:

  • Pasted the snippet await googlesitekit.data.select('modules/analytics-4').hasLostConversionReportingEvents()

    We are expecting it to behave the same as It should behave same as .hasNewConversionReportingEvents() above. Indeed, it behaves the same but given that it returns false instead of empty array, this needs to be double-checked. ⚠

    Image

    When feature flag is off, it always returned undefined value. This is expected. Image


ITEM 3:

  • Pasted the snippet: await googlesitekit.data.dispatch('modules/analytics-4').receiveConversionReportingInlineData({newEvents: ['contact'], lostEvents: ['purchase']}) Then afterwards, without reloading, I tested the 3 snippets:
    • getConversionReportingEventsChange() - returns whole object passed above: {lostEvents: ['purchase'], newEvents: ['contact']} ✅

    • hasNewConversionReportingEvents() - we are expecting it to return only newEvents value, in this case ['contact'] but it's returning as 'true' ❌

    • hasLostConversionReportingEvents()- we are expecting it to return only lostEvents value, in this case: ['purchase'] but it's returning as 'true' ❌

      Image


Other than that .getConversionReportingEventsChange() worked well:

  • Pasted the snippet await googlesitekit.data.select('modules/analytics-4').getConversionReportingEventsChange() Initially, it outputs undefined ✅ On the second attempt, it will resolve the data and outputs {newEvents: Array(0), lostEvents: Array(0)}

    Image

    When feature flag is disabled, returns undefined for both newEvents and lostEvents

    Image

kelvinballoo avatar Nov 06 '24 17:11 kelvinballoo

@kelvinballoo Thanks for you observations. Sorry about that, I update QAB now, we changed in the last CR round, what you see is correct output, hasNewConversionReportingEvents and hasLostConversionReportingEvents return true, or false, or undefined, they will not return arrays any more

zutigrm avatar Nov 07 '24 07:11 zutigrm

QA Update ✅

Thanks @zutigrm , going through the new QAB, everything is checking out. Moving ticket to Approval.

  • getConversionReportingEventsChange() worked well:

    • Pasted the snippet await googlesitekit.data.select('modules/analytics-4').getConversionReportingEventsChange() Initially, it outputs undefined ✅ On the second attempt, it will resolve the data and outputs {newEvents: Array(0), lostEvents: Array(0)} ✅

      Image

      When feature flag is disabled, returns undefined for both newEvents and lostEvents

      Image


  • hasNewConversionReportingEvents() worked well:
    • Pasted the snippet await googlesitekit.data.select('modules/analytics-4').hasNewConversionReportingEvents()

      First call outputs undefined, afterwards it returns 'False' ✅ When feature flag is off, it always returned undefined value, this is expected. ✅

      Image

      When feature flag is off, it always returned undefined value, this is expected. Image


  • hasLostConversionReportingEvents() worked well:
    • Pasted the snippet await googlesitekit.data.select('modules/analytics-4').hasLostConversionReportingEvents()

      We are expecting it to behave the same as .hasNewConversionReportingEvents() above. Indeed, it behaves the same. First call is undefined, followed by 'False' ✅ When feature flag is off, it always returned undefined value. ✅

      Image

      When feature flag is off, it always returned undefined value. This is expected. Image


  • .receiveConversionReportingInlineData worked well:
    • Pasted the snippet: await googlesitekit.data.dispatch('modules/analytics-4').receiveConversionReportingInlineData({newEvents: ['contact'], lostEvents: ['purchase']}) Then afterwards, without reloading, I tested the 3 snippets:

      • getConversionReportingEventsChange() - returns whole object passed above: {lostEvents: ['purchase'], newEvents: ['contact']} ✅
      • hasNewConversionReportingEvents() - returns as true ✅
      • hasLostConversionReportingEvents()- returns as true ✅

      Image

kelvinballoo avatar Nov 07 '24 10:11 kelvinballoo