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

appnexus adapter support empty keyvalues in bidder params #3257

Merged
merged 3 commits into from
Nov 6, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion modules/appnexusBidAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ export const spec = {
params.use_pmt_rule = (typeof params.usePaymentRule === 'boolean') ? params.usePaymentRule : false;
if (params.usePaymentRule) { delete params.usePaymentRule; }

if (utils.isArray(params.keywords) && params.keywords.length > 0) {
params.keywords.forEach(function(keyPairObj) {
if (utils.isArray(keyPairObj.value) && keyPairObj.value.length > 0 && keyPairObj.value[0] === '') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The if statement here and on line 360 are same. Please move in some function.

delete keyPairObj.value;
}
});
}

Object.keys(params).forEach(paramKey => {
let convertedKey = utils.convertCamelToUnderscore(paramKey);
if (convertedKey !== paramKey) {
Expand Down Expand Up @@ -345,7 +353,16 @@ function bidToTag(bid) {
tag.external_imp_id = bid.params.externalImpId;
}
if (!utils.isEmpty(bid.params.keywords)) {
tag.keywords = utils.transformBidderParamKeywords(bid.params.keywords);
let keywords = utils.transformBidderParamKeywords(bid.params.keywords);

if (keywords.length > 0) {
keywords.forEach(function(keyPairObj) {
if (utils.isArray(keyPairObj.value) && keyPairObj.value.length > 0 && keyPairObj.value[0] === '') {
delete keyPairObj.value;
}
});
}
tag.keywords = keywords;
}

if (bid.mediaType === NATIVE || utils.deepAccess(bid, `mediaTypes.${NATIVE}`)) {
Expand Down
2 changes: 1 addition & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1086,7 +1086,7 @@ export function transformBidderParamKeywords(keywords, paramName = 'keywords') {
let values = [];
exports._each(v, (val) => {
val = exports.getValueString(paramName + '.' + k, val);
if (val) { values.push(val); }
if (val || val === '') { values.push(val); }
});
v = values;
} else {
Expand Down
6 changes: 6 additions & 0 deletions test/spec/modules/appnexusBidAdapter_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ describe('AppNexusAdapter', function () {
singleArrNum: [5],
multiValMixed: ['value1', 2, 'value3'],
singleValNum: 123,
emptyStr: '',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if this is valid use case but what will happen if someone adds emptyStr: ' ' (space is added as value).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the space character is treated as the value for the key-pair and is preserved in the ut request.

eg "keywords":[{"key":"color","value":["red"]},{"key":"mytest"},{"key":"myspacetest","value":[" "]}]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As a follow-up to the above, the treatment of the space character has not been changed through this PR. Using a space character as a keyvalue in master would result in the same type of object for the ut request as noted above.

emptyArr: [''],
badValue: {'foo': 'bar'} // should be dropped
}
}
Expand All @@ -285,6 +287,10 @@ describe('AppNexusAdapter', function () {
}, {
'key': 'singleValNum',
'value': ['123']
}, {
'key': 'emptyStr'
}, {
'key': 'emptyArr'
}]);
});

Expand Down
69 changes: 69 additions & 0 deletions test/spec/utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -846,4 +846,73 @@ describe('Utils', function () {
expect(sizes).to.deep.equal([[300, 250], [300, 600]]);
});
});

describe('transformBidderParamKeywords', function () {
it('returns an array of objects when keyvalue is an array', function () {
let keywords = {
genre: ['rock', 'pop']
};
let result = utils.transformBidderParamKeywords(keywords);
expect(result).to.deep.equal([{
key: 'genre',
value: ['rock', 'pop']
}]);
});

it('returns an array of objects when keyvalue is a string', function () {
let keywords = {
genre: 'opera'
};
let result = utils.transformBidderParamKeywords(keywords);
expect(result).to.deep.equal([{
key: 'genre',
value: ['opera']
}]);
});

it('returns an array of objects when keyvalue is a number', function () {
let keywords = {
age: 15
};
let result = utils.transformBidderParamKeywords(keywords);
expect(result).to.deep.equal([{
key: 'age',
value: ['15']
}]);
});

it('returns an array of objects when using multiple keys with values of differing types', function () {
let keywords = {
genre: 'classical',
mix: ['1', 2, '3', 4],
age: 10
};
let result = utils.transformBidderParamKeywords(keywords);
expect(result).to.deep.equal([{
key: 'genre',
value: ['classical']
}, {
key: 'mix',
value: ['1', '2', '3', '4']
}, {
key: 'age',
value: ['10']
}]);
});

it('returns an array of objects when the keyvalue uses an empty string', function() {
let keywords = {
test: [''],
test2: ''
};
let result = utils.transformBidderParamKeywords(keywords);
expect(result).to.deep.equal([{
key: 'test',
value: ['']
}, {
key: 'test2',
value: ['']
}]);
});
});
});