Skip to content

feat: Add placement event mapping for event attributes#57

Open
prestonstarkey wants to merge 5 commits intomparticle-integrations:developmentfrom
prestonstarkey:feat/LE-add-event-mapping-configuration
Open

feat: Add placement event mapping for event attributes#57
prestonstarkey wants to merge 5 commits intomparticle-integrations:developmentfrom
prestonstarkey:feat/LE-add-event-mapping-configuration

Conversation

@prestonstarkey
Copy link

Summary

Currently in the Rokt Kit, you can only map specific events (Event Type, Event Category, & Event Name) to a Rokt attribute.
image

This PR expands the existing functionality to map event attributes matching certain conditions to a Rokt attribute.

Example: "Events with url attribute containing sale will map to saleseeker Rokt attribute.

The existing functionality still works as expected. The conditions are stored in a separate object called placementEventAttributeMappingLookup. It will map an array of conditions to the mapped attribute key.

Testing Plan

Testing locally, unit tests, going to make changes to the connection level to add the JSON custom field to allow for the placement event mapping rules

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Expands the Rokt kit’s placement event mapping to also support mapping event attributes (with optional conditional operators) to Rokt local session attributes, while keeping the existing event-type/category/name mapping behavior intact.

Changes:

  • Added placementEventAttributeMappingLookup plus rule evaluation (exists / equals / contains) and application during processEvent.
  • Parsed new placementEventAttributeMapping setting and exposed a test helper to build the lookup.
  • Added unit tests for lookup generation and conditional attribute mapping behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/Rokt-Kit.js Implements attribute-based mapping rules and applies them during event processing.
test/src/tests.js Adds unit tests covering lookup generation and condition evaluation for attribute mappings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +94 to +98
if (conditions.length === 0) {
return true;
}

var actualValue = getEventAttributeValue(event, rule.eventAttributeKey);
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

When a mapping has no conditions (or conditions default to an empty array), doesEventMatchRule returns true without checking whether the referenced event attribute actually exists on the event. This can cause mapped local session attributes to be set for every processed event even when event.EventAttributes[rule.eventAttributeKey] is missing. Consider treating an empty/missing conditions array as an implicit exists check (or explicitly requiring the attribute key to be present before returning true).

Suggested change
if (conditions.length === 0) {
return true;
}
var actualValue = getEventAttributeValue(event, rule.eventAttributeKey);
var actualValue = getEventAttributeValue(event, rule.eventAttributeKey);
// When no explicit conditions are provided, require that the attribute exists.
if (conditions.length === 0) {
return actualValue !== null;
}

Copilot uses AI. Check for mistakes.
Comment on lines +3577 to +3603
it('should map multiple attributes for the same mapped key (AND across rules)', async () => {
const placementEventAttributeMapping = JSON.stringify([
{
jsmap: null,
map: 'URL',
maptype: 'EventAttributeClass.Name',
value: 'saleSeeker',
conditions: [
{
operator: 'contains',
attributeValue: 'sale',
},
],
},
{
jsmap: null,
map: 'URL',
maptype: 'EventAttributeClass.Name',
value: 'saleSeeker1',
conditions: [
{
operator: 'contains',
attributeValue: 'items',
},
],
},
]);
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

This test name/description is misleading: it says "multiple attributes for the same mapped key (AND across rules)", but the sample mapping uses two different mapped keys (saleSeeker and saleSeeker1) and the assertions validate both keys can be set (which is not an AND-across-rules scenario). Please rename the test (or adjust the mapping to actually cover multiple rules for the same mapped key) to avoid confusion about the intended behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +3518 to +3536
it('should require ALL rules for the same mapped key to match (AND across rules)', async () => {
const placementEventAttributeMapping = JSON.stringify([
{
jsmap: null,
map: 'URL',
maptype: 'EventAttributeClass.Name',
value: 'saleSeeker',
conditions: [
{
operator: 'contains',
attributeValue: 'sale',
},
{
operator: 'contains',
attributeValue: 'items',
},
],
},
]);
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The "AND across rules" behavior in applyPlacementEventAttributeMapping is implemented across multiple mapping entries with the same mapped key (i.e., multiple rules in rulesForMappedAttributeKey). This test only validates AND behavior across multiple conditions within a single mapping entry, so it doesn't exercise the multi-rule aggregation logic. Add a test case with two mapping objects that share the same value to ensure all rules must match before setting the mapped attribute.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We actually want to check if the mapping lookup is empty early so we can avoid any unnecessary processing later in this function. I know in your applyPlacementEventAttributeMapping funciton, you have some guard clauses, but from a readability standpoint, it's probably better to do them here so we avoid cognitive complexity by having to bounce around between functions.

Comment on lines +143 to +148
if (
!self.placementEventAttributeMappingLookup ||
isEmpty(self.placementEventAttributeMappingLookup)
) {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need to check if it exists since isEmpty should be doing that too,

Suggested change
if (
!self.placementEventAttributeMappingLookup ||
isEmpty(self.placementEventAttributeMappingLookup)
) {
return;
}
if (isEmpty(self.placementEventAttributeMappingLookup)) {
return;
}

Comment on lines +72 to +77
if (
typeof actualValue !== 'string' ||
typeof expectedValue !== 'string'
) {
return false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're directly checking if an operator is a string in a lot places. I know it's not a lot of code, but I would recommend creating a simple isString utility function. It'll clean up a lot of these sections and make them more readable.

Suggested change
if (
typeof actualValue !== 'string' ||
typeof expectedValue !== 'string'
) {
return false;
}
if (!isString(actualValue) || !isString(expectedValue)) {
return false;
}

Comment on lines +157 to +162
if (
!rulesForMappedAttributeKey ||
!rulesForMappedAttributeKey.length
) {
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (
!rulesForMappedAttributeKey ||
!rulesForMappedAttributeKey.length
) {
continue;
}
if (isEmpty(rulesForMappedAttributeKey)) {
continue;
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants