Skip to content

IX Bid Adapter: First Party Data Support#6784

Closed
umakajan wants to merge 5 commits intoprebid:masterfrom
indexexchange:IX_first-party-data
Closed

IX Bid Adapter: First Party Data Support#6784
umakajan wants to merge 5 commits intoprebid:masterfrom
indexexchange:IX_first-party-data

Conversation

@umakajan
Copy link
Contributor

@umakajan umakajan commented May 18, 2021

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

Description of change

In this PR, we are updating our bidder to read first party data from the standard Prebid locations

A link to a PR on the docs repo at prebid/prebid.github.io#2971

@umakajan umakajan force-pushed the IX_first-party-data branch from 501da5d to c752376 Compare May 21, 2021 11:32
@ChrisHuie ChrisHuie self-requested a review May 21, 2021 19:47
@ChrisHuie ChrisHuie self-assigned this May 21, 2021
@ChrisHuie ChrisHuie requested review from robertrmartinez and removed request for ChrisHuie and robertrmartinez May 24, 2021 10:15
@ChrisHuie ChrisHuie removed their assignment May 24, 2021
@jeanstemp
Copy link

Docs PR: prebid/prebid.github.io#2971

@umakajan
Copy link
Contributor Author

Hey @gwhigs is there anything else you need from us to aide in review of this PR?

@gwhigs
Copy link
Collaborator

gwhigs commented May 28, 2021

Hi @umakajan, I've done an initial read-through and saw no issues. As this is my first review here, I appreciate your patience as I read through related documentation before submitting.

I anticipate submitting my review early next week.

@umakajan
Copy link
Contributor Author

Awesome thanks @gwhigs, no worries, just wanted to confirm PR was ready for review. Appreciate you getting back to us!

@umakajan umakajan marked this pull request as draft May 28, 2021 20:01
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure how this is used, but parsing may fail for referrer values with existing QSP.

Copy link
Collaborator

Choose a reason for hiding this comment

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

r.site.page may also be undefined here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@umakajan just to clarify my first comment — the existing logic assumes no QSP are present in the referrer value and always appends firstPartyString with ? (see L478).

For referrer values with existing QSP this will result in a URL with multiple ?, which may fail to parse.

@bretg
Copy link
Contributor

bretg commented Jun 3, 2021

docs PR prebid/prebid.github.io#2971

@umakajan umakajan force-pushed the IX_first-party-data branch from 7024c8f to 9acd28f Compare June 3, 2021 18:50
@patmmccann
Copy link
Collaborator

Can you confirm this is intentionally still in draft @ix-prebid-development

@umakajan
Copy link
Contributor Author

@patmmccann Yes, it is intentionally marked as draft, we are waiting to confirm with our beta publisher on this.

@MartianTribe
Copy link

Docs PR:

prebid/prebid.github.io#2971

@umakajan umakajan force-pushed the IX_first-party-data branch 2 times, most recently from 1c67a95 to 6a3ef77 Compare June 17, 2021 16:54
@umakajan umakajan force-pushed the IX_first-party-data branch from 2eeeae8 to 6a3ef77 Compare July 19, 2021 14:42
@umakajan umakajan force-pushed the IX_first-party-data branch from 6a3ef77 to 11edb20 Compare July 19, 2021 15:51
@umakajan umakajan closed this Jul 21, 2021
@patmmccann
Copy link
Collaborator

Hi @umakajan, we were excited about this PR; can you share your reasons for closing it?

@umakajan
Copy link
Contributor Author

Hi @patmmccann, there are some additional changes we need to make which we discovered from a recent round of testing.

The plan was to reopen this when it was ready for review, rather than keeping it in draft. Would you prefer if we kept it open as draft instead?

@patmmccann
Copy link
Collaborator

patmmccann commented Jul 21, 2021

that's up to you, we have just been keeping an eye on this bc of the close partnership between our companies. Thanks for the detail.

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.

7 participants

Comments