Skip to content

feat: Add all identities from mp user to rokt attributes#24

Merged
Mansi-mParticle merged 2 commits into
developmentfrom
feat/SQDSDKS-7187-add-Identities-to-rokt
May 16, 2025
Merged

feat: Add all identities from mp user to rokt attributes#24
Mansi-mParticle merged 2 commits into
developmentfrom
feat/SQDSDKS-7187-add-Identities-to-rokt

Conversation

@Mansi-mParticle

Copy link
Copy Markdown
Contributor

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

  • This PR adds user identities to the attributes sent to the Rokt SDK and removes the attribute filtering logic, since it's already handled through filteredUser within the Rokt kit.

Testing Plan

  • Was this tested locally? If not, explain why.
  • Tested with sample app and executed test cases

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

Comment thread src/main/kotlin/com/mparticle/kits/RoktKit.kt
@Mansi-mParticle Mansi-mParticle merged commit 3ada269 into development May 16, 2025
16 checks passed
Comment on lines +120 to +122
assertTrue(result.containsKey("key1"))
assertTrue(result.containsKey("key2"))
assertTrue(result.containsKey("key3"))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you also have a test that checks the value as well?

Comment on lines +146 to +149
assertTrue(result.containsKey("key1"))
assertTrue(result.containsKey("key2"))
assertTrue(result.containsKey("key3"))
assertTrue(result.containsKey("email"))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you also have a test that checks the value as well?

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.

3 participants