Fix identify and group signals calls (#1115)

Author: silesky

.changeset/breezy-knives-arrive.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-next': patch
+---
+
+Update argument resolver to user interface rather than full User

.changeset/thing-chicken-sun.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-signals': patch
+---
+
+Fix group and identify calls.

packages/browser/src/core/arguments-resolver/index.ts

@@ -15,7 +15,7 @@ import {
   GroupTraits,
   UserTraits,
 } from '../events'
-import { ID, User } from '../user'
+import { ID, WithId } from '../user'
 
 /**
  * Helper for the track method
@@ -107,7 +107,7 @@ export function resolvePageArguments(
 /**
  * Helper for group, identify methods
  */
-export const resolveUserArguments = <T extends Traits, U extends User>(
+export const resolveUserArguments = <T extends Traits, U extends WithId>(
   user: U
 ): ResolveUser<T> => {
   return (...args): ReturnType<ResolveUser<T>> => {

packages/browser/src/core/user/index.ts

@@ -50,7 +50,11 @@ const defaults = {
   },
 }
 
-export class User {
+export interface WithId {
+  id(id?: ID): ID
+}
+
+export class User implements WithId {
   static defaults = defaults
 
   private idKey: string

packages/signals/signals-integration-tests/package.json

@@ -15,7 +15,7 @@
     "watch:test": "yarn test --watch",
     "tsc": "yarn run -T tsc",
     "eslint": "yarn run -T eslint",
-    "server": "http-server --port 3000",
+    "server": "http-server --port 5432",
     "browser": "playwright test --debug"
   },
   "packageManager": "yarn@3.4.1",

packages/signals/signals-integration-tests/playwright.config.ts

@@ -8,7 +8,7 @@ import path from 'path'
 const config: PlaywrightTestConfig = {
   webServer: {
     command: 'yarn run server',
-    url: 'http://127.0.0.1:3000',
+    url: 'http://127.0.0.1:5432',
     reuseExistingServer: !process.env.CI,
   },
   testDir: './src',

packages/signals/signals-integration-tests/src/helpers/base-page-object.ts

@@ -1,24 +1,32 @@
 import { CDNSettingsBuilder } from '@internal/test-helpers'
 import { Page, Request } from '@playwright/test'
+import { logConsole } from './log-console'
+import { SegmentEvent } from '@segment/analytics-next'
 
 export class BasePage {
   protected page!: Page
-  public signalsApiReq!: Request
-  public trackingApiReq!: Request
+  public lastSignalsApiReq!: Request
+  public signalsApiReqs: SegmentEvent[] = []
+  public lastTrackingApiReq!: Request
+  public trackingApiReqs: SegmentEvent[] = []
+
   public url: string
   public edgeFnDownloadURL = 'https://cdn.edgefn.segment.com/MY-WRITEKEY/foo.js'
-  public edgeFn: string
+  public edgeFn!: string
 
-  constructor(path: string, edgeFn: string) {
-    this.edgeFn = edgeFn
-    this.url = 'http://localhost:3000/src/tests' + path
+  constructor(path: string) {
+    this.url = 'http://localhost:5432/src/tests' + path
   }
 
   /**
    * load and setup routes
+   * @param page
+   * @param edgeFn - edge function to be loaded
    */
-  async load(page: Page) {
+  async load(page: Page, edgeFn: string) {
+    logConsole(page)
     this.page = page
+    this.edgeFn = edgeFn
     await this.setupMockedRoutes()
     await this.page.goto(this.url)
     // expect analytics to be loaded
@@ -30,8 +38,10 @@ export class BasePage {
 
   private async setupMockedRoutes() {
     // clear any existing saved requests
-    this.signalsApiReq = undefined as any as Request
-    this.trackingApiReq = undefined as any as Request
+    this.signalsApiReqs = []
+    this.trackingApiReqs = []
+    this.lastSignalsApiReq = undefined as any as Request
+    this.lastTrackingApiReq = undefined as any as Request
 
     await Promise.all([
       this.mockSignalsApi(),
@@ -42,7 +52,8 @@ export class BasePage {
 
   async mockTrackingApi() {
     await this.page.route('https://api.segment.io/v1/*', (route, request) => {
-      this.trackingApiReq = request
+      this.lastTrackingApiReq = request
+      this.trackingApiReqs.push(request.postDataJSON())
       if (request.method().toLowerCase() !== 'post') {
         throw new Error(`Unexpected method: ${request.method()}`)
       }
@@ -56,15 +67,16 @@ export class BasePage {
     })
   }
 
-  waitForTrackingApiFlush() {
-    return this.page.waitForResponse('https://api.segment.io/v1/*')
+  waitForTrackingApiFlush(timeout = 5000) {
+    return this.page.waitForResponse('https://api.segment.io/v1/*', { timeout })
   }
 
   async mockSignalsApi() {
     await this.page.route(
       'https://signals.segment.io/v1/*',
       (route, request) => {
-        this.signalsApiReq = request
+        this.lastSignalsApiReq = request
+        this.signalsApiReqs.push(request.postDataJSON())
         if (request.method().toLowerCase() !== 'post') {
           throw new Error(`Unexpected method: ${request.method()}`)
         }
@@ -79,8 +91,10 @@ export class BasePage {
     )
   }
 
-  waitForSignalsApiFlush() {
-    return this.page.waitForResponse('https://signals.segment.io/v1/*')
+  waitForSignalsApiFlush(timeout = 5000) {
+    return this.page.waitForResponse('https://signals.segment.io/v1/*', {
+      timeout,
+    })
   }
 
   async mockCDNSettings() {

packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts

@@ -0,0 +1,58 @@
+import { test, expect } from '@playwright/test'
+import { IndexPage } from './index-page'
+import fs from 'fs'
+import path from 'path'
+import { SegmentEvent } from '@segment/analytics-next'
+/**
+ * This test ensures that
+ */
+const indexPage = new IndexPage()
+
+const normalizeSnapshotEvent = (el: SegmentEvent) => {
+  return {
+    type: el.properties?.type,
+    event: el.event,
+    userId: el.userId,
+    groupId: el.groupId,
+    anonymousId: expect.any(String),
+    integrations: el.integrations,
+    properties: el.properties,
+    context: {
+      page: el.context?.page,
+    },
+  }
+}
+
+const snapshot = (
+  JSON.parse(
+    fs.readFileSync(
+      path.join(__dirname, 'snapshots/all-segment-events-snapshot.json'),
+      {
+        encoding: 'utf-8',
+      }
+    )
+  ) as SegmentEvent[]
+).map(normalizeSnapshotEvent)
+
+test('Segment events', async ({ page }) => {
+  const basicEdgeFn = `
+    // this is a process signal function
+    const processSignal = (signal) => {
+      analytics.identify('john', { found: true })
+      analytics.group('foo', { hello: 'world' })
+      analytics.alias('john', 'johnsmith')
+      analytics.track('a track call',  {foo: 'bar'})
+      analytics.page('Retail Page', 'Home', { url: 'http://my-home.com', title: 'Some Title' });
+  }`
+
+  await indexPage.load(page, basicEdgeFn)
+  await Promise.all([
+    indexPage.clickButton(),
+    indexPage.waitForSignalsApiFlush(),
+    indexPage.waitForTrackingApiFlush(),
+  ])
+
+  const trackingApiReqs = indexPage.trackingApiReqs.map(normalizeSnapshotEvent)
+
+  expect(trackingApiReqs).toEqual(snapshot)
+})

packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts

@@ -4,8 +4,17 @@ import { IndexPage } from './index-page'
 
 const indexPage = new IndexPage()
 
+const basicEdgeFn = `
+    // this is a process signal function
+    const processSignal = (signal) => {
+      if (signal.type === 'interaction') {
+        const eventName = signal.data.eventType + ' ' + '[' + signal.type + ']'
+        analytics.track(eventName, signal.data)
+      }
+  }`
+
 test.beforeEach(async ({ page }) => {
-  await indexPage.load(page)
+  await indexPage.load(page, basicEdgeFn)
 })
 
 test('network signals', async () => {
@@ -15,7 +24,8 @@ test('network signals', async () => {
   await indexPage.mockRandomJSONApi()
   await indexPage.makeFetchCallToRandomJSONApi()
   await indexPage.waitForSignalsApiFlush()
-  const batch = indexPage.signalsApiReq.postDataJSON().batch as SegmentEvent[]
+  const batch = indexPage.lastSignalsApiReq.postDataJSON()
+    .batch as SegmentEvent[]
   const networkEvents = batch.filter(
     (el: SegmentEvent) => el.properties!.type === 'network'
   )
@@ -41,7 +51,7 @@ test('instrumentation signals', async () => {
     indexPage.waitForSignalsApiFlush(),
   ])
 
-  const signalReqJSON = indexPage.signalsApiReq.postDataJSON()
+  const signalReqJSON = indexPage.lastSignalsApiReq.postDataJSON()
 
   const isoDateRegEx = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/
   const instrumentationEvents = signalReqJSON.batch.filter(
@@ -59,19 +69,19 @@ test('instrumentation signals', async () => {
   })
 })
 
-test('interaction signals', async ({ page }) => {
+test('interaction signals', async () => {
   /**
    * Make a button click, see if it:
    * - creates an interaction signal that sends to the signals endpoint
    * - creates an analytics event that sends to the tracking endpoint
    */
   await Promise.all([
-    page.click('button'),
+    indexPage.clickButton(),
     indexPage.waitForSignalsApiFlush(),
     indexPage.waitForTrackingApiFlush(),
   ])
 
-  const signalsReqJSON = indexPage.signalsApiReq.postDataJSON()
+  const signalsReqJSON = indexPage.lastSignalsApiReq.postDataJSON()
   const interactionSignals = signalsReqJSON.batch.filter(
     (el: SegmentEvent) => el.properties!.type === 'interaction'
   )
@@ -104,7 +114,7 @@ test('interaction signals', async ({ page }) => {
     },
   })
 
-  const analyticsReqJSON = indexPage.trackingApiReq.postDataJSON()
+  const analyticsReqJSON = indexPage.lastTrackingApiReq.postDataJSON()
 
   expect(analyticsReqJSON).toMatchObject({
     writeKey: '<SOME_WRITE_KEY>',
@@ -128,7 +138,7 @@ test('navigation signals', async ({ page }) => {
   {
     // on page load, a navigation signal should be sent
     await indexPage.waitForSignalsApiFlush()
-    const signalReqJSON = indexPage.signalsApiReq.postDataJSON()
+    const signalReqJSON = indexPage.lastSignalsApiReq.postDataJSON()
     const navigationEvents = signalReqJSON.batch.filter(
       (el: SegmentEvent) => el.properties!.type === 'navigation'
     )
@@ -153,7 +163,7 @@ test('navigation signals', async ({ page }) => {
       window.location.hash = '#foo'
     })
     await indexPage.waitForSignalsApiFlush()
-    const signalReqJSON = indexPage.signalsApiReq.postDataJSON()
+    const signalReqJSON = indexPage.lastSignalsApiReq.postDataJSON()
 
     const navigationEvents = signalReqJSON.batch.filter(
       (el: SegmentEvent) => el.properties!.type === 'navigation'

packages/signals/signals-integration-tests/src/tests/signals-vanilla/index-page.ts

@@ -1,18 +1,9 @@
 import { BasePage } from '../../helpers/base-page-object'
 import { promiseTimeout } from '@internal/test-helpers'
 
-const edgeFn = `
-    // this is a process signal function
-    const processSignal = (signal) => {
-      if (signal.type === 'interaction') {
-        const eventName = signal.data.eventType + ' ' + '[' + signal.type + ']'
-        analytics.track(eventName, signal.data)
-      }
-  }`
-
 export class IndexPage extends BasePage {
   constructor() {
-    super(`/signals-vanilla/index.html`, edgeFn)
+    super(`/signals-vanilla/index.html`)
   }
 
   async makeAnalyticsPageCall(): Promise<unknown> {
@@ -24,7 +15,7 @@ export class IndexPage extends BasePage {
   }
 
   async mockRandomJSONApi() {
-    await this.page.route('http://localhost:3000/api/foo', (route) => {
+    await this.page.route('http://localhost:5432/api/foo', (route) => {
       return route.fulfill({
         contentType: 'application/json',
         status: 200,
@@ -37,7 +28,7 @@ export class IndexPage extends BasePage {
 
   async makeFetchCallToRandomJSONApi(): Promise<void> {
     return this.page.evaluate(() => {
-      return fetch('http://localhost:3000/api/foo', {
+      return fetch('http://localhost:5432/api/foo', {
         method: 'POST',
         headers: {
           'Content-Type': 'application/json',
@@ -48,4 +39,8 @@ export class IndexPage extends BasePage {
         .catch(console.error)
     })
   }
+
+  async clickButton() {
+    return this.page.click('#some-button')
+  }
 }