Skip to content

fix: Refactor RoktManager with deferred call handling and message queue processing#1051

Merged
alexs-mparticle merged 7 commits intodevelopmentfrom
fix/SQDSDKS-7404-queuing
Aug 12, 2025
Merged

fix: Refactor RoktManager with deferred call handling and message queue processing#1051
alexs-mparticle merged 7 commits intodevelopmentfrom
fix/SQDSDKS-7404-queuing

Conversation

@alexs-mparticle
Copy link
Collaborator

@alexs-mparticle alexs-mparticle commented Aug 5, 2025

Instructions

  1. PR target branch should be against development
  2. PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
  3. PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

  • Refactors the queueMessage logic of the Rokt Manager to defer promises for Rokt functions so that consumers can properly await the results.

Testing Plan

  • Was this tested locally? If not, explain why.
  • Using a sample web app, make a call to selectPlacements that triggers before the mParticle web sdk finishes loading the Rokt Web Kit. The call should return placements correctly after queuing.

As an example:

mParticle.ready( async function () {
  try {
    const selection = await window.mParticle.Rokt.selectPlacements({
        identifier: `mparticle-layout-test`,
        attributes: {
            email: 'test@email.com',
            firstname: 'Kain',
            lastname: 'Marko',
        }
    });
    selection.on('OFFER_ENGAGEMENT').subscribe(function (data) {
        console.log('offer engagement fired', data);
    });
  } catch (error) {
    console.error('Error selectPlacements:', error);
  }
});

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

@alexs-mparticle alexs-mparticle changed the title Enhance RoktManager with deferred call handling and message queue processing fix: Refactor RoktManager with deferred call handling and message queue processing Aug 5, 2025
Copy link
Member

Choose a reason for hiding this comment

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

How about removing messageQueue completely and have all the logic go based off if pendingPromises has anything in it? Then the type for the key could include resolve, reject, payload, method. This way we don't need 2 different queues.

Copy link
Member

Choose a reason for hiding this comment

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

nit: Add this identical test above to line 1103 but it'd be

expect(roktManager['pendingPromises'].has(messageId)).toBe(true);

Copy link
Member

Choose a reason for hiding this comment

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

Can this be combined with the first test? Seems like it's mostly doing the same thing...but adding a few extra checks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They're different tests. This one is handling a generic promise that resolves, but the test above handles a promise that has been rejected.

Copy link
Member

Choose a reason for hiding this comment

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

i was referring to not the test above, but the first test should resolve pending promise with success result

Copy link
Member

Choose a reason for hiding this comment

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

const queuedMessage = roktManager['messageQueue'].values()[0].value;

Copy link
Member

Choose a reason for hiding this comment

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

here and elsewhere where you have .next

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it.only('should create a deferred promise with unique messageId', () => {
it('should create a deferred promise with unique messageId', () => {

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 6, 2025

Copy link
Member

@rmi22186 rmi22186 left a comment

Choose a reason for hiding this comment

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

tested locally and looks good. great work!

this will require a docs update. we need to let customers know to wrap selectplacement in mParticle.ready here https://docs.rokt.com/developers/integration-guides/getting-started/ecommerce/ecommerce-sdk-integration#5-show-a-placement

@alexs-mparticle alexs-mparticle changed the base branch from master to development August 12, 2025 14:38
@alexs-mparticle alexs-mparticle merged commit a8b8cbc into development Aug 12, 2025
43 of 49 checks passed
github-actions bot pushed a commit that referenced this pull request Aug 12, 2025
## [2.43.2](v2.43.1...v2.43.2) (2025-08-12)

### Bug Fixes

* Refactor RoktManager with deferred call handling and message queue processing ([#1051](#1051)) ([a8b8cbc](a8b8cbc))
@mparticle-automation
Copy link
Collaborator

🎉 This PR is included in version 2.43.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments