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

MinuteMedia: Add new bidder #3019

Merged
merged 6 commits into from
Mar 1, 2024
Merged

MinuteMedia: Add new bidder #3019

merged 6 commits into from
Mar 1, 2024

Conversation

Net-burst
Copy link
Collaborator

No description provided.

@Net-burst Net-burst marked this pull request as ready for review February 27, 2024 20:16
public void openrtb2AuctionShouldRespondWithBidsFromMinuteMedia() throws IOException, JSONException {
// given

WIRE_MOCK_RULE.stubFor(post(urlPathEqualTo("/minutemedia-exchange"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

As you added query param to test application props pls add withQueryParam check

@@ -0,0 +1,9 @@
package org.prebid.server.bidder.minutemedia.model;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be part of proto package

Comment on lines 116 to 119
} else if (imp.getXNative() != null) {
return BidType.xNative;
} else if (imp.getAudio() != null) {
return BidType.audio;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls cover native and banner with unit tests

}

private static BidType getBidType(Bid bid, List<Imp> imps) {
for (Imp imp : imps) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

From go they use mtype check

final String orgId;

try {
orgId = extractFirstImpOrdId(bidRequest.getImp());
Copy link
Collaborator

Choose a reason for hiding this comment

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

They are checking each imp for org, not just first

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only first. They iterate over request.imp object, but they return right away.

func extractOrg(openRTBRequest *openrtb2.BidRequest) (string, error) {
	var err error
	for _, imp := range openRTBRequest.Imp {
		var bidderExt adapters.ExtImpBidder
		if err = json.Unmarshal(imp.Ext, &bidderExt); err != nil {
			return "", fmt.Errorf("failed to unmarshal bidderExt: %w", err)
		}

		var impExt openrtb_ext.ImpExtMinuteMedia
		if err = json.Unmarshal(bidderExt.Bidder, &impExt); err != nil {
			return "", fmt.Errorf("failed to unmarshal ImpExtMinuteMedia: %w", err)
		}

		return strings.TrimSpace(impExt.Org), nil
	}

	return "", errors.New("no imps in bid request")
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't say that I like how it's done, but there was a discussion about it and they made an explicit decision to not do any kind of validation for this field...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree, org is the required field and always will be here

@SerhiiNahornyi SerhiiNahornyi merged commit 28ae7cf into master Mar 1, 2024
3 checks passed
@SerhiiNahornyi SerhiiNahornyi deleted the bidder/minute-media branch March 1, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port PR from PBS-Go: MinuteMedia: Add GPP macros Port PR from PBS-Go: New adapter: MinuteMedia
2 participants