Skip to content

Conversation

@rodrigosantorato
Copy link

@rodrigosantorato rodrigosantorato commented Nov 14, 2025

This PR Adds additional fields from our Report Events API to our impression action destination.
See our docs

The Objects page and placement have conditionals. Those ifs protect current users of getting errors, by making sure we only read values from the segment Product Viewed Event when we're certain those values do not cause a validation error on the Topsort Report Events API. This protects both current users and future users who are unaware of our validations.

Unit tests

santorato@MacBook-Pro-de-Rodrigo action-destinations % yarn cloud test --testPathPattern=src/destinations/topsort yarn run v1.22.22 $ yarn workspace @segment/action-destinations test --testPathPattern=src/destinations/topsort $ jest --testPathPattern=src/destinations/topsort ts-jest[ts-jest-transformer] (WARN) Define ts-jestconfig underglobals` is deprecated. Please do
transform: {
<transform_regex>: ['ts-jest', { /* ts-jest config goes here in Jest */ }],
},
See more at https://kulshekhar.github.io/ts-jest/docs/getting-started/presets#advanced
ts-jest[config] (WARN)
The "ts-jest" config option "isolatedModules" is deprecated and will be removed in v30.0.0. Please use "isolatedModules: true" in /Users/santorato/code/action-destinations/packages/destination-actions/tsconfig.json instead, see https://www.typescriptlang.org/tsconfig/#isolatedModules

PASS src/destinations/topsort/impression/tests/index.test.ts
PASS src/destinations/topsort/purchase/tests/index.test.ts
PASS src/destinations/topsort/impression/tests/snapshot.test.ts
PASS src/destinations/topsort/impressionsList/tests/index.test.ts
PASS src/destinations/topsort/click/tests/index.test.ts
PASS src/destinations/topsort/impressionsList/tests/snapshot.test.ts
PASS src/destinations/topsort/purchase/tests/snapshot.test.ts
PASS src/destinations/topsort/click/tests/snapshot.test.ts

Test Suites: 8 passed, 8 total
Tests: 25 passed, 25 total
Snapshots: 18 passed, 18 total
Time: 0.631 s, estimated 1 s
Ran all test suites matching /src/destinations/topsort/i.
✨ Done in 1.91s.`

Local e2e test

Using this payload with the new fields
{ "messageId": "test-message-y0dt0hyf3x", "timestamp": "2025-11-12T16:33:07.760Z", "type": "track", "email": "[email protected]", "position": 1, "properties": { "resolvedBidId": "SSMwRQoQBpE1CGG5cUmYBIh_O32g2hIQAZpWyyspdBKykINT91tvrxoQAZT62y7od6KOGfcUSYiZjiIICgQ3NjQ2EAEwyadxQOX0BkgBUOOXypunMw", "property3": true, "property4": false, "additionalAttribution": { "id": "entity-id-456", "type": "product" }, "externalVendorId": "vendor-123", "entity": { "id": "entity-id-456", "type": "product" }, "placement": { "page": 1, "pageSize": 20, "productId": "product-pdp-999", "categoryIds": [ "cat-1", "cat-2" ], "searchQuery": "test search query" }, "page": { "type": "search", "pageId": "search-results-page-123" }, "object": { "type": "listing", "clickType": "product" }, "channel": "onsite" }, "userId": "test-user-9c8y3pgoj3", "event": "Product Viewed", "anonymousId": "au9awm036xq", "context": { "active": true, "app": { "name": "InitechGlobal", "version": "545", "build": "3.0.1.545", "namespace": "com.production.segment" }, "campaign": { "name": "TPS Innovation Newsletter", "source": "Newsletter", "medium": "email", "term": "tps reports", "content": "image link" }, "device": { "id": "B5372DB0-C21E-11E4-8DFC-AA07A5B093DB", "advertisingId": "7A3CBEA0-BDF5-11E4-8DFC-AA07A5B093DB", "adTrackingEnabled": true, "manufacturer": "Apple", "model": "iPhone7,2", "name": "maguro", "type": "ios", "token": "ff15bc0c20c4aa6cd50854ff165fd265c838e5405bfeb9571066395b8c9da449" }, "ip": "8.8.8.8", "library": { "name": "analytics.js", "version": "2.11.1" }, "locale": "en-US", "location": { "city": "San Francisco", "country": "United States", "latitude": 40.2964197, "longitude": -76.9411617, "speed": 0 }, "network": { "bluetooth": false, "carrier": "T-Mobile US", "cellular": true, "wifi": false }, "os": { "name": "iPhone OS", "version": "8.1.3" }, "page": { "path": "This should work", "referrer": "", "search": "", "title": "Analytics Academy", "url": "https://segment.com/academy/" }, "referrer": { "id": "ABCD582CDEFFFF01919", "type": "dataxu" }, "screen": { "width": 320, "height": 568, "density": 2 }, "groupId": "12345", "timezone": "Europe/Amsterdam", "userAgent": "Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B143 Safari/601.1" }, "receivedAt": "2025-11-12T16:33:07.760Z", "sentAt": "2025-11-12T16:33:07.760Z", "version": 2 }

image

Local e2e test for backwards compatibility

Using the default payload + resolvedBidId which is required
image

Testing

Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [If destination is already live] Tested for backward compatibility of destination. Note: New required fields are a breaking change.
  • [Segmenters] Tested in the staging environment
  • [Segmenters] [If applicable for this change] Tested for regression with Hadron.

// Prevents this field from being automatically filled with a value that fails the validation on the Report Events API
if (t !== 'mobile' && t !== 'desktop') {
t = undefined
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this function into a functions file in the impressions folder?

}
}
},
object: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want this field to be a list of objects? or is this supposed to be a single object with 3 sub fields?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I recommend adding this attribute to your object fields.
defaultObjectUI:'keyvalue'

It will default the UI for the field to show the key:value pairs the customer is expected to input into the object.

label: 'Entity ID',
description: "The marketplace's ID of the entity associated with the interaction.",
type: 'string',
required: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making the id sub field required will break the Integration for anny customers who are already using the 'Additional attribution information' field but who were not already providing data in the id sub field.

Adding this configuration change is OK only if you don't have customers using this integration already. Please confirm.

}
},
default: {
'@path': '$.properties.additionalAttribution'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make all property default mappings snake_cased please?

label: 'Entity',
description:
'Entity is meant for reporting organic events, not sponsored or promoted products. It refers to the object involved in the organic interaction. If resolvedBidId has any value, entity will be disregarded.',
type: 'object',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you intend for this to be an array or Entities or a single Entity?

Comment on lines +161 to +178
default: {
'@if': {
exists: { '@path': '$.context.page.path' },
then: {
'@merge': {
objects: [
{
path: { '@path': '$.context.page.path' },
position: { '@path': '$.position' }
},
{ '@path': '$.properties.placement' }
],
direction: 'right'
}
},
else: { '@path': '$.properties.page' }
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This default path isn't going to work.

You can specify separate default paths for each of your sub fields, like this. Note that you can use separate if, then, else for each sub field if you need to.


default: {
  searchQuery : {
    '@if': {
          exists: { '@path': '$.context.page.path' },
          then:  { '@path': '$.context.page.path' },
          else: { '@path':  '$.properties.page_path'}
    }
  },
  categoryIds: {
    '@if': {
          exists: { '@path': '$.context.page.category_ids' },
          then:  { '@path': '$.context.page.category_ids' },
          else: { '@path':  '$.properties.category_ids'}
    }
 etc...
  }
}

Comment on lines +205 to +221
default: {
'@if': {
exists: { '@path': '$.properties.page.type' },
then: {
'@merge': {
objects: [
{
value: { '@path': '$.context.page.title' }
},
{ '@path': '$.properties.page' }
],
direction: 'right'
}
},
else: { '@path': '$.properties.page' }
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again same as above - this default mapping isn't going to work.

# JetBrains byproduct
.idea
coverage
.nvmrc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove any changes to the .gitignore file.

"process": "^0.11.10",
"timers-browserify": "^2.0.12",
"ts-jest": "^27.0.7",
"ts-jest": "^29.4.5",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PLease remove any changes to the package.json file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants