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

KRKPD-619 Adding site to Kargo adapter. #24

Merged
merged 6 commits into from
Oct 5, 2023
Merged

KRKPD-619 Adding site to Kargo adapter. #24

merged 6 commits into from
Oct 5, 2023

Conversation

njflynn
Copy link

@njflynn njflynn commented Oct 2, 2023

Type of change

  • Bugfix

  • [ x] 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

Description of change

Adding support to Kargo adapter for site object

@njflynn njflynn self-assigned this Oct 2, 2023
juliangan07
juliangan07 requested changes Oct 4, 2023
View reviewed changes
modules/kargoBidAdapter.js Outdated
@@ -95,6 +95,7 @@ function buildRequests(validBidRequests, bidderRequest) {
},
imp: impressions,
user: getUserIds(tdidAdapter, bidderRequest.uspConsent, bidderRequest.gdprConsent, firstBidRequest.userIdAsEids, bidderRequest.gppConsent),
site: firstBidRequest.site
Copy link

@juliangan07 juliangan07 Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

site is actually nested in another object ortb2.
i.e: firstBidRequest.ortb2.site

Sample query available in the ticket description.

juliangan07
juliangan07 requested changes Oct 4, 2023
View reviewed changes
modules/kargoBidAdapter.js Outdated
@@ -95,6 +95,7 @@ function buildRequests(validBidRequests, bidderRequest) {
},
imp: impressions,
user: getUserIds(tdidAdapter, bidderRequest.uspConsent, bidderRequest.gdprConsent, firstBidRequest.userIdAsEids, bidderRequest.gppConsent),
site: firstBidRequest.ortb2.site
Copy link

Choose a reason for hiding this comment

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

Might need to add null checks for ortb2:

select
count(time_id_utc) as total
from stage_kraken_bid_request e
where time_id_utc = 2023100310
and e.src:data:body:bidRequest:prebidRawBidRequests[0] is not null
and e.src:data:body:bidRequest:prebidRawBidRequests[0]:ortb2 is null;

---------------------------------------------

TOTAL
33,079

select
count(time_id_utc) as total
from stage_kraken_bid_request e
where time_id_utc = 2023100310
and e.src:data:body:bidRequest:prebidRawBidRequests[0]:ortb2 is not null
and e.src:data:body:bidRequest:prebidRawBidRequests[0]:ortb2:site is null;

------------------------------------------------

TOTAL
0

Copy link
Author

Choose a reason for hiding this comment

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

Updated

juliangan07
juliangan07 requested changes Oct 4, 2023
View reviewed changes
Copy link

@juliangan07 juliangan07 left a comment

Choose a reason for hiding this comment

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

Minor thing, SF data suggests that when ortb2 is defined, site is always defined, but not the other way around.

modules/kargoBidAdapter.js Outdated Show resolved Hide resolved
juliangan07
juliangan07 approved these changes Oct 4, 2023
View reviewed changes
Copy link

@juliangan07 juliangan07 left a comment

Choose a reason for hiding this comment

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

Looks good!

@njflynn njflynn requested a review from jsadwith October 4, 2023 15:13
jsadwith
jsadwith approved these changes Oct 4, 2023
View reviewed changes
Copy link

@jsadwith jsadwith left a comment

Choose a reason for hiding this comment

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

Looks great, Neil.

@njflynn njflynn merged commit 203abca into master Oct 5, 2023
sj1815 pushed a commit that referenced this pull request Feb 27, 2024
@YakirLavi @noamtzuberi
@noamtzu @OronW @lasloche @innay
Rise Bid Adapter : support currency (prebid#10790)
27b57ab 
* add Rise adapter

* fixes

* change param isOrg to org

* Rise adapter

* change email for rise

* fix circle failed

* bump

* bump

* bump

* remove space

* Upgrade Rise adapter to 5.0

* added isWrapper param

* addes is_wrapper parameter to documentation

* added is_wrapper to test

* removed isWrapper

* Rise Bid Adapter: support Coppa param (#24)

* MinuteMedia Bid Adapter: support Coppa param (#25)

* Revert "MinuteMedia Bid Adapter: support Coppa param (#25)" (#26)

This reverts commit 66c4e7b.

* bump

* update coppa fetch

* setting coppa param update

* update Coppa tests

* update test naming

* Rise Bid Adapter: support plcmt and sua (#27)

* update minuteMediaBidAdapter - support missing params (#29)

* RIseBidAdapter: support currency (#35)

---------

Co-authored-by: Noam Tzuberi <[email protected]>
Co-authored-by: noamtzu <[email protected]>
Co-authored-by: Noam Tzuberi <[email protected]>
Co-authored-by: Laslo Chechur <[email protected]>
Co-authored-by: OronW <[email protected]>
Co-authored-by: lasloche <[email protected]>
Co-authored-by: inna <[email protected]>
Co-authored-by: YakirLavi <[email protected]>
sj1815 pushed a commit that referenced this pull request Feb 27, 2024
@YakirLavi @noamtzuberi
@noamtzu @OronW @lasloche @innay
MinuteMedia Bid Adapter : support currency (prebid#10791)
645b562 
* add Rise adapter

* fixes

* change param isOrg to org

* Rise adapter

* change email for rise

* fix circle failed

* bump

* bump

* bump

* remove space

* Upgrade Rise adapter to 5.0

* added isWrapper param

* addes is_wrapper parameter to documentation

* added is_wrapper to test

* removed isWrapper

* Rise Bid Adapter: support Coppa param (#24)

* MinuteMedia Bid Adapter: support Coppa param (#25)

* Revert "MinuteMedia Bid Adapter: support Coppa param (#25)" (#26)

This reverts commit 66c4e7b.

* bump

* update coppa fetch

* setting coppa param update

* update Coppa tests

* update test naming

* Rise Bid Adapter: support plcmt and sua (#27)

* update minuteMediaBidAdapter - support missing params (#29)

* add currency param to bid object and tests

* update getFloor function and tests

* adding test to currency param

* adding doc & currency bidfloor support & update tests

* update currency test

* remove default test

---------

Co-authored-by: Noam Tzuberi <[email protected]>
Co-authored-by: noamtzu <[email protected]>
Co-authored-by: Noam Tzuberi <[email protected]>
Co-authored-by: Laslo Chechur <[email protected]>
Co-authored-by: OronW <[email protected]>
Co-authored-by: lasloche <[email protected]>
Co-authored-by: inna <[email protected]>
Co-authored-by: YakirLavi <[email protected]>
sj1815 pushed a commit that referenced this pull request Feb 27, 2024
@innay @noamtzuberi
@noamtzu @OronW @lasloche @YakirLavi
MinuteMedia BidAdapter : add support for gpp (prebid#11052)
6e13ea0 
* add Rise adapter

* fixes

* change param isOrg to org

* Rise adapter

* change email for rise

* fix circle failed

* bump

* bump

* bump

* remove space

* Upgrade Rise adapter to 5.0

* added isWrapper param

* addes is_wrapper parameter to documentation

* added is_wrapper to test

* removed isWrapper

* Rise Bid Adapter: support Coppa param (#24)

* MinuteMedia Bid Adapter: support Coppa param (#25)

* Revert "MinuteMedia Bid Adapter: support Coppa param (#25)" (#26)

This reverts commit 66c4e7b.

* bump

* update coppa fetch

* setting coppa param update

* update Coppa tests

* update test naming

* Rise Bid Adapter: support plcmt and sua (#27)

* update minuteMediaBidAdapter - support missing params (#29)

* support gpp for minutemedia adapter

* removed spaces

* removed extra character

---------

Co-authored-by: Noam Tzuberi <[email protected]>
Co-authored-by: noamtzu <[email protected]>
Co-authored-by: Noam Tzuberi <[email protected]>
Co-authored-by: Laslo Chechur <[email protected]>
Co-authored-by: OronW <[email protected]>
Co-authored-by: lasloche <[email protected]>
Co-authored-by: YakirLavi <[email protected]>
Co-authored-by: YakirLavi <[email protected]>
Co-authored-by: Inna Yaretsky <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fionasequeira fionasequeira fionasequeira approved these changes

@juliangan07 juliangan07 juliangan07 approved these changes

@jsadwith jsadwith jsadwith approved these changes

@SKearney11 SKearney11 Awaiting requested review from SKearney11

@sj1815 sj1815 Awaiting requested review from sj1815

@ssadman22 ssadman22 Awaiting requested review from ssadman22

Assignees

@njflynn njflynn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@njflynn @jsadwith @juliangan07 @fionasequeira