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

I1503: Added dealTierSatisfied parameters in exchange.pbsOrtbBid and openrtb_ext.ExtBidPrebid and dealPriority in openrtb_ext.ExtBidPrebid #1558

Merged

Conversation

ShriprasadM
Copy link
Contributor

@ShriprasadM ShriprasadM commented Oct 29, 2020

Added

dealPriority in response.seatbid[i].bid[j].ext.prebid.dealpriority
dealTierSatisfied in response.seatbid[i].bid[j].ext.prebid.dealTierSatisfied

…].bid[j].ext.prebid.dealpriority and response.seatbid[i].bid[j].ext.prebid.dealTierSatisfied respectively.
@@ -303,6 +303,7 @@ func validateAndNormalizeDealTier(impDeal *DealTier) bool {
func updateHbPbCatDur(bid *pbsOrtbBid, dealTierInfo *DealTierInfo, bidCategory map[string]string) {
if bid.dealPriority >= dealTierInfo.MinDealTier {
Copy link
Contributor Author

@ShriprasadM ShriprasadM Oct 29, 2020

Choose a reason for hiding this comment

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

Looking at function name - updateHbPbCatDur, Can we move the following condition outside of this function
if bid.dealPriority >= dealTierInfo.MinDealTier {

after the change, It will look like

if validateDealTier(impDeal[bidder]) && topBidPerBidder.dealPriority >= impDeal[bidder].MinDealTier {
topBidPerBidder.dealTierSatisfied = true
updateHbPbCatDur(topBidPerBidder, impDeal[bidder], bidCategory)
}

@ShriprasadM ShriprasadM changed the title I1503: Added dealPriority and dealTierSatisfied in response.seatbid[i… I1503: Added dealPriority and dealTierSatisfied parameters in exchange.pbsOrtbBid and openrtb_ext.ExtBidPrebid Oct 29, 2020
@ShriprasadM ShriprasadM changed the title I1503: Added dealPriority and dealTierSatisfied parameters in exchange.pbsOrtbBid and openrtb_ext.ExtBidPrebid I1503: Added dealTierSatisfied parameters in exchange.pbsOrtbBid and openrtb_ext.ExtBidPrebid and dealPriority in openrtb_ext.ExtBidPrebid Oct 29, 2020
@ShriprasadM ShriprasadM marked this pull request as ready for review October 29, 2020 10:58
@SyntaxNode SyntaxNode requested a review from guscarreon October 29, 2020 14:16
@SyntaxNode SyntaxNode requested a review from bsardo November 2, 2020 15:28
Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1351,7 +1351,7 @@ func (m *mockExchangeVideo) HoldAuction(ctx context.Context, bidRequest *openrtb
if debugLog != nil && debugLog.Enabled {
m.cache.called = true
}
ext := []byte(`{"prebid":{"targeting":{"hb_bidder_appnexus":"appnexus","hb_pb_appnexus":"20.00","hb_pb_cat_dur_appnex":"20.00_395_30s","hb_size":"1x1", "hb_uuid_appnexus":"837ea3b7-5598-4958-8c45-8e9ef2bf7cc1"},"type":"video"},"bidder":{"appnexus":{"brand_id":1,"auction_id":7840037870526938650,"bidder_id":2,"bid_ad_type":1,"creative_info":{"video":{"duration":30,"mimes":["video\/mp4"]}}}}}`)
ext := []byte(`{"prebid":{"targeting":{"hb_bidder_appnexus":"appnexus","hb_pb_appnexus":"20.00","hb_pb_cat_dur_appnex":"20.00_395_30s","hb_size":"1x1", "hb_uuid_appnexus":"837ea3b7-5598-4958-8c45-8e9ef2bf7cc1"},"type":"video","dealpriority":0,"dealtiersatisfied":false},"bidder":{"appnexus":{"brand_id":1,"auction_id":7840037870526938650,"bidder_id":2,"bid_ad_type":1,"creative_info":{"video":{"duration":30,"mimes":["video\/mp4"]}}}}}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

This modification is not needed because, when ommited, both missing JSON int and boolean fields default to 0 and false respectively. Anyway, not a big deal if included.

Copy link
Collaborator

@bsardo bsardo left a comment

Choose a reason for hiding this comment

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

LGTM

@bsardo bsardo merged commit d821b3b into prebid:master Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants