Skip to content

Conversation

patrick-wu
Copy link

Summary

  • Adding setExperimentAllocationData to the RoktManager for clients to save experiment allocation data
  • data will be passed down to the Rokt Kit during select placement call with specific attribute keys

Testing Plan

  • TODO: Write unit tests for the new function

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

Comment on lines 87 to 125
if (this.experimentAllocation) {
options.attributes['rokt.experimentid'] = this.experimentAllocation.experimentId;
options.attributes['rokt.bucketid'] = this.experimentAllocation.bucketId;
options.attributes['rokt.userid'] = this.experimentAllocation.userId;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

You should put this in the try/catch block below. The queuing logic should be the first thing in this function because it runs before the rokt web sdk and kit initialize.

@patrick-wu patrick-wu force-pushed the feat/experiment-alloc branch from 33e1016 to 7e2dc18 Compare May 1, 2025 17:02
@alexs-mparticle alexs-mparticle requested a review from Copilot May 1, 2025 17:03
Copy link

@Copilot 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

This PR adds the experiment allocation functionality to the RoktManager, enabling clients to pass experiment allocation data to the Rokt Kit during placement selection.

  • Introduces the IRoktExperimentAllocation interface.
  • Adds the experimentAllocation property and updates selectPlacements to include experiment allocation attributes if available.
  • Implements the setExperimentAllocationData function for setting experiment allocation data.

throw new Error('Error setting extension data: ' + errorMessage);
}
}

Copy link

Copilot AI May 1, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a JSDoc comment above setExperimentAllocationData to clarify its purpose, input parameters, and usage.

Suggested change
/**
* Sets the experiment allocation data for a user.
*
* @param {string} userId - The unique identifier for the user.
* @param {string} experimentId - The unique identifier for the experiment.
* @param {string} bucketId - The unique identifier for the bucket within the experiment.
*
* This method stores the provided experiment allocation data in the `experimentAllocation` object.
*/

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.

Another useful suggestion and something we should be doing more often. Not a blocker.

Comment on lines 142 to 144
mappedAttributes['rokt.experimentid'] = this.experimentAllocation.experimentId;
mappedAttributes['rokt.bucketid'] = this.experimentAllocation.bucketId;
mappedAttributes['rokt.userid'] = this.experimentAllocation.userId;
Copy link

Copilot AI May 1, 2025

Choose a reason for hiding this comment

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

Consider extracting the hard-coded attribute keys ('rokt.experimentid', 'rokt.bucketid', 'rokt.userid') to constants to improve maintainability and reduce potential typos.

Suggested change
mappedAttributes['rokt.experimentid'] = this.experimentAllocation.experimentId;
mappedAttributes['rokt.bucketid'] = this.experimentAllocation.bucketId;
mappedAttributes['rokt.userid'] = this.experimentAllocation.userId;
mappedAttributes[ROKT_EXPERIMENT_ID] = this.experimentAllocation.experimentId;
mappedAttributes[ROKT_BUCKET_ID] = this.experimentAllocation.bucketId;
mappedAttributes[ROKT_USER_ID] = this.experimentAllocation.userId;

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.

I think this is a useful suggestion but not a blocker.

@alexs-mparticle alexs-mparticle requested a review from rmi22186 May 1, 2025 17:21
private placementAttributesMapping: Dictionary<string>[] = [];

private experimentAllocation: IRoktExperimentAllocation | null = null;

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

Copy link

sonarqubecloud bot commented May 1, 2025

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