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.

Sign up for GitHub

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

Jump to bottom

Dailymotion Bid Adaptor: initial release #10970

Merged
merged 27 commits into from Apr 18, 2024
Merged

Dailymotion Bid Adaptor: initial release #10970

merged 27 commits into from Apr 18, 2024

Conversation

kvnsw
Copy link
Contributor

@kvnsw kvnsw commented Jan 22, 2024 •
edited

Type of change

Description of change

A new bidder adapter dailymotionBidAdapter added.

By setting the following configuration options, you'll get a constant response to any request to validate your adapter integration:

const adUnits = [
  {
    bids: [{
      bidder: 'dailymotion',
      params: {
        apiKey: 'dailymotion-testing'
      }
    }],
    code: 'test-ad-unit',
    mediaTypes: {
      video: {
        api: [2, 7],
        context: 'instream',
        playerSize: [ [1280, 720] ],
        startDelay: 0
      },
    }
  }
];

@kvnsw kvnsw mentioned this pull request Jan 22, 2024
Merged
3 tasks
@kvnsw kvnsw mentioned this pull request Jan 31, 2024
Merged
@kvnsw
Copy link
Contributor Author

kvnsw commented Feb 13, 2024

Hello @jlquaccia, following changes related to @dgirardi's feedback, could we have a second review? :) Thanks!

@ChrisHuie ChrisHuie requested review from ChrisHuie and removed request for jlquaccia February 20, 2024 23:36
@ChrisHuie ChrisHuie assigned ChrisHuie and unassigned jlquaccia Feb 20, 2024
patmmccann
patmmccann requested changes Feb 24, 2024
View reviewed changes
modules/dailymotionBidAdapter.md Outdated Show resolved Hide resolved
This was referenced Mar 1, 2024
Merged
Closed
Merged
Aditi0101 and others added 8 commits March 1, 2024 15:41
@Aditi0101 @kvnsw
Dailymotion Bid Adaptor: support video metadata
9aab2ea
@kvnsw
Merge branch 'prebid:master' into master
7ee40c4
@kvnsw
Dailymotion Bid Adaptor: add support for sending adUnitCode
32d6391
@kvnsw
Merge branch 'prebid:master' into master
d5542db
@kvnsw
Dailymotion Bid Adaptor: add support for sending startDelay
ce0536c
@kvnsw
Merge branch 'prebid:master' into master
9a97052
@sebmil-daily
feat(LEO-528): Allow multiple IAB categories in video metadata
3092928 
The same way as we can have an array of IAB categories level 1 in the ORTB request, this PR introduces an array for the IAB categories level 2.

To be forward compatible with level [2.2](https://github.com/InteractiveAdvertisingBureau/Taxonomies/blob/main/Content%20Taxonomies/Content%20Taxonomy%202.2.tsv) and [3.0](https://github.com/InteractiveAdvertisingBureau/Taxonomies/blob/main/Content%20Taxonomies/Content%20Taxonomy%203.0.tsv) specifications, the category IDs should be sent as strings.
@sebmil-daily
Merge pull request #10 from dailymotion-oss/LEO-528_multiple_iabcat2
4b78d35 
feat(LEO-528): Allow multiple IAB categories level 2 in video metadata
@kvnsw
Copy link
Contributor Author

kvnsw commented Mar 25, 2024

Hello @patmmccann, @dgirardi, @ChrisHuie,
Following our changes to address your feedback, could you please provide an additional review/validation of this PR?
Thank you!

@kvnsw kvnsw requested a review from sebmil-daily April 8, 2024 12:11
@kvnsw
Copy link
Contributor Author

kvnsw commented Apr 9, 2024

Hello @patmmccann, @dgirardi, @ChrisHuie, sorry to bother, I think we have addressed all your feedbacks now and have a final submission. Please let us know if we can move forward :) Thank you!

@karimMourra karimMourra self-requested a review April 11, 2024 21:47
@ChrisHuie ChrisHuie assigned karimMourra and unassigned ChrisHuie Apr 15, 2024
patmmccann
patmmccann reviewed Apr 17, 2024
View reviewed changes
modules/dailymotionBidAdapter.js Outdated
config: {
api_key: bid.params.apiKey
},
coppa: config.getConfig('coppa'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

please get this from the request and not the confgi

@patmmccann patmmccann assigned patmmccann and unassigned karimMourra Apr 17, 2024
@patmmccann
Copy link
Collaborator

@kvnsw taking over this review from Karim; looking forward to working with you. Feel free to slack me

@kvnsw kvnsw mentioned this pull request Apr 18, 2024
Merged
@kvnsw kvnsw requested a review from patmmccann April 18, 2024 15:37
@patmmccann patmmccann merged commit 08ac74f into prebid:master Apr 18, 2024
2 of 3 checks passed
@karimMourra
Copy link
Collaborator

@patmmccann Note that the Video Taskforce had some concerns about this PR:

In an effort to simplify configuration for publishers, the Prebid Video Taskforce is requiring bid adapters to follow the oRTB spec.

  • mediaTypes.video should contain fields from oRTB section 3.2.7

  • For content specific fields, we encourage reading from site.content, see ortb 2.6 section 3.2.13

Reference: ortb 2.6 spec: https://iabtechlab.com/wp-content/uploads/2022/04/OpenRTB-2-6_FINAL.pdf](https://github.com/prebid/Prebid.js/pull/10970)

anything outside of the spec can be included in the bidder params (i.e. params.video). Though the implementation wouldn't change, we are concerned with the examples and documentation mentioning non-ortb fields in mediaTypes.video

@m-gendi
Copy link

m-gendi commented Apr 18, 2024 •
edited

Thank you @patmmccann and @karimMourra for your reviews, we appreciate it.
We will address the remaining comments in a new PR, aiming to resolve them.

@sebmil-daily sebmil-daily mentioned this pull request Apr 19, 2024
Merged
1 task
mefjush pushed a commit to adhese/Prebid.js that referenced this pull request Apr 25, 2024
@kvnsw @Aditi0101
@sebmil-daily @rumesh @mefjush
Dailymotion Bid Adaptor: initial release (prebid#10970)
b2d0494 
* Dailymotion Bid Adaptor: initial release

* .md file lint issue resolved

* Dailymotion Bid Adaptor: build bidder request based on param with fallbacks

* Dailymotion Bid Adaptor: support video metadata

* Dailymotion Bid Adaptor: add support for sending adUnitCode

* Dailymotion Bid Adaptor: add support for sending startDelay

* feat(LEO-528): Allow multiple IAB categories in video metadata

The same way as we can have an array of IAB categories level 1 in the ORTB request, this PR introduces an array for the IAB categories level 2.

To be forward compatible with level [2.2](https://github.com/InteractiveAdvertisingBureau/Taxonomies/blob/main/Content%20Taxonomies/Content%20Taxonomy%202.2.tsv) and [3.0](https://github.com/InteractiveAdvertisingBureau/Taxonomies/blob/main/Content%20Taxonomies/Content%20Taxonomy%203.0.tsv) specifications, the category IDs should be sent as strings.

* Dailymotion bid adapter: Clarify the video metadata to provide in each player context

* Dailymotion bid adapter: Move API key to bid params

* Dailymotion bid adapter: Verify API key is string

Co-authored-by: Rumesh <rumesh@users.noreply.github.com>

* Dailymotion bid adapter: Move API key to bid params (fix tests)

* Dailymotion Bid Adaptor: add gpp support and get coppa from request

* Dailymotion Bid Adaptor: fix lint error

* Dailymotion Bid Adaptor: add iabcat1 and fallback to ortb2 for iabcat2

* Dailymotion Bid Adaptor: get iabcats from ortb2.site.content.data

---------

Co-authored-by: Kevin Siow <kevin.siow@dailymotion.com>
Co-authored-by: Aditi Chaudhary <aditichaudhary175@gmail.com>
Co-authored-by: Sébastien Millet <sebastien.millet@dailymotion.com>
Co-authored-by: Rumesh <rumesh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patmmccann patmmccann patmmccann approved these changes

@sebmil-daily sebmil-daily sebmil-daily approved these changes

@karimMourra karimMourra Awaiting requested review from karimMourra

@ChrisHuie ChrisHuie Awaiting requested review from ChrisHuie

@dgirardi dgirardi Awaiting requested review from dgirardi

Assignees

@patmmccann patmmccann

Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@kvnsw @patmmccann @karimMourra @m-gendi @ChrisHuie @dgirardi @sebmil-daily @jlquaccia @Aditi0101