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.

Sign up for GitHub

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

calling for currency file even when default rates are specified #3470

Merged
merged 3 commits into from
Jan 22, 2019
Merged

calling for currency file even when default rates are specified #3470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b362d43
calling for currency file even when default rates are specified
bretg Jan 22, 2019
aaa1f42
fixed broken unit tests
Jan 22, 2019
4ca394a
added unit test to check that currency file is still requested even w…
Jan 22, 2019
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
7 changes: 6 additions & 1 deletion modules/currency.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const CURRENCY_RATE_PRECISION = 4;
var bidResponseQueue = [];
var conversionCache = {};
var currencyRatesLoaded = false;
var needToCallForCurrencyFile = true;
Copy link
Member

Choose a reason for hiding this comment

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

global state is a bad practice and should be refactored but I won't block the PR as it's already all over this module...

var adServerCurrency = 'USD';

export var currencySupportEnabled = false;
Expand Down Expand Up @@ -56,6 +57,7 @@ export function setConfig(config) {
if (typeof config.rates === 'object') {
currencyRates.conversions = config.rates;
currencyRatesLoaded = true;
needToCallForCurrencyFile = false; // don't call if rates are already specified
}

if (typeof config.defaultRates === 'object') {
Expand Down Expand Up @@ -122,7 +124,9 @@ function initCurrency(url) {

hooks['addBidResponse'].addHook(addBidResponseHook, 100);

if (!currencyRates.conversions) {
// call for the file if we haven't already
if (needToCallForCurrencyFile) {
needToCallForCurrencyFile = false;
ajax(url,
{
success: function (response) {
Expand Down Expand Up @@ -150,6 +154,7 @@ function resetCurrency() {
conversionCache = {};
currencySupportEnabled = false;
currencyRatesLoaded = false;
needToCallForCurrencyFile = true;
currencyRates = {};
bidderCurrencyDefault = {};
}
Expand Down
40 changes: 40 additions & 0 deletions test/spec/modules/currency_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,33 @@ describe('currency', function () {
expect(currencySupportEnabled).to.equal(true);
});

it('currency file is called even when default rates are specified', function() {
// RESET to request currency file (specifically url value for this test)
setConfig({ 'adServerCurrency': undefined });

// DO NOT SET DEFAULT RATES, currency file should be requested
setConfig({
'adServerCurrency': 'JPY'
});
fakeCurrencyFileServer.respond();
expect(fakeCurrencyFileServer.requests.length).to.equal(1);
expect(fakeCurrencyFileServer.requests[0].url).to.equal('https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json?date=20030306');

// RESET to request currency file (specifically url value for this test)
setConfig({ 'adServerCurrency': undefined });

// SET DEFAULT RATES, currency file should STILL be requested
setConfig({
'adServerCurrency': 'JPY',
'defaultRates': {
'GBP': { 'CNY': 66, 'JPY': 132, 'USD': 264 },
'USD': { 'CNY': 60, 'GBP': 120, 'JPY': 240 }
} });
fakeCurrencyFileServer.respond();
expect(fakeCurrencyFileServer.requests.length).to.equal(2);
expect(fakeCurrencyFileServer.requests[1].url).to.equal('https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json?date=20030306');
});

it('date macro token $$TODAY$$ is replaced by current date (formatted as yyyymmdd)', function () {
// RESET to request currency file (specifically url value for this test)
setConfig({ 'adServerCurrency': undefined });
Expand All @@ -64,6 +91,9 @@ describe('currency', function () {
fakeCurrencyFileServer.respond();
expect(fakeCurrencyFileServer.requests[0].url).to.equal('https://cdn.jsdelivr.net/gh/prebid/currency-file@1/latest.json?date=20030306');

// RESET to request currency file (specifically url value for this test)
setConfig({ 'adServerCurrency': undefined });

// date macro should not modify 'conversionRateFile' if TOKEN is not found
setConfig({
'adServerCurrency': 'JPY',
Expand All @@ -72,6 +102,9 @@ describe('currency', function () {
fakeCurrencyFileServer.respond();
expect(fakeCurrencyFileServer.requests[1].url).to.equal('http://test.net/currency.json?date=foobar');

// RESET to request currency file (specifically url value for this test)
setConfig({ 'adServerCurrency': undefined });

// date macro should replace $$TODAY$$ with date for 'conversionRateFile' is configured
setConfig({
'adServerCurrency': 'JPY',
Expand All @@ -80,6 +113,9 @@ describe('currency', function () {
fakeCurrencyFileServer.respond();
expect(fakeCurrencyFileServer.requests[2].url).to.equal('http://test.net/currency.json?date=20030306');

// RESET to request currency file (specifically url value for this test)
setConfig({ 'adServerCurrency': undefined });

// MULTIPLE TOKENS used in a url is not supported. Only the TOKEN at left-most position is REPLACED
setConfig({
'adServerCurrency': 'JPY',
Expand Down Expand Up @@ -220,6 +256,7 @@ describe('currency', function () {

it('should result in NO_BID when currency support is not enabled and fromCurrency is not USD', function () {
setConfig({});

var bid = { 'cpm': 1, 'currency': 'GBP' };
var innerBid;
addBidResponseHook('elementId', bid, function(adCodeId, bid) {
Expand All @@ -241,6 +278,9 @@ describe('currency', function () {
});

it('should result in NO_BID when fromCurrency is not supported in file', function () {
// RESET to request currency file
setConfig({ 'adServerCurrency': undefined });

fakeCurrencyFileServer.respondWith(JSON.stringify(getCurrencyRates()));
setConfig({ 'adServerCurrency': 'JPY' });
fakeCurrencyFileServer.respond();
Expand Down