airgridRtdProvider: use provided tag insertion method `loadExternalScript` #9901

naripok · 2023-05-03T19:16:27Z

Type of change

Feature
New bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other

Description of change

Use loadExternalScript instead of appendChild to insert the ad tag.
Fixes issue #8421

CC @ydennisy

AirGrid RTD Provider: update audience data setting method for the Ortb2 configuration

chore: use provided tag insertion method

naripok · 2023-05-05T14:22:45Z

Seems like our changes are breaking a seemingly unrelated module (mass).
I looked around and can't make sense of how the changes here would affect this module in particular.
Maybe somebody has a good hint/insight about how to fix it?

The test error message:

SUMMARY:
✔ 12427 tests completed
✖ 1 test failed

FAILED TESTS:
  MASS Module
    ✖ should have a default renderer
      Chrome Headless 113.0.5672.63 (Linux x86_64)
    TypeError: Cannot read properties of undefined (reading 'parentNode')
        at render (/tmp/_karma_webpack_879703/commons.js:96375:9)
        at Context.<anonymous> (/tmp/_karma_webpack_879703/commons.js:308431:5)

[11:29:18] 'test-all-features-disabled' errored after 1.13 min
[11:29:18] Error: Karma tests failed with exit code 1
    at ChildProcess.<anonymous> (/home/tau/Projects/airgrid/Prebid.js/gulpfile.js:342:12)
    at ChildProcess.emit (node:events:513:28)
    at ChildProcess.emit (node:domain:552:15)
    at ChildProcess._handle.onexit (node:internal/child_process:291:12)
    at Process.callbackTrampoline (node:internal/async_hooks:130:17)
[11:29:18] 'test' errored after 1.78 min

modules/airgridRtdProvider.js

patmmccann · 2023-05-06T11:31:37Z

@dgirardi could you assist this commiter?

ydennisy · 2023-05-11T17:04:10Z

@naripok we have had some feedback that we should be updating bid request object in a different way.

Ref: https://docs.prebid.org/dev-docs/add-rtd-submodule.html#reqBidsConfigObj

Recommended: use one of these First Party Data conventions
For global first party data, including bidder-specific data, modify the reqBidsConfigObj as shown below

dgirardi · 2023-05-11T18:21:46Z

The MASS test failure should be fixed with #9939

naripok · 2023-05-11T19:05:03Z

The MASS test failure should be fixed with #9939

Thank you so much @dgirardi!

dgirardi · 2023-05-11T19:08:10Z

Regarding how to pass audience data to bidders, I believe you had it very close in an old version:

https://github.com/AirGrid/Prebid.js/blame/0eb76d7620c3bfccd254340fbc189985373fda07/modules/airgridRtdProvider.js

where that is modifying the ortb2Fragments parameter in the way recommended by the docs; the only missing piece is what to do about appnexus - since it does not understand user.ext.data.

My suggestion is to restore the old logic setting user.ext.data (without using mergeBidderConfig), and expand it to also update user.keywords with the appnexus-specific perid=<audienceId>.

feat: extend module data setting using `user.keywords` for appnexus

naripok · 2023-05-15T16:49:12Z

Regarding how to pass audience data to bidders, I believe you had it very close in an old version:

https://github.com/AirGrid/Prebid.js/blame/0eb76d7620c3bfccd254340fbc189985373fda07/modules/airgridRtdProvider.js

where that is modifying the ortb2Fragments parameter in the way recommended by the docs; the only missing piece is what to do about appnexus - since it does not understand user.ext.data.

My suggestion is to restore the old logic setting user.ext.data (without using mergeBidderConfig), and expand it to also update user.keywords with the appnexus-specific perid=<audienceId>.

Thank you very much for the help, @dgirardi.
I've restored the old logic and extended it for setting ortb2.user.keywords with the appnexus values, removing mergeBidderConfig usage.
If you have a second, would you let me know if it looks good?
@ydennisy, please, take a look too if you can.

dgirardi · 2023-05-15T16:58:47Z

modules/airgridRtdProvider.js

  const bidders = deepAccess(rtdConfig, 'params.bidders');
  if (!bidders || bidders.length === 0 || !audiences || audiences.length === 0) return;

+  const agOrtb2 = {}
+  deepSetValue(agOrtb2, 'ortb2.user.ext.data.airgrid', audiences);


this is out of spec - data should be an array, sorry for not catching this earlier.

Oh, I saw it and missed anyway...
Thank you for the catch! Let me fix that.

Wait, sorry about the confusion @dgirardi, but we're using user.ext.data here, not user.data, which is an array.
Should we be using user.data instead?
I saw it was present here as an obj and we were using it also in the previous implementation, but I'm not sure anyway.

user.ext.data is the safer choice, it has been "promoted" to user.data in ORTB version 2.6 but afaik that hasn't been widely adopted yet.

Thanks for the explanation!
Does this mean that we're good, then? 😅

No objection to forking the PR, but I'd prefer then if you rolled this back to just the loadExternalScript change so that the incorrect values in user.ext are not forgotten.

@dgirardi, I've reverted the data setting changes so this PR includes only the loadExternalScript changes.

Now, for the data setting method, as per last feedback, I understand that the changes introduced in this PR are bad, and we should revert those to the previous setting method, right?
I'm just a little bit confused with all the back and forth we had, so, I'm sorry to ask, but could you please again be very specific in which should be the desired data setting method you'd like to see for the next PR?
Is this the preferred way to go forward?

the method is the one you just had in the previous commit, with the difference that the data should be formatted as a ORTB user.data segment.

Oh, alright, got ya!
Thanks!

@dgirardi, I've pushed the data setting changes to a PR in our fork here. I'll open a PR here once I know it is good and we have this PR merged.
Would you mind checking out the question in this comment, please?
I'm doing some guesswork there, and it would be great to have some confirmation that I'm getting it right.
Thank you very much!

ydennisy · 2023-05-16T12:19:26Z

@naripok I think we should mark this as draft.

patmmccann · 2023-05-26T16:17:12Z

@naripok please resolve conflicts

naripok · 2023-05-26T16:27:56Z

@naripok please resolve conflicts

@patmmccann Done :)

* master: Colossus Bid Adapter: gpp support (prebid#10096) Increment version to 8.1.0-pre Prebid 8.0.0 release Prebid 8: initial release (prebid#10071) Increment version to 7.54.1-pre Prebid 7.54.0 release Update topicsFpdModule.js (prebid#9990) Added GPP support (prebid#10094) Teads Bid Adapter:Remove auctionId from since it is not used anymore (prebid#10098) HypeLab Bid Adapter: Initial Release (prebid#10003) Oxxion Rtd Module: listen onBidResponseEvent instead of onAuctionEndEvent (prebid#10083) PubMatic Analytics Adapter : Added extra fields in tracker for analytics (prebid#10090) AdHash Bidder Adapter: update for brand safety (prebid#10087) videojsVideoProvider: use contrib-ads event names (prebid#10081) Increment version to 7.54.0-pre Prebid 7.53.0 release Add bidgency alias (prebid#10077) Weborama RTD Module : start Bidder specific handling removal (prebid#10005) ZetaGlobalSspBidAdapter: change logic around mediaType (prebid#10074) add tmax to sovrnBidAdapter (prebid#10059) Mediasquare Bid Adapter: Avoid to run bidwon event on video impressions (prebid#10068) FreePass User ID Module : initial release (prebid#9814) Conceptx Bid Adapter : initial release (prebid#9835) beOp BidAdapter: FirstPartyData management and ingest Permutive segments (prebid#9947) StroeerCore Bid Adapter: add price floor support (prebid#9962) YieldlabBidAdapter updated native asset mapping (prebid#9989) Mediasquare Bid Adapter: handle context for video bids (prebid#10055) Add new alias - Apester (prebid#10057) Criteo Bid Adapter: Pass outstream video renderer method (prebid#10054) Add: Viously GVL ID (prebid#10061) 1plus x RTD Adapter: remove bidder specific handling enforcement (DC 3634) (prebid#10001) Sirdata RTD Module : bidder specific handling removal (prebid#9970) feat: pass video.plcmt [PB-1736] (prebid#10058) Update adkernelBidAdapter.js (prebid#10056) Eskimi Bid Adapter: Added support for video (prebid#9985) Richaudience Bid Adapter : add compability with GPID (prebid#9928) Add GVL ID for pairId userId submodule. (prebid#10053) Update nexx360BidAdapter.js (prebid#10042) Logicad Bid Adapter: Add topics and uach support (prebid#10045) change session_id fallback to bidderRequestId (prebid#10047) ZetaGlobalSsp Bid Adapter : process array of sizes (prebid#10039) AdagioBidAdapter: change outstream renderer (prebid#10035) update greenbids analytics endpoint (prebid#10037) ID5 User Id module - get whole ext object from server response (prebid#10036) Criteo Bid Adapter : Bump fastbid version to 136 (prebid#9973) UID2 & EUID Modules: Add support for EUID and prefer localStorage for both modules. (prebid#9968) Schain module: do not share schain objects across different bid requests (prebid#10021) airgridRtdProvider: use provided tag insertion method `loadExternalScript` (prebid#9901) Increment version to 7.53.0-pre Prebid 7.52.0 release Yahoo ConnectId - fixed tests. (prebid#10033) Core & Multiple modules: activity controls (prebid#9802) Yahoo ConnectId - gpp consent module usage. (prebid#10022) Yahoo bid adapter: User sync pixels, consent signals update (prebid#10028) Core: fix `markWinningBidAsUsed` to set bid as winning and not just rendered (prebid#10014) Adriver Id System: add cfa param to url (prebid#10024) Undertone Bid Adapter : added GPP and video placements (prebid#10016) Oxxion Analytics Adapter : initial adapter release (prebid#9682) Connect id : storage duration updates and no longer require puid or he to be set (prebid#9965) Update ad generation adapter 1.6.0: update userSync (prebid#9984) fix module type (prebid#10019) Stv Bid Adapter: add schain support (prebid#10010) GrowthCode RTD : initial release (prebid#9852) ShareThrough Bid Adapter : fix playerSize (prebid#10011) chore: update default video placement value [PB-1560] (prebid#9948) Smartadserver Bid Adapter: support GPID (prebid#10004) Criteo Adapter: Add support of bcat/badv/bapp (prebid#10013) Zeta Global SSP Bid Adapter: Added support for video.plcmt (prebid#10009) Pair Id System: less noisy errors (prebid#9998) Increment version to 7.52.0-pre Prebid 7.51.0 release Appnexus Bid Adapter: added uol as an alias (prebid#10002) Adquery Bid Adapter : added bid request: version and bidPageUrl (prebid#9946) MinuteMedia Bid Adapter: support sua and plcmt params (prebid#9997) Adkernel Bid Adapter: add didnadisplay alias (prebid#10000) Adrino Bid Adapter: pass adUnitCode to adserver (prebid#9993)

…ript` (prebid#9901) * chore: update `getAudiencesAsBidderOrtb2` implementation and test * chore: use provided tag insertion method * fix: add `airgrid` to `_approvedLoadExternalJSList` * fix: use 'sdk' path if no publisherId is provided * fix: use accountId as path param for script url * fix: assign edktInitializor props before `loadExternalScript` call * fix: set `edktInitializor.invoked` before calling `loadExternalScript` * fix: restore method for setting `user.ext.data` feat: extend module data setting using `user.keywords` for appnexus * fix: rollback changes to data setting method

naripok added 8 commits March 28, 2023 10:08

chore: update getAudiencesAsBidderOrtb2 implementation and test

ea68e95

Merge pull request #1 from AirGrid/chore/update-airgrid-rtd-module

56abb14

AirGrid RTD Provider: update audience data setting method for the Ortb2 configuration

Merge remote-tracking branch 'upstream/master'

7b5f106

chore: use provided tag insertion method

a885439

fix: add airgrid to _approvedLoadExternalJSList

b2d56ba

fix: use 'sdk' path if no publisherId is provided

3209565

Merge pull request #2 from AirGrid/chore/1459-use-tag-insertion-method

d5e12e6

chore: use provided tag insertion method

Merge branch 'master' of https://github.com/prebid/Prebid.js

c6d9fdd

naripok marked this pull request as draft May 4, 2023 19:36

naripok marked this pull request as ready for review May 4, 2023 19:36

naripok marked this pull request as draft May 4, 2023 19:47

naripok added 2 commits May 5, 2023 09:21

Merge branch 'prebid:master' into master

665b1aa

fix: use accountId as path param for script url

7798c95

naripok mentioned this pull request May 5, 2023

Multiple adapters: use our tag insertion methods #8421

Closed

ydennisy reviewed May 5, 2023

View reviewed changes

modules/airgridRtdProvider.js Outdated Show resolved Hide resolved

naripok added 2 commits May 5, 2023 12:47

fix: assign edktInitializor props before loadExternalScript call

20a3926

fix: set edktInitializor.invoked before calling loadExternalScript

6cc0ac9

dgirardi mentioned this pull request May 11, 2023

MASS module: fix tests #9939

Merged

1 task

naripok added 2 commits May 15, 2023 12:08

Merge branch 'prebid:master' into master

5b0ca57

fix: restore method for setting user.ext.data

b2af960

feat: extend module data setting using `user.keywords` for appnexus

naripok requested a review from ydennisy May 15, 2023 16:46

naripok marked this pull request as ready for review May 15, 2023 16:46

dgirardi reviewed May 15, 2023

View reviewed changes

dgirardi mentioned this pull request May 15, 2023

Move special Bid adapter handling from RTD modules into those adapters #8596

Closed

naripok marked this pull request as draft May 16, 2023 12:31

fix: rollback changes to data setting method

7bde0ca

naripok marked this pull request as ready for review May 18, 2023 13:49

naripok requested a review from dgirardi May 18, 2023 14:22

dgirardi approved these changes May 18, 2023

View reviewed changes

ydennisy approved these changes May 19, 2023

View reviewed changes

jsnellbaker assigned dgirardi May 22, 2023

jsnellbaker added the maintenance label May 22, 2023

Merge branch 'master' into master

c4f8e53

patmmccann merged commit 27ea1ad into prebid:master Jun 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

airgridRtdProvider: use provided tag insertion method `loadExternalScript` #9901

airgridRtdProvider: use provided tag insertion method `loadExternalScript` #9901

naripok commented May 3, 2023

naripok commented May 5, 2023 •

edited

Loading

patmmccann commented May 6, 2023

ydennisy commented May 11, 2023

dgirardi commented May 11, 2023

naripok commented May 11, 2023 •

edited

Loading

dgirardi commented May 11, 2023

naripok commented May 15, 2023

dgirardi May 15, 2023

naripok May 15, 2023

naripok May 15, 2023

dgirardi May 15, 2023

naripok May 15, 2023 •

edited

Loading

dgirardi May 16, 2023

naripok May 18, 2023

dgirardi May 18, 2023

naripok May 18, 2023

naripok May 18, 2023

ydennisy commented May 16, 2023

patmmccann commented May 26, 2023

naripok commented May 26, 2023 •

edited

Loading

airgridRtdProvider: use provided tag insertion method loadExternalScript #9901

airgridRtdProvider: use provided tag insertion method loadExternalScript #9901

Conversation

naripok commented May 3, 2023

Type of change

Description of change

naripok commented May 5, 2023 • edited Loading

patmmccann commented May 6, 2023

ydennisy commented May 11, 2023

dgirardi commented May 11, 2023

naripok commented May 11, 2023 • edited Loading

dgirardi commented May 11, 2023

naripok commented May 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naripok May 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ydennisy commented May 16, 2023

patmmccann commented May 26, 2023

naripok commented May 26, 2023 • edited Loading

airgridRtdProvider: use provided tag insertion method `loadExternalScript` #9901

airgridRtdProvider: use provided tag insertion method `loadExternalScript` #9901

naripok commented May 5, 2023 •

edited

Loading

naripok commented May 11, 2023 •

edited

Loading

naripok May 15, 2023 •

edited

Loading

naripok commented May 26, 2023 •

edited

Loading