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

Ftrack Id Module: adding parameter to callback in getId return object #8678

Merged
merged 14 commits into from
Jul 21, 2022

Conversation

ftxmoJason
Copy link
Contributor

@ftxmoJason ftxmoJason commented Jul 12, 2022

Type of change

  • Other

Description of change

  1. Adding a cb parameter to the callback property in the object returned by getId() so that more pbjs get ID methods can be used
  2. Tweaking the eids.js so that a value is returned. We tried to return our IDs as an object but had to JSON.stringify() it.

@TheMediaGrid
Copy link
Contributor

@patmmccann any idea on how Flashtalking could return the IDs directly within the uids object of the eids? 'return data' didn't work so @ftxmoJason had to go with json.stringify, but we'd like the structure within uids to be flat and the IDs to be listed right in the JSPN structure

@ftxmoJason
Copy link
Contributor Author

To build on @TheMediaGrid 's comment. Our IDs are formatted like so:

{
    "HHID": ["<USERS HH ID>"],
    "DeviceID": ["<USERS DEVICE ID>"],
    "SingleDeviceID": ["USERS SINGLE DEVICE ID"]
}

And we can't return an object from eids.js getValue, so we JSON.stringify() the object and return it. Is that the best approach?

@ftxmoJason
Copy link
Contributor Author

@ftxmojared Just trying to ping you in this thread for awareness.

@ChrisHuie ChrisHuie changed the title Ftrack getid mods - adding parameter to callback in getId return object Ftrack Id Module: adding parameter to callback in getId return object Jul 13, 2022
@patmmccann
Copy link
Collaborator

have you studied this #8429

@ftxmoJason
Copy link
Contributor Author

Thanks @patmmccann for the suggestion.
I tweaked the createEidsArray method and added a condition for 'ftrack', similar to how 'pubProvidedId' did it.

Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

i did an initial pass through and first thing we need are test cases for the new functionality. also, are there any changes to publisher implementation of the config or does this only impact those reading the EIDs array?

Comment on lines +371 to +382
} else if (subModuleKey === 'ftrackId') {
// ftrack has multiple IDs so we add each one that exists
let eid = {
'atype': 1,
'id': (bidRequestUserId[subModuleKey]['DeviceID'] || []).join('|'),
'ext': {}
}
for (let id in bidRequestUserId[subModuleKey]) {
eid.ext[id] = (bidRequestUserId[subModuleKey][id] || []).join('|');
}

eids.push(eid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this block needs tests cases, please

Copy link
Contributor

Choose a reason for hiding this comment

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

no changes to the publisher config implementation, just the eids object interaction is changed
I will also let @ftxmoJason comment on the test cases

@TheMediaGrid
Copy link
Contributor

@smenzer can you please take a look at the updates here? we'd really like this PR to make it to the next release if possible

test/spec/modules/ftrackIdSystem_spec.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@smenzer smenzer left a comment

Choose a reason for hiding this comment

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

LGTM

This probably warrants an update to the prebid docs repository, but I'll go ahead and approve this now since this is good to go.

@smenzer smenzer merged commit f7196dc into prebid:master Jul 21, 2022
ahmadlob referenced this pull request in taboola/Prebid.js Jul 27, 2022
… (#8678)

* Adding cb to the callback

* Delete package-lock.json

Removing package-lock.json because it has conflicts and its not on ftrack to change it either way

* Un conflicting the package-lock

* ftrack: getting data to return in the EIDS

* ftrack: getting data to return in the EIDS lint

* ftrack: making the eids return all IDs

* ftrack: making the eids return all IDs

* ftrack: tweaking the EID schema per MediaGrid's requests

* Added tests for getUserIdsAsEids method

* Updated the ftrack decode function with suggested code and updated related tests

* Updated getUserIdsAsEids tests to match value directly

Co-authored-by: Jason Lydon <[email protected]>
Co-authored-by: Jared <[email protected]>
ccorbo pushed a commit to ccorbo/Prebid.js that referenced this pull request Jul 27, 2022
…prebid#8678)

* Adding cb to the callback

* Delete package-lock.json

Removing package-lock.json because it has conflicts and its not on ftrack to change it either way

* Un conflicting the package-lock

* ftrack: getting data to return in the EIDS

* ftrack: getting data to return in the EIDS lint

* ftrack: making the eids return all IDs

* ftrack: making the eids return all IDs

* ftrack: tweaking the EID schema per MediaGrid's requests

* Added tests for getUserIdsAsEids method

* Updated the ftrack decode function with suggested code and updated related tests

* Updated getUserIdsAsEids tests to match value directly

Co-authored-by: Jason Lydon <[email protected]>
Co-authored-by: Jared <[email protected]>
JacobKlein26 pushed a commit to nextmillenniummedia/Prebid.js that referenced this pull request Feb 9, 2023
…prebid#8678)

* Adding cb to the callback

* Delete package-lock.json

Removing package-lock.json because it has conflicts and its not on ftrack to change it either way

* Un conflicting the package-lock

* ftrack: getting data to return in the EIDS

* ftrack: getting data to return in the EIDS lint

* ftrack: making the eids return all IDs

* ftrack: making the eids return all IDs

* ftrack: tweaking the EID schema per MediaGrid's requests

* Added tests for getUserIdsAsEids method

* Updated the ftrack decode function with suggested code and updated related tests

* Updated getUserIdsAsEids tests to match value directly

Co-authored-by: Jason Lydon <[email protected]>
Co-authored-by: Jared <[email protected]>
jorgeluisrocha pushed a commit to jwplayer/Prebid.js that referenced this pull request May 23, 2023
…prebid#8678)

* Adding cb to the callback

* Delete package-lock.json

Removing package-lock.json because it has conflicts and its not on ftrack to change it either way

* Un conflicting the package-lock

* ftrack: getting data to return in the EIDS

* ftrack: getting data to return in the EIDS lint

* ftrack: making the eids return all IDs

* ftrack: making the eids return all IDs

* ftrack: tweaking the EID schema per MediaGrid's requests

* Added tests for getUserIdsAsEids method

* Updated the ftrack decode function with suggested code and updated related tests

* Updated getUserIdsAsEids tests to match value directly

Co-authored-by: Jason Lydon <[email protected]>
Co-authored-by: Jared <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants