Missena: Update params #4057
Open
Missena: Update params #4057
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
ysfbsf
reviewed
Nov 19, 2024
static/bidder-info/missena.yaml
Outdated
| url: https://sync.missena.io/iframe?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&t={{.APIKey}}&redirect={{.RedirectURL}} |
There was a problem hiding this comment.
Suggested change
| url: https://sync.missena.io/iframe?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&t={{.APIKey}}&redirect={{.RedirectURL}} | |
| url: https://sync.missena.io/iframe?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redirect={{.RedirectURL}} |
{{.APIKey}} is not a supported macro in Prebid Server
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
SyntaxNode
reviewed
Nov 20, 2024
| url: https://sync.missena.io/iframe?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&t=YOUR_API_KEY&redirect={{.RedirectURL}} |
There was a problem hiding this comment.
Is an api key from Missena now required for user syncing?
There was a problem hiding this comment.
Thank you for confirming. It's not appropriate to define user syncing which does not work out of the box. Please update this config to indicate that user syncing is possible, but requires configuring with a Missena provided api key. To signal availability of user syncing to host companies, please use the supports attribute documented here.
You're welcomed to keep the url and usermacro definitions if they're commented out.
SyntaxNode
reviewed
Nov 20, 2024
adapters/missena/missena.go
Outdated
| } | ||
| currency, err := getCurrency(request.Cur) | ||
| if err != nil { | ||
| // TODO: convert unsupported currency on response |
There was a problem hiding this comment.
Please resolve the TODO.
There was a problem hiding this comment.
I'd actually like to manage it on a next release. is it possible? should I remove the comment and consider some currencies unsupported
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
Code coverage summaryNote:
missenaRefer here for heat map coverage report |
SyntaxNode
reviewed
Dec 13, 2024
| url: https://sync.missena.io/iframe?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&t=YOUR_API_KEY&redirect={{.RedirectURL}} |
There was a problem hiding this comment.
Thank you for confirming. It's not appropriate to define user syncing which does not work out of the box. Please update this config to indicate that user syncing is possible, but requires configuring with a Missena provided api key. To signal availability of user syncing to host companies, please use the supports attribute documented here.
You're welcomed to keep the url and usermacro definitions if they're commented out.
| body, errm := json.Marshal(missenaRequest) | ||
| if errm != nil { | ||
| return nil, errm | ||
| body, err := json.Marshal(missenaRequest) |
There was a problem hiding this comment.
Please use jsonutil.Marshal as a drop in replacement for json.Marshal.
| } | ||
| return bidder, nil | ||
| } | ||
|
|
||
| func (a *adapter) makeRequest(missenaParams MissenaInternalParams, reqInfo *adapters.ExtraRequestInfo, impID string, request *openrtb2.BidRequest) (*adapters.RequestData, error) { | ||
| url := a.endpoint + "?t=" + missenaParams.ApiKey | ||
| func getCurrency(currencies []string) (string, error) { |
There was a problem hiding this comment.
I think is better to use MAP:
currencySet := make(map[string]struct{}, len(currencies))
for _, cur := range currencies {
currencySet[cur] = struct{}{}
}
if _, found := currencySet[defaultCur]; found {
return defaultCur, nil
}
if _, found := currencySet["EUR"]; found {
return "EUR", nil
}
return "", fmt.Errorf("no currency supported %v", currencies)
}```
what Do You think?
| @@ -124,26 +209,20 @@ func (a *adapter) MakeRequests(request *openrtb2.BidRequest, requestInfo *adapte | |||
| continue | |||
There was a problem hiding this comment.
NIP:
Message: fmt.Sprintf("Error parsing bidderExt object: %v, input: %s", err, string(imp.Ext)),
| httpRequests = append(httpRequests, newHttpRequest) | ||
|
|
||
| break |
There was a problem hiding this comment.
break in the loop causes only the first imp to be processed. If this is intentional, it's worth making it clearer in the code, e.g., with a comment or better naming.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.