-
Notifications
You must be signed in to change notification settings - Fork 290
[TOPSORT] Add Optional Fields for Impressions #3416
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
base: main
Are you sure you want to change the base?
[TOPSORT] Add Optional Fields for Impressions #3416
Conversation
| // 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 | ||
| } |
There was a problem hiding this comment.
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: { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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', |
There was a problem hiding this comment.
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?
| 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' } | ||
| } | ||
| } |
There was a problem hiding this comment.
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...
}
}
| default: { | ||
| '@if': { | ||
| exists: { '@path': '$.properties.page.type' }, | ||
| then: { | ||
| '@merge': { | ||
| objects: [ | ||
| { | ||
| value: { '@path': '$.context.page.title' } | ||
| }, | ||
| { '@path': '$.properties.page' } | ||
| ], | ||
| direction: 'right' | ||
| } | ||
| }, | ||
| else: { '@path': '$.properties.page' } | ||
| } | ||
| } |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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.
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) Definets-jestconfig underglobals` is deprecated. Please dotransform: {
<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 }Local e2e test for backwards compatibility
Using the default payload + resolvedBidId which is required

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.