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

viantOrtbBidAdapter': deals support #11864

Merged

Conversation

skapoor-viant
Copy link
Contributor

@skapoor-viant skapoor-viant commented Jun 25, 2024

Type of change

  • Feature

Description of change

Added pmp deals json in final request created by viantOrtbBidAdapter.

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

@patmmccann
Copy link
Collaborator

This is very interesting, please also support the standard location on the imp object though

@patmmccann patmmccann marked this pull request as draft June 25, 2024 23:52
@skapoor-viant skapoor-viant marked this pull request as ready for review June 26, 2024 00:03
@skapoor-viant
Copy link
Contributor Author

This is very interesting, please also support the standard location on the imp object though

@patmmccann I fixed the rest code changes from my side, but please let me know if there's something from my side I would need to do for duplicated code check?

@patmmccann
Copy link
Collaborator

Duplicated code very minor, safe to ignore or fix if you like and prefer to not have the warning, I marked draft because you didn't have any tests and you didn't support the standard location for the deals object on the ortb2Imp part of the request

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

@patmmccann
Copy link
Collaborator

patmmccann commented Jun 26, 2024

Adding some detail here, a publisher should be able to define a pmp like this https://docs.prebid.org/dev-docs/adunit-reference.html#first-party-data ; not just via your adapter parameters. It is a requirement you support the standard openrtb way of forming a bid request when you provide a parameter. The most frequent publisher complaint about prebid is bidder parameters having redundant implementations of the same information

   code: "test-div",
   mediaTypes: {
       banner: {
           sizes: [[300,250]]
       }
   },
   ortb2Imp: {
            "pmp": {
               "private_auction": 0,
               "deals": [
                   {
                       "id": "5678",
                       "bidfloor": 1.75,
                       "bidfloorcur": "USD",
                       "wseat": [
                           "16"
                       ],
                       "at": 3
                   },
                   {
                       "id": "1234",
                       "bidfloor": 2,
                       "bidfloorcur": "USD",
                       "wseat": [
                           "3539"
                       ],
                       "at": 1
                   },
               .....
   // ...
});`````

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

@skapoor-viant
Copy link
Contributor Author

skapoor-viant commented Jun 26, 2024

Adding some detail here, a publisher should be able to define a pmp like this https://docs.prebid.org/dev-docs/adunit-reference.html#first-party-data ; not just via your adapter parameters. It is a requirement you support the standard openrtb way of forming a bid request when you provide a parameter. The most frequent publisher complaint about prebid is bidder parameters having redundant implementations of the same information

   code: "test-div",
   mediaTypes: {
       banner: {
           sizes: [[300,250]]
       }
   },
   ortb2Imp: {
            "pmp": {
               "private_auction": 0,
               "deals": [
                   {
                       "id": "5678",
                       "bidfloor": 1.75,
                       "bidfloorcur": "USD",
                       "wseat": [
                           "16"
                       ],
                       "at": 3
                   },
                   {
                       "id": "1234",
                       "bidfloor": 2,
                       "bidfloorcur": "USD",
                       "wseat": [
                           "3539"
                       ],
                       "at": 1
                   },
               .....
   // ...
});`````

@patmmccann I am doing the changes based on your comment, but I do see the same json which I used is also being used by pubmatic adapter. Please check the link: pubmatic
Is prebid enforcing this change for all the adapters to standardize the adUnits?

@skapoor-viant
Copy link
Contributor Author

@patmmccann I have made the changes and added test case for that. Please review it.

Copy link

Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:

Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. Keep up the great work! 🚀

@skapoor-viant
Copy link
Contributor Author

@patmmccann Could you please review it again at your earliest convenience? It is currently a blocker for our team and needs to be merged as soon as possible.

@patmmccann
Copy link
Collaborator

@pm-harshad-mane @jlquaccia @pm-manasi-moghe please see discussion above

@patmmccann patmmccann changed the title Added deals support in viantOrtbBidAdapter viantOrtbBidAdapter': deals support Jun 28, 2024
@patmmccann
Copy link
Collaborator

@skapoorviant looks great! Please document

@patmmccann patmmccann merged commit 75e3375 into prebid:master Jun 28, 2024
4 of 5 checks passed
@patmmccann patmmccann self-assigned this Jun 28, 2024
DecayConstant pushed a commit to mediavine/Prebid.js that referenced this pull request Jul 18, 2024
* Added deals support in viantOrtbBidAdapter

* refactoring

* refactor

* refactor

* refactor

* refactor

* added test case

* refactor

* refactor test file

* added test case and addressed suggestions
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.

2 participants