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

feat/dailymotion-adaptor #4

Merged
merged 1 commit into from
Feb 1, 2024
Merged

feat/dailymotion-adaptor #4

merged 1 commit into from
Feb 1, 2024

Conversation

kvnsw
Copy link
Collaborator

@kvnsw kvnsw commented Jan 31, 2024

Changes following PR review comment prebid#10970 (comment)

kvnsw
kvnsw commented Jan 31, 2024
View reviewed changes
modules/dailymotionBidAdapter.js Outdated
page: bidderRequest?.refererInfo?.page || '',
},
uspConsent: bidderRequest?.uspConsent || '',
},
config: config.getConfig('dailymotion'),
coppa: config.getConfig('coppa'),
request: bid,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wonder if we should do the same for bid considering the comment, will ask there after we add this change to our submission

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point, doing this also for bid would reduce the payload for the request by roughly half.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would be good to measure the before / after to tackle the payload reduction at the same time.
Maybe in the future we'll change the key names to make it even shorter if necessary

Copy link

Choose a reason for hiding this comment

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

the same applies to responses as well; they're much simpler so not as likely to change, but we make no promises that they won't. (Sorry to interject, I just realized I did not address responses in my original comment).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hello @dgirardi thanks for your feedbacks :)

I believe we've applied the necessary changes on the request part (pending internal review). I'm just wondering if it's necessary to do the same for the responses. The idea is that by having the least manipulation possible, we can troubleshoot directly on our back-end service, as it would be the only place where the response object is being built.

It would also seem that the bid validity checks are being done within bidderFactory.js (and maybe elsewhere) and should lead to https://github.com/dailymotion-oss/Prebid.js/blob/feat/dailymotion-adaptor/src/constants.json#L129 error in case of invalidity. In case of breaking change of the logic, this would lead to our inability to bid which we would be able to react to.

Looking at some other adaptors, it would seem that this approach is being adopted elsewhere (eg: https://github.com/dailymotion-oss/Prebid.js/blob/feat/dailymotion-adaptor/modules/ad2ictionBidAdapter.js#L48). Another common approach seems to be to check the existence of keys within the response object (eg: https://github.com/dailymotion-oss/Prebid.js/blob/feat/dailymotion-adaptor/modules/addefendBidAdapter.js#L63).

Considering that it's not likely that the responses format will change (although no guarantee), and these different points, do you think it's reasonable to move forward in this manner? Or do you think we should at least do some checks on our side?

Thanks for your time!

Copy link

Choose a reason for hiding this comment

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

If I were responsible for long term maintenance of the service I would definitely not consider it reasonable, but it's your call to make - there's no requirement to decouple, or to do it now. Mine is just a recommendation; I know that adapters like those you linked would not even show up in our radar when we're rolling out a breaking change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you have any recommendation about how we could get notified of a breaking change ?

Copy link

Choose a reason for hiding this comment

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

Decouple! :) the problem with trying to get notified is that when everything is hidden server side, we don't even know what might be breaking, we gauge impact of changes by looking at the codebase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hello @dgirardi!
After internal discussion, we are comfortable to start with this version and iterate on the decoupling part later.
Let us know if that works with you :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hello @dgirardi, did you have any chances to look at our changes? Do not hesitate if you have any other feedback

sebmil-daily
sebmil-daily reviewed Jan 31, 2024
View reviewed changes
modules/dailymotionBidAdapter.js Show resolved Hide resolved
test/spec/modules/dailymotionBidAdapter_spec.js Outdated Show resolved Hide resolved
modules/dailymotionBidAdapter.js Outdated
page: bidderRequest?.refererInfo?.page || '',
},
uspConsent: bidderRequest?.uspConsent || '',
},
config: config.getConfig('dailymotion'),
coppa: config.getConfig('coppa'),
request: bid,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point, doing this also for bid would reduce the payload for the request by roughly half.

@kvnsw kvnsw force-pushed the feat/dailymotion-adaptor branch from c8c483d to b4f1e3b Compare January 31, 2024 14:26
@kvnsw kvnsw force-pushed the feat/dailymotion-adaptor branch from b4f1e3b to 318d548 Compare February 1, 2024 08:18
@kvnsw kvnsw merged commit 6c25624 into master Feb 1, 2024
@kvnsw kvnsw mentioned this pull request Feb 1, 2024
Merged
1 task
kvnsw pushed a commit that referenced this pull request Mar 1, 2024
@solta-dev @os-solta
Kimberlite Bidder Adapter: initial commit (prebid#11032)
0606e77 
* Kimberlite bid adapter (#1)

* initial: bid adapter

* styling

* Fix: lint (#2)

* Fix: lint (#4)

* review fixes (#6)

* Change: filling request.ext.prebid section (#7)

---------

Co-authored-by: Oleg <oleg.s@solta.io>
kvnsw pushed a commit that referenced this pull request Mar 1, 2024
@PavloMalashnyak
Blockthrough Bid Adapter: initial release (prebid#10870)
ab7068e 
* PE-87: Implement Prebid Adapter (#1)

* PE-87: implement BT Bid Adapter

* PE-87: rework adapter to use ortbConverter lib, make requested changes

* PE-87: update imports

* PE-110: Add user sync logic to the Prebid Adapter (#3)

* PE-110: add user sync logic

* PE-110: update userSync url

* PE-110: check if iframe is enabled before setting params

* PE-111: BT Prebid Adapter can request AA ads or regular ads (#2)

* PE-120: Send Prebid Bidder info to BT Server (#4)

* PE-120: add btBidderCode to the bid object

* PE-120: use single quotes for logs string

* PE-123: Add More Metadata in site.ext.blockthrough (#5)

* PE-123: send additional meta data

* PE-123: send auctionID under imp.ext.prebid.blockthrough

* PE-123: use ortb2 config to set site.ext params

* PE-123: sent auctionId in ext.prebid.blockthrough.auctionID

* PE-123: update logs for bidderConfig setup

* PE-000: check if blockthrough is defined (#6)

* PE-87: remove BT specific logic (#7)

* Implement Blockthrough Prebid Adapter

* PE-87: Implement Prebid Adapter - misc fixes (#9)

* PE-87: rename test file, add bidder config

* PE-87: increase ttl

* PE-000: fix test

* BP-74: Change the way we enable debug (#10)

* BP-79: Send GPID as a part of `imp[].ext` (#11)

* BP-79: send gpid in imp.ext

* BP-79: add optional operator

* BP-90: Update Cookie Sync Logic (#12)

* BP-90: pass bidder to cookie sync

* BP-90: update sync logic, fix typo

* BP-90: use const for syncs variable

* BP-55: Re-add endpoint URLs (#13)

* BP-91: Add prebid JS version to auction request (#14)
kvnsw pushed a commit that referenced this pull request Apr 3, 2024
@pashaGhub @eldzis
setupad Bid Adapter: initial commit (prebid#11008)
48113b1 
* create setupadBidAdapter

* add setupadBidAdapter

* update setupadBidAdapter

* update metrics collection

* update analytics collection

* update getUserSyncs

* add setupadAnalyticsAdapter.js

* test setupadAnalyticsAdapter

* remove test: 1

* add GVLID && bug fixes && test updates

* remove setupadAnalyticsAdapter

* add userID module handling

* add GVLID && bug fixes && test updates

* remove setupadAnalyticsAdapter

* add userID module handling

* clean up && seat bugfix

* clean up logs

* add userID module handling

* update md && clean up

* Send setupad only on bidRequested

* Fix bidResponse and bidWon responses

* Improve bidResponse and bidWon logic

* Revert changes to specific files

* Remove test parameter

* Fix multiple bidResponse and bidTimeout calls to getPixelUrl

* eslint errors fixes(brackets added)

* Add extra checks for events

* Fix BIDDER_CODE const

* update reporting endpoint

* update setupadBidAdapter_spec.js REPORT_ENDPOINT

* update readme

* Revert "Merge branch 'prebid:master' into setupad-adapter"

This reverts commit 1c14dbe, reversing
changes made to 7fe9ea5.

* Revert "Revert "Merge branch 'prebid:master' into setupad-adapter""

This reverts commit a34e3e4.

* # This is a combination of 20 commits.
# This is the 1st commit message:

add setupadBidAdapter

# This is the commit message #2:

update setupadBidAdapter

# This is the commit message #3:

update metrics collection

# This is the commit message #4:

update analytics collection

# This is the commit message #5:

update getUserSyncs

# This is the commit message #6:

add setupadAnalyticsAdapter.js

# This is the commit message #7:

test setupadAnalyticsAdapter

# This is the commit message #8:

remove test: 1

# This is the commit message #9:

add GVLID && bug fixes && test updates

# This is the commit message #10:

remove setupadAnalyticsAdapter

# This is the commit message #11:

add userID module handling

# This is the commit message #12:

clean up && seat bugfix

# This is the commit message #13:

add userID module handling

# This is the commit message #14:

add GVLID && bug fixes && test updates

# This is the commit message #15:

remove setupadAnalyticsAdapter

# This is the commit message #16:

add userID module handling

# This is the commit message #17:

clean up logs

# This is the commit message #18:

update md && clean up

# This is the commit message #19:

Send setupad only on bidRequested

# This is the commit message #20:

Fix bidResponse and bidWon responses

* # This is a combination of 22 commits.tree 8abae7e6dffc9a21ad11770713ba485fc610028a
parent cecfce3
author pavel <pavel@setupad.com> 1706627437 +0200
committer pavel <pavel@setupad.com> 1706627437 +0200
gpgsig -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEqGYI3KX/FkbObQG8FABtd4pCs/AFAmW5EW0ACgkQFABtd4pC
 s/CK3w//WWJSFUlycnnNKTV2XfdcBjooOeZZvjpXVthwr09CCC4uO//kw4bPluhn
 f5fcVFdXzrY1AZ6ch8Wo3msX/Pkso014jIGd5aIWcHpNYFtffACwH/40Y8AcJNZd
 bsOZxVK0awPTz/RihC5eY+0J3cP+iFWP/FlYJoHEQIBXq/Eg6mWoAhxwpL/JvxbY
 QbLFWsRn2ckQ6ftOZgm3/jh8VLaG1zWbWImlWEs5Zel+CorJBTniTj58VbApelYD
 TFMgbSR2I4NGVaqNIrHePnSMsDATxalQ2nZPwY6raKCHWIbvoUPIn/OpDMMbKgC7
 nCwounNmObxFVoj3xusAZppzHpKPasY8xKWb2Kr7zfhZArsOMC6B7fYqQNK0cWG3
 8RR/10oheJD9M2kRlfLiqnRv7ExY08SQ/ZMo9LA8BeRUGBXhh6++8FKhKIHvX1gL
 k1R5W6c+NNWP+PDFsmrFpMn+LpYdl84I7yfYK5dHuw80od7f1wuAVYpswi6Cziy9
 /KY6/rfENvUrGTmWSh5GdDBel89ACCfFkasIKB92xhzKTfjzF/DXkc8XQZOMbt1j
 CsILgWMNfLPMo4Dlgdx/tYCSLLBNEtZ1/hhUcFQ3+0TzLf0GtMkvMnlBnDinqe1n
 1P30fQ2I5W5NJKDPrCOnRymI6QOAPFXtMF11R81mbB9H8asft/E=
 =oJtZ
 -----END PGP SIGNATURE-----

bugfixes

# This is the commit message #22:

Remove test parameter

* # This is a combination of 26 commits.
parent cecfce3
author pavel <pavel@setupad.com> 1706627437 +0200
committer pavel <pavel@setupad.com> 1706627437 +0200
gpgsig -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEqGYI3KX/FkbObQG8FABtd4pCs/AFAmW5EW0ACgkQFABtd4pC
 s/CK3w//WWJSFUlycnnNKTV2XfdcBjooOeZZvjpXVthwr09CCC4uO//kw4bPluhn
 f5fcVFdXzrY1AZ6ch8Wo3msX/Pkso014jIGd5aIWcHpNYFtffACwH/40Y8AcJNZd
 bsOZxVK0awPTz/RihC5eY+0J3cP+iFWP/FlYJoHEQIBXq/Eg6mWoAhxwpL/JvxbY
 QbLFWsRn2ckQ6ftOZgm3/jh8VLaG1zWbWImlWEs5Zel+CorJBTniTj58VbApelYD
 TFMgbSR2I4NGVaqNIrHePnSMsDATxalQ2nZPwY6raKCHWIbvoUPIn/OpDMMbKgC7
 nCwounNmObxFVoj3xusAZppzHpKPasY8xKWb2Kr7zfhZArsOMC6B7fYqQNK0cWG3
 8RR/10oheJD9M2kRlfLiqnRv7ExY08SQ/ZMo9LA8BeRUGBXhh6++8FKhKIHvX1gL
 k1R5W6c+NNWP+PDFsmrFpMn+LpYdl84I7yfYK5dHuw80od7f1wuAVYpswi6Cziy9
 /KY6/rfENvUrGTmWSh5GdDBel89ACCfFkasIKB92xhzKTfjzF/DXkc8XQZOMbt1j
 CsILgWMNfLPMo4Dlgdx/tYCSLLBNEtZ1/hhUcFQ3+0TzLf0GtMkvMnlBnDinqe1n
 1P30fQ2I5W5NJKDPrCOnRymI6QOAPFXtMF11R81mbB9H8asft/E=
 =oJtZ
 -----END PGP SIGNATURE-----

bugfixes

# This is the commit message #22:

Remove test parameter

# This is the commit message #23:

Fix multiple bidResponse and bidTimeout calls to getPixelUrl

# This is the commit message #25:

eslint errors fixes(brackets added)

# This is the commit message #26:

Add extra checks for events

* parent 75178b9
author pavel <pavel@setupad.com> 1706627694 +0200
committer pavel <pavel@setupad.com> 1706627694 +0200
gpgsig -----BEGIN PGP SIGNATURE-----

 iQIzBAABCAAdFiEEqGYI3KX/FkbObQG8FABtd4pCs/AFAmW5Em4ACgkQFABtd4pC
 s/BBUQ/+NXyHoxPM185YJLG9M1ySC/5vTT9W5mfwQ93cVDLCeuGnpsnmi4S21NuQ
 b7gSeokFjwztvVOUmh/xqMp4lTsvL53TUd00b1k4KGVSqgcF00Foit5g8fOGLYsI
 DAoqphYV6MWjpAun+II+ELY8QUkHR1cjTc7PEGtmf+8RnptGVdyJ6C9Ab8u9TQTY
 Apj5Srhfo3Tl8S+WScOxwwB/uqEJR4fhIrJyzFzdLDEb2olSPyrQUs87vQXlhEnK
 buPEg2F5JsRH6sw11Xp3TFNSZGxNnBSlTh9dixou5md4yRCv5a2TMef667N0BVDp
 lGgc7mCrRKXyqzphmmeHudiscEGFjtUPObXoHutSVw22wdARFCTpNFKBLLFn4v8o
 Zv1OvFdNprvHsoeW0HVlZdU7OKnDTRrko6DHk2AahxojjvAFEWuDsGYZNjhdQwRR
 lK1zm+SFQnKI0Eojd+f84fvKod9geGs640jyH/x5R4eYm4yjZb8SkRtd3cca88wS
 OuGq9LIkbU428b46l7VnDwudldTXPUU8eKfUtFRjdGtIWH9I3tK6TsRoCfTcXkv0
 smxYiiU1XHjAkkPFWQWEeFdfZ071snFKVWouU0AoKiq+PdRoS8+3AJqIQUjlA2sH
 AybnSkv9KxY/Rs1bnvMubsQm1GF66qVrbxBU6FILBv1JZYwj4yA=
 =Gbog
 -----END PGP SIGNATURE-----

bugfixes

update setupadBidAdapter_spec.js REPORT_ENDPOINT

update readme

Revert "Merge branch 'prebid:master' into setupad-adapter"

This reverts commit 1c14dbe, reversing
changes made to 7fe9ea5.

Revert "Revert "Merge branch 'prebid:master' into setupad-adapter""

This reverts commit a34e3e4.

* change double quote to single quote

---------

Co-authored-by: pavel <pavel@setupad.com>
Co-authored-by: Elgars Grodnis <elgars@setupad.com>

* bugfix setupadBidAdapter

remove getAdEl, spelling correction

* add onBidWon event

onBidWon event handling moved from custom to native onBidWon method

* minor bugfixes && remove funk getSiteObj && getDeviceObj

---------

Co-authored-by: pavel <pavel@setupad.com>
Co-authored-by: Elgars Grodnis <elgars@setupad.com>
kvnsw pushed a commit that referenced this pull request Jun 18, 2024
@mike-lei @jpanduro-blackbird
Flipp Bid Adapter: fix height parameter (prebid#11633)
07e6e30 
* Flipp Bid Adapter: initial release

* Added flippBidAdapter

* OFF-372 Support DTX/Hero in flippBidAdapter (#2)

* support creativeType

* OFF-422 flippBidAdapter handle AdTypes

---------

Co-authored-by: Jairo Panduro <jpanduro@blackbird-lab.com>

* OFF-465 Add getUserKey logic to prebid.js adapter (#3)

* Support cookie sync and uid

* address pr feedback

* remove redundant check

* OFF-500 Support "startCompact" param for Prebid.JS #4

* set startCompact default value (#5)

* fix docs

* use client bidding endpoint

* update unit testing endpoint

* OFF-876 [Prebid Adapter] Check userKey for empty string (#6)

* add more checks to userKey

* update document

* add uuid format statement

* modify docs

* fix network id

* use compactHeight and standardHeight in customData (#7)

* OFF-1455 [Prebid.js] height should use the compactHeight and standardHeight fields in decisions response (#8)

* Flipp Bid Adapter: initial release

* Added flippBidAdapter

* OFF-372 Support DTX/Hero in flippBidAdapter (#2)

* support creativeType

* OFF-422 flippBidAdapter handle AdTypes

---------

Co-authored-by: Jairo Panduro <jpanduro@blackbird-lab.com>

* OFF-465 Add getUserKey logic to prebid.js adapter (#3)

* Support cookie sync and uid

* address pr feedback

* remove redundant check

* OFF-500 Support "startCompact" param for Prebid.JS #4

* set startCompact default value (#5)

* fix docs

* use client bidding endpoint

* update unit testing endpoint

* OFF-876 [Prebid Adapter] Check userKey for empty string (#6)

* add more checks to userKey

* update document

* add uuid format statement

* modify docs

* fix network id

* use compactHeight and standardHeight in customData

---------

Co-authored-by: Jairo Panduro <jpanduro@blackbird-lab.com>

* Update flippBidAdapter.js

* use compactHeight and standardHeight in customData (#7)

* update docs

* fix unit test

---------

Co-authored-by: Jairo Panduro <jpanduro@blackbird-lab.com>
sebmil-daily pushed a commit that referenced this pull request Jul 29, 2024
@os-solta
kimberliteBidAdapter: video media type support (prebid#11981)
40b906d 
* Kimberlite bid adapter (#1)

* initial: bid adapter

* styling

* Fix: lint (#2)

* Fix: lint (#4)

* review fixes (#6)

* Change: filling request.ext.prebid section (#7)

* Video support

* Fix: tests

* Adapter's version update

* No video defaults
sebmil-daily pushed a commit that referenced this pull request Dec 11, 2024
@doctafaustus @jsfledd
Nativo Bid Adapter: support native and video (prebid#12134)
b60d732 
* Initial nativoBidAdapter document creation (js, md and spec)

* Fulling working prebid using nativoBidAdapter. Support for GDPR and CCPA in user syncs.

* Added defult size settings based on the largest ad unit. Added response body validation. Added consent to request url qs params.

* Changed bidder endpoint url

* Changed double quotes to single quotes.

* Reverted package-json.lock to remove modifications from PR

* Added optional bidder param 'url' so the ad server can force- match an existing placement

* Lint fix. Added space after if.

* Added new QS param to send various adUnit data to adapter endpopint

* Updated unit test for new QS param

* Added qs param to keep track of ad unit refreshes

* Updated bidMap key default value

* Updated refresh increment logic

* Refactored spread operator for IE11 support

* Updated isBidRequestValid check

* Refactored Object.enties to use Object.keys to fix CircleCI testing errors

* Updated bid mapping key creation to prioritize ad unit code over placementId

* Added filtering by ad, advertiser and campaign.

* Merged master

* Added more robust bidDataMap with multiple key access

* Deduped filer values

* Rolled back package.json

* Duped upstream/master's package.lock file ... not sure how it got changed in the first place

* Small refactor of filterData length check. Removed comparison with 0 since a length value of 0 is already falsy.

* Added bid sizes to request

* Fixed function name in spec. Added unit tests.

* Added priceFloor module support

* Added protection agains empty url parameter

* Changed ntv_url QS param to use referrer.location instead of referrer.page

* Removed testing 'only' flag

* Added ntv_url QS param value validation

* Added userId support

* Added unit tests, refactored for bugs

* Wrapped ajax in try/catch

* Added more unit testing

* Updated eid check for duplicate values. Removed error logging as we no longer need it.

* Removed spec test .only. Fixed unit tests that were breaking.

* Added Prebid version to nativo exchange request

* Removed unused bidder methods

* Added OpenRTB payload response. Changes requerst type to POST.

* Removed debug log

* Added/fixed tests

* Handle video mediaType

* Add built renderer files

* Fix no-inner-declarations linting error

* Handle native requests

* Add examples in Nativo readme

* Add 'mediaType' property to tests for compatibility with adapter code

* Remove data URI from VAST XML

* Fix 'no-unused-expressions' lint error

* Fix lint error 'curcly'

* Remove bidder name validation in 'isBidRequestValid' (#4)

* Add GPP consent string to req (#5)

---------

Co-authored-by: Josh <jfledd@gmail.com>
Co-authored-by: Joshua Fledderjohn <jfledderjohn@nativo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@dgirardi dgirardi dgirardi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kvnsw @dgirardi @sebmil-daily