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

Aso Bid Adapter: add new bid adapter #7062

Merged
merged 6 commits into from
Jun 29, 2021

Conversation

adserver-online
Copy link
Contributor

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
  • test parameters for validating bids
//banner
{
  bidder: 'aso',
  params: {
    zone: 73815
  }
}
//video
{
  bidder: 'aso',
  params: {
    zone: 34668
  }
}

@ChrisHuie ChrisHuie changed the title Adserver.Online Bid Adapter: add new bid adapter feature Aso Bid Adapter: add new bid adapter Jun 21, 2021
@jsnellbaker jsnellbaker requested a review from osazos June 21, 2021 13:30
Copy link
Collaborator

@osazos osazos left a comment

Choose a reason for hiding this comment

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

Hi @adserver-online,

A question about your buildRequests() method, If the adUnit is configured with both Banner and Video mediaTypes, you will bid on Banner only. Is it the expected behavior?

Here the missing part to validate the adapter:

  • FloorPrices module support is required with Prebid.js 5.x.
  • you have to provide a renderer for outsream video context.

modules/asoBidAdapter.js Show resolved Hide resolved
@adserver-online adserver-online requested a review from osazos June 24, 2021 06:58
@osazos
Copy link
Collaborator

osazos commented Jun 24, 2021

Hi @adserver-online, thanks for your quick reply. All seems ok for me.

@osazos osazos added LGTM and removed needs review labels Jun 24, 2021
@adserver-online
Copy link
Contributor Author

@osazos We're sorry for the changes after approval. We've made outstream renderer better configurable. Please have a look.

@adserver-online adserver-online requested a review from osazos June 25, 2021 14:46
Copy link
Collaborator

@osazos osazos left a comment

Choose a reason for hiding this comment

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

LGTM

@osazos osazos merged commit f0f970c into prebid:master Jun 29, 2021
@osazos
Copy link
Collaborator

osazos commented Jun 29, 2021

Hi @adserver-online,

I merged your PR, but we get a CI error due to the usage of Object.entries, see bellow.
I don't know why this error has not been reported before. Your tests were all green…

Can you make a new PR to quickly fix it? Ping me in comment when it's done.

Edge 17.17134 (Windows 10): Executed 6794 of 6801 (skipped 11) SUCCESS (32.632 secs / 18.375 secs)
IE 11.0 (Windows 10): Executed 6794 of 6801 (2 FAILED) (skipped 11) (34.419 secs / 22.694 secs)
Chrome 79.0.3945.79 (Windows 10): Executed 6794 of 6801 (skipped 11) SUCCESS (29.922 secs / 15.418 secs)
Chrome 90.0.4430.72 (Windows 10): Executed 6794 of 6801 (skipped 11) SUCCESS (19.792 secs / 10.409 secs)
Edge 90.0.818.39 (Windows 10): Executed 6794 of 6801 (skipped 11) SUCCESS (25.3 secs / 13.331 secs)
Firefox 88.0 (Windows 10): Executed 6794 of 6801 (skipped 11) SUCCESS (22.494 secs / 12.761 secs)
Safari 14.1 (Mac OS 10.15.7): Executed 6794 of 6801 (skipped 11) SUCCESS (15.078 secs / 10.441 secs)
Firefox 72.0 (Windows 10): Executed 6794 of 6801 (skipped 11) SUCCESS (22.481 secs / 13.021 secs)
Safari 12.1.2 (Mac OS 10.14.6): Executed 6794 of 6801 (skipped 11) SUCCESS (11.446 secs / 7.97 secs)
TOTAL: 2 FAILED, 61144 SUCCESS


1) should send GDPR/USP consent data if it applies
     Adserver.Online bidding adapter GDPR/USP compliance
     TypeError: Object doesn't support property or method 'entries'
   at getConsentsIds (webpack:///modules/asoBidAdapter.js:289:3 <- test/test_index.js:119801:21)
   at createBasePayload (webpack:///modules/asoBidAdapter.js:326:5 <- test/test_index.js:119842:5)
   at Anonymous function (webpack:///modules/asoBidAdapter.js:26:7 <- test/test_index.js:119538:7)
   at _each (webpack:///src/utils.js:399:29 <- test/test_index.js:603:29)
   at buildRequests (webpack:///modules/asoBidAdapter.js:25:5 <- test/test_index.js:119537:5)
   at Anonymous function (webpack:///test/spec/modules/asoBidAdapter_spec.js:194:7 <- test/test_index.js:119375:7)

2) should return iframe sync option
     Adserver.Online bidding adapter response handler getUserSyncs
     TypeError: Object doesn't support property or method 'entries'
   at getConsentsIds (webpack:///modules/asoBidAdapter.js:289:3 <- test/test_index.js:119801:21)
   at getUserSyncs (webpack:///modules/asoBidAdapter.js:117:9 <- test/test_index.js:119627:9)
   at Anonymous function (webpack:///test/spec/modules/asoBidAdapter_spec.js:333:9 <- test/test_index.js:119488:9)

@adserver-online
Copy link
Contributor Author

Hi,
We will fix it today asap

@ChrisHuie
Copy link
Collaborator

Merged the fix for ie 🙏 Thanks @adserver-online for jumping on this

prebidtappx pushed a commit to prebidtappx/Prebid.js that referenced this pull request Nov 15, 2021
* Adserver.Online bidder adapter

* Added Price Floors support

* Added outstream renderer

* Docs updated

* coverage improved

* Added config support to outstream renderer

Co-authored-by: dev <[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