Skip to content
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

Refactor the TTD usage and add test #40

Closed
wants to merge 4 commits into from
Closed

Refactor the TTD usage and add test #40

wants to merge 4 commits into from

Conversation

wi101
Copy link

@wi101 wi101 commented Feb 28, 2024

CM-1182

  • Define uid1Eids for the trade desk identifier and refactor its usage.
  • the uid1 respect the priority when ttd is present in different modules.

@@ -644,9 +644,9 @@ describe('User ID', function () {
it('pbjs.getUserIdsAsEids should prioritize user ids according to config available to core', () => {
init(config);
setSubmoduleRegistry([
createMockIdSubmodule('mockId1Module', {id: {uid2: {id: 'uid2_value'}}}),
createMockIdSubmodule('mockId1Module', {id: {uid2: {id: 'uid2_value'}, tdid: {id: 'uid1_value'}}}),
Copy link
Author

Choose a reason for hiding this comment

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

@wi101 wi101 self-assigned this Feb 28, 2024
@@ -236,7 +237,9 @@ export const liveIntentIdSubmodule = {
}

if (value.thetradedesk) {
result.thetradedesk = { 'id': value.thetradedesk, ext: { provider: getRefererInfo().domain || LI_PROVIDER_DOMAIN } }
result.lipb = {...result.lipb, tdid: value.thetradedesk}
result.tdid = { 'id': value.thetradedesk, ext: { rtiPartner: 'TDID', provider: getRefererInfo().domain || LI_PROVIDER_DOMAIN } }

Choose a reason for hiding this comment

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

Why getRefererInfo().domain here? Seems like we are not doing this elsewhere

Copy link
Author

Choose a reason for hiding this comment

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

yes it was requested by Deepthi, #31 (comment)

@3link 3link self-requested a review February 29, 2024 08:47
@@ -667,6 +667,7 @@ describe('User ID', function () {

const ids = {
'uid2': { id: 'uid2_value_from_mockId3Module' },
'uid1': { id: 'uid1_value_from_mockId3Module' },
Copy link

Choose a reason for hiding this comment

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

I think you should add uid1: ['mockId1Module', 'mockId3Module'] in line 657 to have the priorities explicit and this line you then can test if the value from the mockId1Module has won - if I understand the intention of the test correctly.

Maybe it would also be worth putting it into a separate test case?

@wi101 wi101 requested a review from 3link March 1, 2024 12:01
@wi101
Copy link
Author

wi101 commented Mar 5, 2024

PR opened: prebid#11174

@wi101 wi101 closed this Mar 5, 2024
@wi101 wi101 deleted the ttd-refactoring branch March 5, 2024 09:42
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.

4 participants