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

modify about currency #3

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

Takamasa-Murano
Copy link

Type of change

  • Bugfix

  • Feature

  • New bidder adapter

  • Updated bidder adapter

  • Code style update (formatting, local variables)

  • Refactoring (no functional changes, no api changes)

  • Build related changes

  • CI related changes

  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

  • Other

Description of change

To respond to the point of view from Prebid.js, I modify about currency in isBidRequestValid fuction to use config.getConfig('currency.adServerCurrency')

Other information

const currencyType = config.getConfig('currency.adServerCurrency');
if (typeof currencyType === 'string' && ALLOWED_CURRENCIES.indexOf(currencyType) === -1) {
utils.logError('Invalid currency type, we support only JPY and USD!');
return false;
Copy link
Author

Choose a reason for hiding this comment

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

Do you need to add a currency param instead of using the adserverCurrency as you do in your dsp adapter ?

adserverCurrencyの使用で、リクエストしている貨幣種類を返してくれるとのことなので、こちらを使用するように修正

(ref: https://github.com/prebid/Prebid.js/blob/master/modules/dsp_genieeBidAdapter.js#L74

@@ -84,16 +83,18 @@ describe('ssp_genieeBidAdapter', function () {
});

it('should return true when params.zoneId and params.currency exist and params.currency is JPY or USD', function () {
config.setConfig({ currency: { adServerCurrency: 'JPY' } });
Copy link
Author

Choose a reason for hiding this comment

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

リクエストする貨幣種類のセットのためにconfig.setConfigを使用する形に修正

]);
expect(String(request[0].data.ib)).to.have.string('0');
expect(String(request[1].data.ib)).to.have.string('');
expect(String(request[1].data.ib)).to.have.string('0');
expect(String(request[2].data.ib)).to.have.string('0');
Copy link
Author

Choose a reason for hiding this comment

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

検証忘れ

data.ibについては常に0を返す形にしていたので

Copy link

@daikichiteranishi daikichiteranishi left a comment

Choose a reason for hiding this comment

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

LGTM

@Takamasa-Murano Takamasa-Murano merged commit df80c1c into master Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants