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

New Adapter: Vidazoo #3698

Merged
merged 7 commits into from
Jul 1, 2024
Merged

Conversation

saar120
Copy link
Contributor

@saar120 saar120 commented May 22, 2024

No description provided.

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, ad2b2ce

vidazoo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:22:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:29:	MakeRequests		69.2%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:60:	getMediaTypeForBid	75.0%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:75:	MakeBids		81.8%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:123:	extractCid		70.0%
total:									(statements)		76.4%

endpoint: "https://prebidsrvr.cootlogix.com/openrtb/"
maintainer:
email: "dev@vidazoo.com"
gvlVendorID: 744
Copy link
Contributor

Choose a reason for hiding this comment

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

Confirmed GVL ID is for Vidazoo.

@SyntaxNode SyntaxNode changed the title Vidazoo Adapter New Adapter: Vidazoo May 22, 2024
adapters/vidazoo/vidazoo.go Outdated Show resolved Hide resolved
if impExt.ConnectionId != "" {
return strings.TrimSpace(impExt.ConnectionId), nil
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this effectively use the cid from the first impression only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use imp splitting as suggested in the docs

adapters/vidazoo/vidazoo.go Outdated Show resolved Hide resolved
adapters/vidazoo/vidazoo.go Outdated Show resolved Hide resolved
@@ -0,0 +1,18 @@
endpoint: "https://prebidsrvr.cootlogix.com/openrtb/"
Copy link
Contributor

Choose a reason for hiding this comment

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

endpoint is reachable

curl -i --location --request POST 'https://prebidsrvr.cootlogix.com/openrtb/'
HTTP/2 404
access-control-allow-origin: *
cache-control: max-age=0, no-cache, must-revalidate, proxy-revalidate
access-control-allow-credentials: true
access-control-allow-headers: Origin, X-Requested-With, Content-Type, Accept, Authorization, Content-Range, Cache-Control
content-type: application/json; charset=utf-8
content-length: 52

@@ -0,0 +1,18 @@
endpoint: "https://prebidsrvr.cootlogix.com/openrtb/"
maintainer:
email: "dev@vidazoo.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

@saar120 email is sent from Prebid team to verify above contact. Requesting to provide feedback on email thread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

replied

@onkarvhanumante
Copy link
Contributor

@saar120 consider adding json tests for following scenarios:

  • request with multiple imps
  • multi-format imp request

Comment on lines 15 to 18
userSync:
iframe:
url: https://sync.cootlogix.com/api/user/html/pbs_sync?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&us_privacy={{.USPrivacy}}&redirect={{.RedirectURL}}
userMacro: "${userId}"
Copy link
Contributor

Choose a reason for hiding this comment

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

above url is not redirecting or replacing userMacro with user id

https://sync.cootlogix.com/api/user/html/pbs_sync?gdpr=&gdpr_consent=&us_privacy=&redirect=https%3A%2F%2Fprebid.adnxs.com%2Fpbs%2Fv1%2Fsetuid%3Fbidder%3Dcootlogix%26gdpr%3D%26gdpr_consent%3D%26f%3Di%26uid%3D%22%24%7BuserId%7D%22

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@onkarvhanumante
It will return HTML code with the redirect as a pixel.
Did you send it from the EU? maybe it was blocked by gdpr

Copy link
Contributor

@gargcreation1992 gargcreation1992 Jun 17, 2024

Choose a reason for hiding this comment

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

I tried this in APAC region. AFAIK, This is not the expected behaviour from iframe user sync url. You would still need to return the redirect URL with proper userID. @saar120

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gargcreation1992
Fixed - will now return HTML with the redirect and useID.
Can you please check?

Copy link
Contributor

Choose a reason for hiding this comment

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

image

user id was not appended for
https://sync.cootlogix.com/api/user/html/pbs_sync?gdpr=&gdpr_consent=&us_privacy=&redirect=https%3A%2F%2Fprebid.adnxs.com%2Fpbs%2Fv1%2Fsetuid%3Fbidder%3Dcootlogix%26gdpr%3D%26gdpr_consent%3D%26f%3Di%26uid%3D%22%24%7BuserId%7D%22

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@onkarvhanumante Updated the vidazoo.yaml - the userMacro should be without quotes.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://sync.cootlogix.com/api/user/html/pbs_sync?gdpr=&gdpr_consent=&us_privacy=&redirect=https%3A%2F%2Fprebid.adnxs.com%2Fpbs%2Fv1%2Fsetuid%3Fbidder%3Dcootlogix%26gdpr%3D%26gdpr_consent%3D%26f%3Di%26uid%3D%24%7BuserId%7D

iframe url is working

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, 7d64a63

vidazoo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:22:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:29:	MakeRequests		77.8%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:67:	getMediaTypeForBid	66.7%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:83:	MakeBids		94.4%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:123:	extractCid		71.4%
total:									(statements)		82.4%

@saar120
Copy link
Contributor Author

saar120 commented May 23, 2024

@saar120 consider adding json tests for following scenarios:

  • request with multiple imps
  • multi-format imp request

Added test for multiple imps
Multi-format is not supported at the moment

@onkarvhanumante
Copy link
Contributor

@saar120 requesting to raise docs PR. Refer prebid/prebid.github.io#5331 as example

@saar120
Copy link
Contributor Author

saar120 commented May 25, 2024

@saar120 requesting to raise docs PR. Refer prebid/prebid.github.io#5331 as example

@onkarvhanumante
Added to prebid docs (PR)
Please let me know if anything else is needed

@assa-kariv
Copy link

@onkarvhanumante can you please review again?

@saar120
Copy link
Contributor Author

saar120 commented Jun 4, 2024

@onkarvhanumante @SyntaxNode
Hey,
Is there something else needs to be done for this to be merged?
Can you please help?

@assa-kariv
Copy link

@gargcreation1992 @Sonali-More-Xandr @onkarvhanumante @SyntaxNode can someone please assist in completing the review?

@assa-kariv
Copy link

@Enigo @bsardo

@assa-kariv
Copy link

@assa-kariv
Copy link

@Vungle-GordonTian @dmitris please assist

@assa-kariv
Copy link

@gargcreation1992 please assist

@onkarvhanumante
Copy link
Contributor

@saar120 thank you for keeping patience with PR review. Added few comments. Requesting to provide feedback on them

Remove unsupported media types
Added protection in case of empty response currency
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, 0bd805b

vidazoo

Refer here for heat map coverage report

github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:22:	Builder			100.0%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:29:	MakeRequests		77.8%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:67:	getMediaTypeForBid	100.0%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:79:	MakeBids		89.5%
github.com/prebid/prebid-server/v2/adapters/vidazoo/vidazoo.go:122:	extractCid		71.4%
total:									(statements)		84.0%

@saar120
Copy link
Contributor Author

saar120 commented Jun 25, 2024

@saar120 thank you for keeping patience with PR review. Added few comments. Requesting to provide feedback on them

I've updated it as suggested.

@bsardo bsardo assigned bsardo and unassigned Sonali-More-Xandr Jun 28, 2024
@gargcreation1992 gargcreation1992 merged commit 09a3dd4 into prebid:master Jul 1, 2024
5 checks passed
mefjush pushed a commit to adhese/prebid-server that referenced this pull request Jul 19, 2024
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

7 participants