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

GumGum: Override the default currency #3928

Conversation

MartinGumGum
Copy link
Contributor

This PR resolves an issue where bid currency from the Ad Exchange was not being set in the Prebid Server's Go version, causing the currency to default to USD in all cases. The bid currency will now be dynamically set based on the response from the Ad Exchange, ensuring proper handling of different currencies in the bidding process.

Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, f481c0d

gumgum

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:23:	MakeRequests		87.1%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:84:	MakeBids		76.2%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:130:	preprocess		71.4%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:189:	getBiggerFormat		100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:213:	getMediaTypeForImpID	100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:223:	Builder			100.0%
total:									(statements)		82.2%

Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 30c7c78

gumgum

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:23:	MakeRequests		87.1%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:84:	MakeBids		81.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:130:	preprocess		71.4%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:189:	getBiggerFormat		100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:213:	getMediaTypeForImpID	100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:223:	Builder			100.0%
total:									(statements)		83.2%

@MartinGumGum
Copy link
Contributor Author

MartinGumGum commented Sep 26, 2024

@onkarvhanumante Could you please review?

@MartinGumGum
Copy link
Contributor Author

MartinGumGum commented Sep 26, 2024

@minaguib @dlackty @dmitris @asweeney86 Could one of you please review?

@MartinGumGum
Copy link
Contributor Author

@hhhjort Could you please review this PR?

@MartinGumGum
Copy link
Contributor Author

@bsardo Could you please review this PR?

@bsardo bsardo self-assigned this Oct 20, 2024
Comment on lines 25 to 80

func TestResponseWithCurrencies(t *testing.T) {
// Test for USD currency
assertCurrencyInBidResponse(t, "USD", "USD")

// Test for EUR currency
assertCurrencyInBidResponse(t, "EUR", "EUR")
}

func assertCurrencyInBidResponse(t *testing.T, expectedCurrency string, currency string) {
// Create a GumGum bidder
bidder, buildErr := Builder(openrtb_ext.BidderGumGum, config.Adapter{
Endpoint: "https://g2.gumgum.com/providers/prbds2s/bid"}, config.Server{
ExternalUrl: "http://hosturl.com",
GvlID: 1,
DataCenter: "2",
})

if buildErr != nil {
t.Fatalf("Builder returned unexpected error %v", buildErr)
}

// Create a mock BidRequest
prebidRequest := &openrtb2.BidRequest{
Imp: []openrtb2.Imp{},
}

// Create a mock BidResponse with or without currency
mockedBidResponse := &openrtb2.BidResponse{}
if currency != "" {
mockedBidResponse.Cur = currency
}

// Marshal the mock bid response to JSON
body, err := json.Marshal(mockedBidResponse)
if err != nil {
t.Fatalf("Failed to marshal mock bid response: %v", err)
}

// Create a mock ResponseData
responseData := &adapters.ResponseData{
StatusCode: 200,
Body: body,
}

// Call MakeBids
bidResponse, errs := bidder.MakeBids(prebidRequest, nil, responseData)

// Assert no errors
if len(errs) != 0 {
t.Fatalf("Failed to make bids %v", errs)
}

// Assert that the currency is correctly set
assert.Equal(t, expectedCurrency, bidResponse.Currency, "Expected currency to be set to "+expectedCurrency)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The guidance is to test your adapter using the JSON test framework, in which case you can get rid of all of this and instead just copy your exemplary/banner.json test into the supplemental directory, rename it to convert-currency.json and then just add "cur": "EUR" to mockResponse.body and "currency": "EUR" to the object in the expectedBidResponses array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bsardo I have addressed the above comment. Could you please review it? and thx for the review.

Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, 0a7e96e

gumgum

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:23:	MakeRequests		87.1%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:84:	MakeBids		81.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:130:	preprocess		71.4%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:189:	getBiggerFormat		100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:213:	getMediaTypeForImpID	100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:223:	Builder			100.0%
total:									(statements)		83.2%

…f github.com:MartinGumGum/prebid-server into ADTS-484-Set-bid-curency-in-Prebid-Server-GO-version
Copy link

Code coverage summary

Note:

  • Prebid team doesn't anticipate tests covering code paths that might result in marshal and unmarshal errors
  • Coverage summary encompasses all commits leading up to the latest one, ce6101b

gumgum

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:23:	MakeRequests		87.1%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:84:	MakeBids		81.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:130:	preprocess		71.4%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:189:	getBiggerFormat		100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:213:	getMediaTypeForImpID	100.0%
github.com/prebid/prebid-server/v2/adapters/gumgum/gumgum.go:223:	Builder			100.0%
total:									(statements)		83.2%

@MartinGumGum MartinGumGum requested a review from bsardo October 25, 2024 19:12
@MartinGumGum
Copy link
Contributor Author

@bsardo Could you please review this PR again? Thank you!

@bsardo bsardo changed the title GumGum: Override the default currency. GumGum: Override the default currency Nov 4, 2024
@bsardo bsardo merged commit a788661 into prebid:master Nov 4, 2024
5 checks passed
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.

3 participants