Eplanning: Add support for adomain #4227
Conversation
| String crId; | ||
|
|
||
| @JsonProperty("adomain") | ||
| List<String> adomain; |
There was a problem hiding this comment.
as I understand adomain is not an array, it's a plain text
There was a problem hiding this comment.
but in Go they treat it like array:
There was a problem hiding this comment.
check the line 46 on your screenshot, the bidder literally returns the plain text, but the ortb2 response expects an array, so they set an array of a single element to the bid.adomain
There was a problem hiding this comment.
oh, and I just noticed that the the name should be adom instead of adomain, please fix that as well
| @JsonProperty("crid") | ||
| String crId; | ||
|
|
||
| @JsonProperty("adomain") |
There was a problem hiding this comment.
the annotation is redundant
|
|
||
| @JsonProperty("adomain") | ||
| List<String> adomain; | ||
| @JsonProperty() |
There was a problem hiding this comment.
please remove json property
There was a problem hiding this comment.
I know this field isn't needed, but all the other fields have it, and when I edit the adapter, I try to keep things consistent.
There was a problem hiding this comment.
The best practice here is not leaving a code that does nothing. Anyway all other fields have the @JsonProperty because they need them (maybe except for adM - I believe it's just a typo, so you can rename it to adm and remove the annotation as well)
There was a problem hiding this comment.
Just noticed the name test-eplanning-bid-response-1.json ends with 1 which is confusing and weird, let's rename the file
There was a problem hiding this comment.
Okay, but this is not a file made by me. It was already like that.
There was a problem hiding this comment.
"Always leave the code a little better than you found it" (c) The Boy Scout Rule
🔧 Type of changes
✨ What's the context?
#4199