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

RtbSape Bid Adapter: restore for Prebid 5.x #7081

Merged
merged 2 commits into from
Jul 6, 2021
Merged

Conversation

ne0n
Copy link
Contributor

@ne0n ne0n commented Jun 23, 2021

Type of change

  • New bidder adapter
  • Other

Description of change

Restore RtbSape Bid Adapter after remove for Prebid 5.0. Campaign with ads for test now available.

  • test parameters for validating bids
\\banner
mediaTypes: {
  banner: {
    sizes: [[300, 250]]
  }
},
bids: [
  {
    bidder: "rtbsape",
    params: {
      placeId: 553307
    }
  }
]
\\video
{
  bidder: 'rtbsape',
  params: {
    placeId: 553309
  }
}

@ChrisHuie ChrisHuie self-requested a review June 23, 2021 08:20
@ChrisHuie ChrisHuie self-assigned this Jun 23, 2021
@ChrisHuie ChrisHuie self-requested a review June 23, 2021 17:38
@ChrisHuie
Copy link
Collaborator

@ne0n it doesn't appear that the requirements for prebid 5.0 have been added? Adomain, schain, support for floors module, and etc.

@ne0n
Copy link
Contributor Author

ne0n commented Jun 24, 2021

@ne0n it doesn't appear that the requirements for prebid 5.0 have been added? Adomain, schain, support for floors module, and etc.

The adapter works correctly with base Prebid 5.0. But... I'm not sure that we support all modules from Prebid 5.0. The adapter sends the original BidRequest object to the endpoint, but the endpoint might ignore some options. We plan to improve our endpoint as we receive requests from publishers. Is it critical?

@ChrisHuie
Copy link
Collaborator

@ne0n Since you don't support floors and other stuff you don't need to worry about that stuff. Can you please though pass adomain because that is now a requirement for Prebid 5.0 per #6650

@ne0n
Copy link
Contributor Author

ne0n commented Jun 25, 2021

I missed topic #6650, this is my mistake. We always set meta.advertiserDomains for every bid. It is implemented on a server side.

@ne0n
Copy link
Contributor Author

ne0n commented Jun 29, 2021

@ChrisHuie, good day. Everything is fine? Or maybe I need to fix something else?

@ChrisHuie
Copy link
Collaborator

@ne0n if you can add the adomain field so it complies with #6650 and then we are good to go

@ne0n
Copy link
Contributor Author

ne0n commented Jun 29, 2021

@ChrisHuie, adomain is contained in the response from the server. Always for all bids:
image
Adapter method interpretResponse returns the list of bids as they came from the endpoint. This is not enough? Should I set adomain somewhere else? You can check this on page https://cdn-rtb.sape.ru/rtb-test/prebidjs/index.html or on any other pages using test parameters from the description.

@ChrisHuie
Copy link
Collaborator

@ne0n that's fine this way then. Do your unit tests check the server response? Could you add a unit test to check adomain?

@ne0n
Copy link
Contributor Author

ne0n commented Jul 2, 2021

@ChrisHuie. I added adomain check to the adapter and updated the unit tests for this case.

@ChrisHuie ChrisHuie merged commit 498e748 into prebid:master Jul 6, 2021
prebidtappx pushed a commit to prebidtappx/Prebid.js that referenced this pull request Nov 15, 2021
* RtbSape Bid Adapter: restore for Prebid 5.x

* RtbSape Bid Adapter: check adomain (prebid#7081)

Co-authored-by: Dmitry Latyshev <d.latyshev@gmail.com>
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.

2 participants