feat(store_pagespixel): add legacy analytics support #28

tatimblin · 2022-07-15T12:51:57Z

J=CR-2409

tatimblin · 2022-07-15T12:58:42Z

This adds two new AnalyticsEvent types 'PAGE_VIEW' and 'CLICK_EVENT', which get directed to an alternate fetch request. Here is an example usage

      <a
        href="https://yext.com"
        target="_blank"
        onClick={() => analytics.report({
          type: 'CLICK_EVENT',
          product: 'storepages',
          v: 1654287242049,
          pageurl: '%2Fmd%2Feldersburg%2Fbrenda-given',
          pagesReferrer: 'https%3A%2F%2Fbankers_amerisbank_com.yextpages.net%2Fmd%2Feldersburg',
          businessids: 3371543,
          siteId: 31196,
          isStaging: true,
          ids: 34233156,
          pageSetId: 'Locations',
          eventType: 'yext_sites_test',
        })}
      >
        Yext analytics
      </a>

Currently you have to pass everything into the event, but some of the stuff can be done automatically.

'product' will always be 'storepages' so it can be hardcoded
pageurl and pagesReferrer could be determined from window
businessids is available in the config
isStaging, pageSetId, and eventType could probably be derived from the application

Additionally the config would need to be updated to take 1 of 2 config types, currently some answers values are required.

  const analytics = provideAnalytics({
    experienceKey: 'yext-inventory',
    experienceVersion: 'PRODUCTION',
    businessId: 2987593,
  });

Additionally wanted to share our thoughts on the timeline of this. Once pages analytics are updated to the same system as answers, we'd like users to interact with yext analytics the same way regardless of product. This update aims to stand in and let us construct requests as if it that new system was ready so that when it is, reportLegacy can be removed and existing sites just need to upgrade this package in each repo to switch over.

benmcginnis · 2022-07-22T21:05:09Z

tests/infra/PagesAnalyticsReporter.ts

+  const reporter = new PagesAnalyticsReporter(config, mockHttpRequesterService);
+  reporter.pageView();
+  const expectedUrl = '';
+  expect(mockHttpRequesterService.get).toHaveBeenLastCalledWith(expectedUrl, expect.anything());


I'm getting a weird failure trying to run this:

FAIL tests/infra/PagesAnalyticsReporter.ts ● Test suite failed to run TypeError: Cannot read properties of undefined (reading 'generatedLine') at compareByGeneratedPositionsDeflated (node_modules/source-map-support/node_modules/source-map/lib/util.js:344:47) at doQuickSort (node_modules/source-map-support/node_modules/source-map/lib/quick-sort.js:88:11) at Object.<anonymous>.exports.quickSort (node_modules/source-map-support/node_modules/source-map/lib/quick-sort.js:113:3) at BasicSourceMapConsumer.SourceMapConsumer_parseMappings [as _parseMappings] (node_modules/source-map-support/node_modules/source-map/lib/source-map-consumer.js:564:5) at BasicSourceMapConsumer.get [as _generatedMappings] (node_modules/source-map-support/node_modules/source-map/lib/source-map-consumer.js:70:12) at BasicSourceMapConsumer.SourceMapConsumer_originalPositionFor [as originalPositionFor] (node_modules/source-map-support/node_modules/source-map/lib/source-map-consumer.js:655:12) at mapSourcePosition (node_modules/source-map-support/source-map-support.js:244:42) at wrapCallSite (node_modules/source-map-support/source-map-support.js:397:20) at Function.prepareStackTrace (node_modules/source-map-support/source-map-support.js:446:39)

bhainesva · 2022-07-26T15:35:46Z

README.md

+  businessId: 123456, // this comes from the url of your Yext account
+  featureId: 'My Locator', // the name of the feature in your features.json
+  pageType: 'locator', // the type of page, can be 'entity', 'directory', 'locator', or 'static'
+  product: 'storepages',


What should product be? If it's always storepages, is there a reason they need to specify it?

bhainesva · 2022-07-26T15:44:45Z

README.md

+  featureId: 'My Locator', // the name of the feature in your features.json
+  pageType: 'locator', // the type of page, can be 'entity', 'directory', 'locator', or 'static'
+  product: 'storepages',
+  production: false, // set to true if you are in the production environment


Production is a pretty overloaded term for us. Is this production as in the production vs staging deploy? Or production vs. sandbox? Could also see someone confusing this with the Production answers experience tag.

fair point... it's just that staging doesn't exist anymore so I figured it made sense to flip the bit

bhainesva · 2022-07-26T15:45:55Z

README.md


 ```ts
 import { provideAnalytics } from '@yext/analytics';

 const analytics = provideAnalytics({
  experienceKey: '<your experience key>',
  experienceVersion: 'PRODUCTION',
-  businessId: 123456,
+  businessId: 123456, // this comes from the url of your Yext account
+  featureId: 'My Locator', // the name of the feature in your features.json


If you're using the react code features.json is autogenerated and you may never look at it, might need to specify that the name in that case is taken from the file name of your entry template

bhainesva · 2022-07-26T15:48:22Z

README.md

+  product: 'storepages',
+  production: false, // set to true if you are in the production environment
+  siteId: 654321, // the id of your site, you can find this in the url of your deploy page
+  ids: [90210], // if your pageType is 'entity' this is required, and it is the uid of the entity 


Is this required on directory pages (or if it's optional, is there a benefit to providing it?)

in sites, it's necessary to identify the directory entity, since directory pages are entity pages

bhainesva · 2022-07-26T15:54:58Z

etc/analytics.api.md

@@ -129,6 +119,61 @@ export interface CtaEvent {
 // @public
 export type EnumOrString<T extends string> = T | `${T}`;

+// Warning: (ae-forgotten-export) The symbol "BaseAnalyticsConfig" needs to be exported by the entry point index.d.ts


Is this warning something we need to do something about?

i think i've fixed this

src/infra/HttpRequester.ts

bhainesva · 2022-07-26T19:03:08Z

src/models/PagesEventDetails.ts

+
+  constructor(config: PagesAnalyticsConfig, event: PagesEvent) {
+    this.eventType = event.eventType;
+    this.product = 'storepages';


Is this intentionally hardcoded instead of reading from the config?

removed this from the config as it is only ever "storepages" in the context of this library

bhainesva · 2022-07-26T19:03:55Z

src/models/PagesEventDetails.ts

+    this.ids = config.ids;
+    this.pageType = config.pageType;
+
+    if (this.pageType === 'entity' && !this.ids) {


If we want I believe this is enforceable at the type level. Do we also want to check the opposite, that entityIds aren't provided for non 'entity' page types?

i'm not 100% certain. I believe they are optional, for example if you click on a result on a locator i think we can pass the entity id. I haven't had a chance to check

ok i've confirmed that if you set the id currently it treats it as an entity page

bhainesva · 2022-07-26T19:04:17Z

src/models/PagesEventDetails.ts

+
+    this.featureId = config.featureId;
+    this.pagesReferrer = config.pagesReferrer ? config.pagesReferrer : window.document.referrer;
+    this.path = config.path ? config.path : window.location.pathname;


Same question about references to window

bhainesva · 2022-07-26T19:08:01Z

src/services/PagesAnalyticsService.ts

+
+  /**
+   * Reports a user interaction event.
+   * Will perform a promis rejection if the API contains an error or if


Suggested change

* Will perform a promis rejection if the API contains an error or if

* Will perform a promise rejection if the API contains an error or if

tmeyer2115 · 2022-07-28T16:50:52Z

src/infra/HttpRequester.ts

 import fetch from 'cross-fetch';

 /**
 * Responsible for making web requests.
+ *
+ * @public


Does this need to be annotated as public? Ideally it should just be an internal implementation detail.

So i did this because there was a warning in the auto-generated markdown file bc this is part of the method signature of the AnalyticsReporters

the warning is:

// Warning: (ae-forgotten-export) The symbol "HttpRequesterService" needs to be exported by the entry point index.d.ts

Why do the various reporters need to be exported? We treated the AnalyticsReporter as an implementation detail, IIRC. We did not export it, users could not instantiate it manually. They had to use the provide method.

let me look back at master, i thought I was following what it was doing but I may have over-exported things

tmeyer2115 · 2022-07-28T16:53:16Z

src/services/HttpRequesterService.ts

 /**
 * A service for sending requests on the web.
+ *
+ * @public


Same thing about this being public.

same thing about the method signatures

tmeyer2115 · 2022-07-28T16:54:01Z

src/models/search/SearchAnalyticsEventType.ts

@@ -3,7 +3,7 @@
 *
 * @public
 */
-export enum AnalyticsEventType {
+export enum SearchAnalyticsEventType {


If AnalyticsEventType was exported by the library, we will need an alias.

feat(store_pagespixel): add legacy analytics support

3720e93

J=CR-2409

benmcginnis and others added 3 commits July 22, 2022 16:59

wip, having trouble with jest locally

720ffd5

Automated update to repo's documentation from github action

ef25a23

add tests but they are not working

5152ee8

benmcginnis reviewed Jul 22, 2022

View reviewed changes

benmcginnis and others added 8 commits July 25, 2022 13:21

test working

e462922

implement remaining test permutations for pages analytics reporter

52da3a9

update analyticsProvider to be more of a factory

131c0b4

refactor to create separate providers for the three types of analytics

cd0de3d

update readme

0e9db59

update readme some more

00ddf38

fix jsdoc errors and add some todos

231f6b3

Automated update to repo's documentation from github action

d1d1f15

bhainesva reviewed Jul 26, 2022

View reviewed changes

benmcginnis and others added 4 commits July 28, 2022 10:56

complete refactor based on spec feedback

4cff913

Automated update to repo's documentation from github action

fcbe05c

update readme and add pageType param after conversation with peralta

8281ae5

Automated update to repo's documentation from github action

6c0f271

tmeyer2115 reviewed Jul 28, 2022

View reviewed changes

benmcginnis and others added 5 commits July 28, 2022 13:15

address tom's comments

f5a355c

Automated update to repo's documentation from github action

3ca3bb2

fix final tom comment and link in readme

4c433ca

Automated update to repo's documentation from github action

fe36c0d

fix imports

b04b0e7

tmeyer2115 self-requested a review July 28, 2022 18:37

tmeyer2115 approved these changes Jul 28, 2022

View reviewed changes

benmcginnis and others added 3 commits July 28, 2022 16:15

make debug optional so as to not change the api

b160906

update test site and fix issues found during manual testing

8dd1ecf

Automated update to repo's documentation from github action

5c4d583

make debugging work on search events

03a6734

benmcginnis merged commit 90e1daa into pages/trunk Jul 28, 2022

benmcginnis deleted the pages/doit branch July 28, 2022 20:45

benmcginnis mentioned this pull request Aug 9, 2022

Add Support for Pages Analytics #30

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(store_pagespixel): add legacy analytics support #28

feat(store_pagespixel): add legacy analytics support #28

tatimblin commented Jul 15, 2022

tatimblin commented Jul 15, 2022 •

edited

benmcginnis Jul 22, 2022

bhainesva Jul 26, 2022

bhainesva Jul 26, 2022

benmcginnis Jul 27, 2022

bhainesva Jul 26, 2022

benmcginnis Jul 27, 2022

bhainesva Jul 26, 2022

benmcginnis Jul 27, 2022

bhainesva Jul 26, 2022

benmcginnis Jul 27, 2022

bhainesva Jul 26, 2022

benmcginnis Jul 27, 2022

bhainesva Jul 26, 2022

benmcginnis Jul 26, 2022

benmcginnis Jul 27, 2022

bhainesva Jul 26, 2022

bhainesva Jul 26, 2022

tmeyer2115 Jul 28, 2022

benmcginnis Jul 28, 2022

benmcginnis Jul 28, 2022

tmeyer2115 Jul 28, 2022

benmcginnis Jul 28, 2022

tmeyer2115 Jul 28, 2022

benmcginnis Jul 28, 2022

tmeyer2115 Jul 28, 2022

	* Will perform a promis rejection if the API contains an error or if
	* Will perform a promise rejection if the API contains an error or if

feat(store_pagespixel): add legacy analytics support #28

feat(store_pagespixel): add legacy analytics support #28

Conversation

tatimblin commented Jul 15, 2022

tatimblin commented Jul 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tatimblin commented Jul 15, 2022 •

edited