-
Notifications
You must be signed in to change notification settings - Fork 224
Core: Support seatnonbid Codes in Modules #3762
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
Conversation
…d-codes-in-modules # Conflicts: # extra/modules/greenbids-real-time-data/src/main/java/org/prebid/server/hooks/modules/greenbids/real/time/data/v1/GreenbidsRealTimeDataProcessedAuctionRequestHook.java
…dules # Conflicts: # extra/modules/greenbids-real-time-data/src/main/java/org/prebid/server/hooks/modules/greenbids/real/time/data/config/GreenbidsRealTimeDataConfiguration.java # extra/modules/greenbids-real-time-data/src/main/java/org/prebid/server/hooks/modules/greenbids/real/time/data/core/GreenbidsInvocationService.java # extra/modules/ortb2-blocking/src/test/java/org/prebid/server/hooks/modules/ortb2/blocking/core/BidsBlockerTest.java # extra/modules/pb-richmedia-filter/src/test/java/org/prebid/server/hooks/modules/pb/richmedia/filter/core/BidResponsesMraidFilterTest.java # src/main/java/org/prebid/server/analytics/reporter/greenbids/GreenbidsAnalyticsReporter.java # src/main/java/org/prebid/server/auction/BidResponseCreator.java # src/main/java/org/prebid/server/auction/model/BidRejectionTracker.java # src/test/java/org/prebid/server/auction/BidResponseCreatorTest.java # src/test/java/org/prebid/server/auction/ExchangeServiceTest.java # src/test/java/org/prebid/server/auction/model/BidRejectionTrackerTest.java
...rver/hooks/modules/pb/richmedia/filter/v1/PbRichmediaFilterAllProcessedBidResponsesHook.java
Outdated
Show resolved
Hide resolved
| return CollectionUtils.isEmpty(accounts) || accounts.contains(account); | ||
| } | ||
|
|
||
| //todo: should it be more strict? e.g. allowing rejecting only imps/bids on the particular stages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public Map<String, List<Rejected>> getAllRejected() { | ||
| final Map<String, List<Rejected>> missingImpIds = new HashMap<>(); | ||
| for (String impId : involvedImpIds) { | ||
| final Set<String> succeededBids = succeededBidsIds.getOrDefault(impId, Collections.emptySet()); | ||
| if (succeededBids.isEmpty() && !rejectedBids.containsKey(impId)) { | ||
| missingImpIds.computeIfAbsent(impId, key -> new ArrayList<>()) | ||
| .add(Pair.of(null, BidRejectionReason.NO_BID)); | ||
| .add(RejectedImp.of(impId, BidRejectionReason.NO_BID)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is only used in tests. Please, remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's used in the analytics as well
...server/hooks/modules/greenbids/real/time/data/core/GreenbidsInvocationResultCreatorTest.java
Outdated
Show resolved
Hide resolved
...va/org/prebid/server/hooks/modules/greenbids/real/time/data/util/TestBidRequestProvider.java
Show resolved
Hide resolved
src/main/java/org/prebid/server/auction/model/BidRejectionTracker.java
Outdated
Show resolved
Hide resolved
| @Value(staticConstructor = "of") | ||
| public class RejectedBid implements Rejected { | ||
|
|
||
| BidderBid bid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to store whole bid meta, can be simplified to:
@Accessors(fluent = true)
@Value(staticConstructor = "of")
public class RejectedBid implements Rejected {
String bidId;
String impId;
BidRejectionReason reason;
public static RejectedBid of(BidderBid bid, BidRejectionReason reason) {
return of(bid.getBid().getId(), bid.getBid().getImpid(), reason);
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole bid might be needed for the analytics, because usually if a bid is rejected it's removed from the BidResponse, so this way the rejected bid is preserved.
🔧 Type of changes
✨ What's the context?
What's the context for the changes?
🧠 Rationale behind the change
Why did you choose to make these changes? Were there any trade-offs you had to consider?
🔎 New Bid Adapter Checklist
🧪 Test plan
How do you know the changes are safe to ship to production?
🏎 Quality check