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

Adview: adapter currency fix. #2060

Merged
merged 19 commits into from
Nov 23, 2021
Merged

Adview: adapter currency fix. #2060

merged 19 commits into from
Nov 23, 2021

Conversation

AdviewOpen
Copy link
Contributor

for the suggestion, fixed the response currency setting.

adapters/adview/adview.go Outdated Show resolved Hide resolved
adapters/adview/adview.go Show resolved Hide resolved
@SyntaxNode SyntaxNode changed the title adapter currency fix. Adview: adapter currency fix. Oct 27, 2021
(2)add test codes for currency.
AlexBVolcy
AlexBVolcy previously approved these changes Nov 1, 2021
Copy link
Contributor

@AlexBVolcy AlexBVolcy left a comment

Choose a reason for hiding this comment

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

LGTM

guscarreon
guscarreon previously approved these changes Nov 5, 2021
Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

LGTM.

adapters/adview/adview.go Show resolved Hide resolved
@mansinahar
Copy link
Contributor

Needs #2071 to be merged in before we can merge this one in

@mansinahar mansinahar assigned AlexBVolcy and unassigned mansinahar Nov 16, 2021
AlexBVolcy
AlexBVolcy previously approved these changes Nov 16, 2021
Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

Looks good to me @AdviewOpen. Just one more ask: Now that pull request #2071 has been merged, can you please re-incorporate the JSON currency conversion test case you got rid of in a previous commit? JSON tests with currency conversion are now supported. As shown below, file adapters/adview/adviewtest/supplemental/currency_rate_not_found.json covers the error scenario in line 67.

Can we also add a JSON currency test to cover lines 71 and 72?
image

@AdviewOpen
Copy link
Contributor Author

Roger , and test script has been already in git, great thx for all of you.

Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

LGTM. @AdviewOpen we appreciate your response to our feedback

@guscarreon guscarreon merged commit 7270c82 into prebid:master Nov 23, 2021
wwwyyy pushed a commit to wwwyyy/prebid-server that referenced this pull request Dec 20, 2021
* 'master' of https://github.com/wwwyyy/prebid-server: (23 commits)
  Add GPID Reserved Bidder Name (prebid#2107)
  Marsmedia adapter - param zoneId can get string and int (prebid#2105)
  Algorix: add Server Region Support (prebid#2096)
  New Adapter: Bizzclick (prebid#2056)
  Collect Prometheus Go process metrics (prebid#2101)
  Added channel support (prebid#2037)
  Rubicon: update fpd fields resolving (prebid#2091)
  Beachfront - Currency conversion (prebid#2078)
  New Adapter: Groupm (Alias Of PubMatic) (prebid#2042)
  Adform adapter lacked gross/net parameter support (prebid#2084)
  add iframe sync for openx (prebid#2094)
  changes to the correct user sync url (prebid#2092)
  Smaato: Add instl to tests (prebid#2089)
  Correct MakeRequests() output assertion in adapter JSON tests (prebid#2087)
  Adview: adapter currency fix. (prebid#2060)
  New Adapter: Coinzilla (prebid#2074)
  Syncer Level ExternalURL Override (prebid#2072)
  33Across: Enable Support for SRA requests (prebid#2079)
  Currency conversion on adapter JSON tests (prebid#2071)
  New Adapter: VideoByte (prebid#2058)
  ...
jorgeluisrocha pushed a commit to jwplayer/prebid-server that referenced this pull request Sep 28, 2022
Co-authored-by: wildcat0601 <wildcat.wang@qq.com>
shunj-nb pushed a commit to ParticleMedia/prebid-server that referenced this pull request Nov 8, 2022
Co-authored-by: wildcat0601 <wildcat.wang@qq.com>
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.

None yet

5 participants