-
Notifications
You must be signed in to change notification settings - Fork 845
Scope3: Add rtd targeting in seatbid #4606
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
base: master
Are you sure you want to change the base?
Conversation
| require.False(t, exists) | ||
|
|
||
| // Verify targeting section exists (add_to_targeting: true) | ||
| prebidData, exists := extMap["prebid"].(map[string]interface{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and elsewhere, I do wonder if gjson would make this easier than all of the casting, but in test code, I don't care that much. One possible benefit is that you can just get the result at any depth of gjsonpath and then check if result.Exists() rather than needing to check and cast each level.
| } else { | ||
| targetingMap[segment] = "true" | ||
| if m.cfg.SingleSegmentKey != "" { | ||
| currentVal := gjson.GetBytes(payload.BidResponse.Ext, "prebid.targeting."+m.cfg.SingleSegmentKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like "prebid.targeting."+m.cfg.SingleSegmentKey could be a member - there's no need to do string concat for every request if the m.cfg.SingleSegmentKey never changes once the server is running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put another way - at Builder time, do the concat and just set m.cfg.SingleSegmentKey to that (or make new struct field if you want/need it untouched for some reason)
| if currentVal.Exists() { | ||
| newPayload, err := sjson.SetBytes(payload.BidResponse.Ext, "prebid.targeting."+m.cfg.SingleSegmentKey, currentVal.String()+","+segment) | ||
| if err != nil { | ||
| return payload, err | ||
| } | ||
| payload.BidResponse.Ext = newPayload | ||
| } else { | ||
| newPayload, err := sjson.SetBytes(payload.BidResponse.Ext, "prebid.targeting."+m.cfg.SingleSegmentKey, segment) | ||
| if err != nil { | ||
| return payload, err | ||
| } | ||
| payload.BidResponse.Ext = newPayload | ||
| } | ||
| } else { | ||
| newPayload, err := sjson.SetBytes(payload.BidResponse.Ext, "prebid.targeting."+segment, "true") | ||
| if err != nil { | ||
| return payload, err | ||
| } | ||
| payload.BidResponse.Ext = newPayload | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this could be simplified significantly by having the logic choose or override the key and value and doing the newpayload and error checking only once.
The targeting also needs to be added to
seatbid[].bid[]