Yahoo Ads Bid Adapter: Fix to not set bidResponse vastUrl field with bid nurl value

[zach-bowman]

Type of change

• Bugfix
• Updated bidder adapter

Description of change

This PR fixes undesired behavior within the YahooAdsBidAdapter (per Yahoo business/product) which incorrectly sets the bidResponse.vastUrl field with bid.nurl value. YahooAdsBidAdapter is updated to remove the setting of the bidResponse.vastUrl field as well as removes the associated test. At present [with this change], the YahooAdsBidAdapter should not consume the nurl in any case nor provide creative responses in the nurl field.

Other information

Contact Email: [email protected]

[github-actions]

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

• modules/adtrgtmeBidAdapter.js has 36 duplicated lines with modules/yahooAdsBidAdapter.js

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. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀

[patmmccann]

Cc @slimkrazy @radubarbos

[github-actions]

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

• modules/adtrgtmeBidAdapter.js has 36 duplicated lines with modules/yahooAdsBidAdapter.js

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. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀

[radubarbos]

LGTM

[zach-bowman]

Hi all, I just wanted to ask about the failing "Check for Duplicated Code" item above -- since it appears that some of the logic we have is now used by another adapter -- should I still extract the duplicated code? I just wanted to make sure this is the appropriate path forward as we would then share our intended specific logic across adapters (which may change). Happy to do so if this is the case (new to contributing, so just wanted to make sure). Same with #12139 . Thanks in advance for the input!

[zach-bowman]

Hi reviewers, I just wanted to followup on the above question. In short: as I attempted to address the failing check, it seemed incorrect to remove that logic from our adapter as well as touch another adapter -- as this should be specific and subject to change per company. Let me know your suggestion on how to best address this so we can get this change out. Thanks so much in advance!

[zach-bowman]

Hi reviewers, I just wanted to followup on the above question. In short: as I attempted to address the failing check, it seemed incorrect to remove that logic from our adapter as well as touch another adapter -- as this should be specific and subject to change per company. Let me know your suggestion on how to best address this so we can get this change out. Thanks so much in advance!

Hi @Fawke (cc @ChrisHuie / @jsnellbaker / @patmmccann) -- just wanted to followup to see if there's any advice on the above to get this PR through -- thanks again! New to the project for Yahoo, and just wanted to follow protocol to get this resolved (sorry for the extra pings)

[zach-bowman]

Update: looks like the other PR was merged.

[patmmccann]

• since it appears that some of the logic we have is now used by another adapter -- should I still extract the duplicated code? I just wanted to make sure this is the appropriate path forward as we would then share our intended specific logic across adapters

This would be much appreciated, we're hoping to improve maintainability and bundle size!

[zach-bowman]

Thank you!