OFF-1457: height should use the compactHeight and standardHeight fields in decisions response #8
Conversation
|
@MishelleBit there are tests that are failing due to the change and need to be fixed https://github.com/wishabi/prebid-server/actions/runs/10944550258/job/30386689526?pr=8 |
MishelleBit
changed the title
|
This PR is missing a Jira ticket reference in the title or description. |
|
🥷 Code experts: hasan-kanjee hasan-kanjee has most 🧠 knowledge in the files. See details
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
MishelleBit
changed the title
MishelleBit
changed the title
There was a problem hiding this comment.
@MishelleBit can you look at the tests again and make sure we are only updating the relevant values. We should be updating the mockResponse customData and the expectedBidResponses h value.
adapters/flipp/flipp.go
Outdated
| adTypes = []int64{4309, 641} | ||
| dtxTypes = []int64{5061} | ||
| flippExtParams openrtb_ext.ImpExtFlipp | ||
| key string |
There was a problem hiding this comment.
you can just initialize the key here to be standardHeight. Also maybe we can make it a bit more descriptive like customDataKey
There was a problem hiding this comment.
initializing the key there to be standardHeight causes some errors. I think it's because the key will never be modified again in buildBid to standardHeight from compactHeight if it's needed @hasan-kanjee
adapters/flipp/flipp.go
Outdated
| if flippExtParams.Options.StartCompact { | ||
| bid.H = defaultCompactHeight | ||
| key = "compactHeight" | ||
| } else { | ||
| bid.H = defaultStandardHeight | ||
| key = "standardHeight" | ||
| } |
There was a problem hiding this comment.
then here you can do something like
bid.H = defaultStandardHeight
if flippExtParams.Options.StartCompact {
bid.H = defaultCompactHeight
key = "compactHeight"
} | customDataMap := customDataInterface.(map[string]interface{}) | ||
|
|
||
| bid.H = defaultStandardHeight | ||
| customDataKey = "standardHeight" |
There was a problem hiding this comment.
@MishelleBit looks good you can just move this to the top where you are declaring the customDataKey
There was a problem hiding this comment.
LGTM @MishelleBit can you please update the PR title name as well
MishelleBit
changed the title
No description provided.