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

PS bid adapter update to copy site object from config #5083

Merged
merged 3 commits into from
Apr 21, 2020

Conversation

mmoschovas
Copy link
Contributor

@mmoschovas mmoschovas commented Apr 6, 2020

Type of change

  • Other

Description of change

Updated Prebid Server bid adapter to check for the site object in the prebid configuration. If exists, this will be copied over into the request and publisher/page will either be created properties or will overwrite existing config defined values. This will allow other OpenRTB fields within the site object to be passed to Prebid Server, i.e. site.content

Other information

#3783

… prebid configuration. If exists, this will be copied over into the request and publisher/page will either be created properties or will overwrite existing config defined values. This will allow other OpenRTB fields within the site object to be passed to Prebid Server, i.e. site.content
@jsnellbaker
Copy link
Collaborator

@mmoschovas Please check the lint errors in the circleci test report link and fix them.

}
request.site = (typeof config.getConfig('site') === 'object') ? config.getConfig('site') : {};
request.site.publisher = { id: _s2sConfig.accountId },
request.site.page = pageUrl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if someone sets site in setConfig, we want to use it.

But we want to overwrite publisher and page no matter what?

Here is what the publisher object specifies in openRTB protocol:

image

So if we did:

pbjs.setConfig({
   site: {
      ...
      publisher: {
         id: 'some id'
         domain: 'prebid.org'
         ...
      }
});

Then we would basically not include the other keys in the user defined publisher object.

Maybe it makes more sense to use utils.deepSet in order to set the id of publisher, so that if publisher already exists we do not overwrite the entire thing but just the id key?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a test for this as well

… as opposed to overwritting entire publisher object
@idettman
Copy link
Contributor

idettman commented Apr 8, 2020

Changed pull request text formatting to better match Prebid.js pull request conventions.
Also added a related pull request (#3783) for reference/tracking.

@aleksatr
Copy link
Contributor

LGTM

Copy link
Collaborator

@robertrmartinez robertrmartinez left a comment

Choose a reason for hiding this comment

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

Nice Looks good!

@robertrmartinez robertrmartinez merged commit cf74b6c into prebid:master Apr 21, 2020
iggyfisk pushed a commit to happypancake/Prebid.js that referenced this pull request Jun 22, 2020
* Updated Prebid Server bid adapter to check for the site object in the prebid configuration. If exists, this will be copied over into the request and publisher/page will either be created properties or will overwrite existing config defined values. This will allow other OpenRTB fields within the site object to be passed to Prebid Server, i.e. site.content

* Update to fix lint error and set site.publisher.id using deepSet util as opposed to overwritting entire publisher object

* Updated prebid bid adapter spec to include test for site object update

Co-authored-by: Michael Moschovas <[email protected]>
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.

5 participants