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

bidWatch Analytics Adapter : add creative endpoint #8710

Merged
merged 11 commits into from
Aug 3, 2022
Merged

bidWatch Analytics Adapter : add creative endpoint #8710

merged 11 commits into from
Aug 3, 2022

Conversation

matthieularere-msq
Copy link
Contributor

@matthieularere-msq matthieularere-msq commented Jul 19, 2022

Type of change

  • Feature

Description of change

  • Makes a dereferenced copy of events received in order to be able to modify them without altering ad delivery
  • Remove ad creative code from the payload send to our endpoint to minimize its size
  • Remove gdpr vendor data for the same reason (not needed as long as we have the consent string)
  • Add a new endpoint call dedicated to creatives which each bid response sent individually.
  • Compute cpm increment for a bid won (difference between the bid won cpm and the second best price in the same transaction)

@ChrisHuie
Copy link
Collaborator

@matthieularere-msq can you please add unit tests for this pr?

@matthieularere-msq
Copy link
Contributor Author

@ChrisHuie not that much. I've just add one line which should make sure that the lines related to gdpr vendor data is covered. However I don't really see how to add specific test for what was added as I don't see how to get access to my module's specific functions ?

@matthieularere-msq
Copy link
Contributor Author

Ok, I found the way to add test for what my changes.

Copy link
Collaborator

@dgirardi dgirardi left a comment

Choose a reason for hiding this comment

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

I have some suggestions below but this LGTM as it is.

let increment = args['cpm'];
if (typeof saveEvents['auctionEnd'] == 'object') {
for (let i = 0; i < saveEvents['auctionEnd'].length; i++) {
let tmpAuction = saveEvents['auctionEnd'][i];
Copy link
Collaborator

Choose a reason for hiding this comment

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

these types of loops would look a bit cleaner as

for (let tmpAuction of saveEvents['auctionEnd']) { ... }`

or

saveEvents['auctionEnd'].forEach((tmpAuction) => {...})

Although I think auction, bid would be better names than tmpAuction, tmpBid etc.

if (removead && typeof arg['ad'] != 'undefined') {
arg['ad'] = 'emptied';
}
if (typeof arg['gdprConsent'] != 'undefined' && typeof arg['gdprConsent']['vendorData'] != 'undefined') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO it'd be better to explicitly list what to keep rather than what to remove. As more features are added your payload will probably continue to grow, and I can't imagine new, unrecognized fields being too useful on your end. It would also make it less likely that we'd accidentally rename or remove some field that is important to you, because a simple code search will show us what you are depending on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion @dgirardi
I am no javascript specialist so I often go to the easiest way I know without searching for subtilities. As I have another PR to send very shortly I'll make the suggested changes with this next one.

@ChrisHuie ChrisHuie merged commit 7d12744 into prebid:master Aug 3, 2022
JacobKlein26 pushed a commit to nextmillenniummedia/Prebid.js that referenced this pull request Feb 9, 2023
* New Analytics Adapter bidwatch

* test for bidwatch Analytics Adapter

* change maintainer address

* Update bidwatchAnalyticsAdapter.js

* Update bidwatchAnalyticsAdapter.js

* Update bidwatchAnalyticsAdapter.md

* Update bidwatchAnalyticsAdapter.md

* add features to bidwatchAnalyticsAdapter

* update tests

* add test and made improvements
jorgeluisrocha pushed a commit to jwplayer/Prebid.js that referenced this pull request May 23, 2023
* New Analytics Adapter bidwatch

* test for bidwatch Analytics Adapter

* change maintainer address

* Update bidwatchAnalyticsAdapter.js

* Update bidwatchAnalyticsAdapter.js

* Update bidwatchAnalyticsAdapter.md

* Update bidwatchAnalyticsAdapter.md

* add features to bidwatchAnalyticsAdapter

* update tests

* add test and made improvements
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.

None yet

3 participants