Add smartxBidAdapter #5275

smartclip-adtech · 2020-05-20T14:50:55Z

Type of change

Bugfix
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

new smartclipBidAdapter

test parameters for validating bids

    var adUnits = [{
        code: 'video1',
        mediaTypes: {
            video: {
                context: 'outstream',
                playerSize: [640, 360]
            }
        },
        bids: [{
            bidder: 'smartx',
            params: {
                tagId: 'Nu68JuOWAvrbzoyrOR9a7A',
                publisherId: '11986',
                siteId: '22860',
                bidfloor: 0.3,
                bidfloorcur: "EUR",
                at: 2,
                cur: ["EUR"],
                outstream_options: {
                    slot: 'video1'
                },
                user: {
                    data: [{
                        id: 'emq',
                        name: 'emq',
                        segment: [{
                            id: 'emq',
                            name: 'emq',
                            value: 'e0:k14:e24'
                        }]
                    }, {
                        id: 'gs',
                        name: 'gs',
                        segment: [{
                            id: 'gs',
                            name: 'gs',
                            value: 'tone_of_voice_dislike:tone_of_voice_negative:gs_health'
                        }]
                    }]
                }
            }
        }]
    }];

Be sure to test the integration with your adserver using the Hello World sample page.

contact email of the adapter’s maintainer: adtech@smartclip.tv
official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

A link to a PR on the docs repo at https://github.com/prebid/prebid.github.io/

Other information

This reverts commit da2aea7.

lgtm-com · 2020-05-28T15:48:28Z

This pull request introduces 4 alerts when merging ee2546c into 707a103 - view on LGTM.com

new alerts:

4 for Invocation of non-function

stale · 2020-06-12T07:25:09Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lgtm-com · 2020-06-15T08:22:54Z

This pull request introduces 4 alerts when merging 7f09b06 into 8083239 - view on LGTM.com

new alerts:

4 for Invocation of non-function

stale · 2020-06-29T08:58:28Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

smartclip-adtech · 2020-06-29T08:59:31Z

The adapter is still "in review" - i don't know why.
Is there anything we have to do now?

lgtm-com · 2020-06-29T14:25:27Z

This pull request introduces 4 alerts when merging 4a96e3a into 240e605 - view on LGTM.com

new alerts:

4 for Invocation of non-function

jsnellbaker · 2020-08-21T13:50:14Z

@Skylinar

Thanks for making that update. The tests/files here look good.

Did you make the docs PR? I think that's the only outstanding item currently. If you did, please link it here.

Thanks!

muuki88 · 2020-08-21T20:24:55Z

modules/smartxBidAdapter.md

+            },
+            video: {
+                context: 'outstream',
+                playerSize: [640, 360]


Does the player size itself to this fixed size or does it scale itself in a responsive way like the direct integration at the moment does?

The player sizes in a responsive way.

Okay. So these sizes are just a hint? Or are they used in anyway?

We need these values for our system.

But for what 😂

Some SSPs use those to calculate the ratio - then I need to make sure we insert widht x height with optimal ratio for our content

Some SSPs use those for the demand site - then we input the sizes with the highest demand

Some SSPs actually size the player with this - then I don't know what to do on a responsive site 😛

Hi @muuki88,
we use the sizes to pass them on to the DSPs.

muuki88 · 2020-08-21T20:29:21Z

modules/smartxBidAdapter.js

+      utils.logError(BIDDER_CODE + ': siteId is not present in bidder params');
+      return false;
+    }
+    if (!utils.getBidIdParameter('bidfloor', bid.params)) {


This parameter is actually optional, right? Below you check for empty string instead of undefined.

I would prefer leaving this parameter out instead of setting it to an empty string.

Also this seems to mix two types. Number and string , which I'm not sure is intended

No these are not optional. In the old approaches of the adapter these were optional, so these queries were written like this. Now these parameters are mandatory and so I threw out these queries and save the bidfloor and bidfloorcur in constants within the next commit.

muuki88 · 2020-08-21T20:30:00Z

modules/smartxBidAdapter.js

+      utils.logError(BIDDER_CODE + ': bidfloorcur is not present in bidder params');
+      return false;
+    }
+    if (utils.deepAccess(bid, 'mediaTypes.video.context') == 'outstream') {


Triple equals is usually the safer choice

…n the markdown with more use cases

lgtm-com · 2020-08-24T12:34:10Z

This pull request introduces 4 alerts when merging a0a22f9 into 32066aa - view on LGTM.com

new alerts:

4 for Invocation of non-function

Skylinar · 2020-08-25T12:08:44Z

Hi @jsnellbaker,
here is the PR for docs: #2272

Thanks

lgtm-com · 2020-08-25T12:34:55Z

This pull request introduces 4 alerts when merging aa4dbc1 into 01f4b28 - view on LGTM.com

new alerts:

4 for Invocation of non-function

lgtm-com · 2020-08-25T12:54:21Z

This pull request introduces 4 alerts when merging 8b346a3 into 01f4b28 - view on LGTM.com

new alerts:

4 for Invocation of non-function

muuki88 · 2020-08-25T13:31:50Z

modules/smartxBidAdapter.js

+          };
+          const context1 = utils.deepAccess(currentBidRequest, 'mediaTypes.video.context');
+          const context2 = utils.deepAccess(currentBidRequest, 'params.ad_unit');
+          if (context1 == 'outstream' || context2 == 'outstream') {


I overlooked those. Triple equals again is safer.

Why have two options configuring the context ? What happens when the context2 is oustream but the others are instream ? Then a player is loaded, but everything is configured for instream ?

Maybe this parameter can be removed all together?

Removed context2, as we are not using params.ad_unit for context identifier.

muuki88 · 2020-08-25T13:32:54Z

modules/smartxBidAdapter.js

+      var contextcustom = utils.deepAccess(bid, 'mediaTypes.video.context');
+      var placement = 1;
+
+      if (contextcustom == 'outstream') {


Triple equals.

The context in the bid.params is not taken into account. See my comment below. Maybe the context in the params is redundant and can be removed.

… in params.ad_units. Adjusted query. smartxBidAdapter_spec.js - fixed typo

lgtm-com · 2020-08-25T14:16:39Z

This pull request introduces 4 alerts when merging 3f2ceec into 01f4b28 - view on LGTM.com

new alerts:

4 for Invocation of non-function

jsnellbaker · 2020-08-25T14:57:06Z

@smartclip-adtech @Skylinar

Just to confirm - are there any further changes planned (in lieu of the recent conversations)? Or are you all set from your end at this point? Thanks.

Skylinar · 2020-08-25T15:01:46Z

We are ready at this time, but there will be updates in the future, for sure. Thanks a lot!

Skylinar · 2020-08-28T12:58:05Z

modules/smartxBidAdapter.js

+    var rs = this.readyState;
+    if (rs && rs != 'complete' && rs != 'loaded') return;
+    try {
+      SmartPlay(elementId, smartPlayObj);


Hi @jsnellbaker,
the adapter is released, thanks a lot.

We are facing now problems due to minified code that gets delivered when downloading the adapter from the official page. At this line we create our Outstream-Script Player and we are calling several functions (sc_smartIntxtStart(n), sc_smartIntxtNoad(n), sc_smartIntxtEnd(n), SmartPlay(elementId, smartPlayObj)) related to our out-stream player but in the minified version they get replaced with (void 0)(r) and console error Prebid ERROR: error caught : TypeError: (void 0) is not a function so the outstream part doesn't work, what can we do so that the function createOutstreamScript(bid) is not changed in the later download version?

What if you try to call the script function from the window instead of calling it on its own? Since it doesn't seem the SmartPlay variable gets defined within the scope of the bidAdapter code (which is likely why it converted it to the void(0) to check if it was undefined), calling from the window sould give it a definite reliable source even in the minified state.

We are loading our out-stream playerlibary with the "https://dco.smartclip.net/?plc=7777777" call, there the mentioned functions are available. I have implemented the prebid.js on a test page and replaced the voids with the originally defined functions, everything works fine there.So in our opinion it is only necessary to prevent that the 4 function calls are not changed. Is this possible?

The issue with the minified build is that it is likely seeing these variables never get assigned an actual value. While this isn't the case in a real page (because you expect the script to execute and overwrite the placeholders), it doesn't change from what build stack sees in the code.

In regards to the 3 'callback' functions, why are they defined here in the adapter file and not included as part of the script code? These functions don't read any values from inside the bidAdapter file to pass along into the script file. It's just my guess, but the callback functions appear to be internal functions that are within the script file. Could these 3 functions (onStartCallback, onCappedCallback, onEndCallback) just be taken out of the config object and moved inside the script itself?

For the SmartPlay call at line 376, I tried to change the reference to be window.SmartPlay(... and it appeared to work with the build file was minified (see example below).

Can you give this a try on your end?

We use callback functions to give customers the flexibility to use callbacks should certain events occur. For example, customers can define the function sc_smartIntxtStart() on their webpage, which will be called by our player when the promotional video starts and the event gets fired.

The issue with the minified build is that it is likely seeing these variables never get assigned an actual value.

Do you mean this variables?

// eslint-disable-next-line camelcase var sc_smartIntxtStart; // eslint-disable-next-line camelcase var sc_smartIntxtNoad; // eslint-disable-next-line camelcase var sc_smartIntxtEnd; var SmartPlay;

If yes: We added them back then on your hint due to lint errors, actually we don't need them and can leave them out. Or is it possible that exactly these variables confuse the minifier?
If not: Do you mean the functions?

Here is the test site only with added 4 functions, I've tested it with window. before and it seems to work as well but I would prefer to let them be as they are and use this solution only if no other is available:
https://gt-adtech.de/var/www/testsite/prod/prebid_test.html
Could you please check if there are options for the minifier to ignore these 4 functions or could be the solution to remove the above not needed variables?

Thanks

Hi @jsnellbaker,
did you already have time to have a look at it? We want to fix the problem as soon as possible, because the current version does not work like this.

Thanks

Hi @Skylinar

I meant callback functions altogether. We can't have improperly defined functions within the code since it could lead to the chance that someone doesn't define the overridden function properly on their site and it would cause a hard undefined exception error in prebid - interfering with the rest of the auction.

If they're values that can be overwritten by the page so the publisher can customize what happens, then maybe this part of the logic could be handled inside the script itself instead of passing it in as part of a tentative config.

By removing them from being in an 'undefined'/uninitialized state in the adapter, it would avoid the lint errors and confusing part of the logic (since on its own, it's not clear where these functions are coming from or what they're doing).

I understand if there may be other reasons why this may not be an option (or at least something quick to implement).
In this case, I think you'll have to read these functions from the window to preserve their proper function status. I don't know of anything else we can do in the minifying step to just skip over this part of the code.

Hi @jsnellbaker,
I've created a new PR where we call it with window. Could you please check?

#5689

Thank you!

Skylinar · 2020-08-28T12:58:28Z

modules/smartxBidAdapter.js

+    },
+    onStartCallback: function (m, n) {
+      try {
+        sc_smartIntxtStart(n);


See comment above.

Skylinar · 2020-08-28T12:58:36Z

modules/smartxBidAdapter.js

+    },
+    onCappedCallback: function (m, n) {
+      try {
+        sc_smartIntxtNoad(n);


See comment above.

Skylinar · 2020-08-28T12:58:42Z

modules/smartxBidAdapter.js

+    },
+    onEndCallback: function (m, n) {
+      try {
+        sc_smartIntxtEnd(n);


See comment above.

Skylinar · 2020-08-28T13:00:50Z

modules/smartxBidAdapter.js

+  // const slot = utils.getBidIdParameter('slot', bid.renderer.config.outstream_options);
+  utils.logMessage('[SMARTX][renderer] Handle SmartX outstream renderer');
+  const elementId = bid.adUnitCode;
+  // eslint-disable-next-line camelcase


Is it possible that lines 335-340 are causing the problem? As far as i can remember it was pointed out in the course of the PR that we should define the callback functions as empty variables before to pass the lint test.

* Add smartclipBidAdapter * Updated imported files * .js file * Beautify code * Revert "Beautify code" This reverts commit da2aea7. * testing * partial tab fix * syntax fixes * further syntax fixes * Trailing spaces... * const parameters fpr smartInTxt Functions * const to var * // eslint-disable-next-line camelcase * new commit * Update Targeting * Update syntax * beautify code * remove trailing spaces * Update smartclipBidAdapter.md * Update smartclipBidAdapter.js * Update smartclipBidAdapter.js * rename files * added first unit tests, further adjustments * adjusted queries, ran gulp lint, etc * adjustments, further tests * extendet unit tests * naming for spec * adjusted bidfloor and bidfloorcur, removed old queries, adjustments in the markdown with more use cases * added more use cases within md file * layout fix * md layout * smartxBidAdapter.js - triple equals, removed context2 due to not used in params.ad_units. Adjusted query. smartxBidAdapter_spec.js - fixed typo Co-authored-by: smartclip AdTechnology <adtech@smartclip.com> Co-authored-by: Gino Cirlini <skylinar@web.de>

This reverts commit aee5421.

Add smartclipBidAdapter

aa0c2de

smenzer added the needs review label May 22, 2020

smenzer assigned afewcc May 22, 2020

smartclip-adtech and others added 11 commits May 28, 2020 14:57

Updated imported files

ad6b063

.js file

0fe9f34

Beautify code

da2aea7

Revert "Beautify code"

48650b7

This reverts commit da2aea7.

testing

4da1497

partial tab fix

62f18a9

syntax fixes

02a6733

further syntax fixes

7e07c6b

Trailing spaces...

e9bcbfb

const parameters fpr smartInTxt Functions

f937042

const to var

ee2546c

smartclip-adtech mentioned this pull request Jun 5, 2020

Our ci/circleci: build always fails. smartclipBidAdapter #5275 #5335

Closed

stale bot added the stale label Jun 12, 2020

// eslint-disable-next-line camelcase

0435415

stale bot removed the stale label Jun 15, 2020

new commit

7f09b06

stale bot added the stale label Jun 29, 2020

stale bot removed the stale label Jun 29, 2020

Update Targeting

4a96e3a

smartclip-adtech added 2 commits June 29, 2020 16:46

Update syntax

cf081d6

beautify code

6a46039

jsnellbaker added needs docs and removed needs unit tests needs update labels Aug 21, 2020

muuki88 reviewed Aug 21, 2020

View reviewed changes

adjusted bidfloor and bidfloorcur, removed old queries, adjustments i…

a0a22f9

…n the markdown with more use cases

muuki88 approved these changes Aug 24, 2020

View reviewed changes

smartclip-adtech added 2 commits August 25, 2020 14:12

added more use cases within md file

c4bc219

layout fix

aa4dbc1

smartclip-adtech changed the title ~~Add smartclipBidAdapter~~ Add smartxBidAdapter Aug 25, 2020

md layout

8b346a3

muuki88 reviewed Aug 25, 2020

View reviewed changes

smartxBidAdapter.js - triple equals, removed context2 due to not used…

3f2ceec

… in params.ad_units. Adjusted query. smartxBidAdapter_spec.js - fixed typo

jsnellbaker approved these changes Aug 25, 2020

View reviewed changes

jsnellbaker merged commit 71ff934 into prebid:master Aug 25, 2020

Skylinar reviewed Aug 28, 2020

View reviewed changes

smartclip-adtech mentioned this pull request Sep 2, 2020

Fix smartxBidAdapter smartPlay functions #5689

Merged

1 task

BrightMountainMediaInc pushed a commit to BrightMountainMediaInc/Prebid.js that referenced this pull request Sep 14, 2020

Revert "Add smartxBidAdapter (prebid#5275)"

b74fdbe

This reverts commit aee5421.

Add smartxBidAdapter #5275

Add smartxBidAdapter #5275

Conversation

smartclip-adtech commented May 20, 2020 • edited Loading

Type of change

Description of change

Other information

lgtm-com bot commented May 28, 2020

stale bot commented Jun 12, 2020

lgtm-com bot commented Jun 15, 2020

stale bot commented Jun 29, 2020

smartclip-adtech commented Jun 29, 2020

lgtm-com bot commented Jun 29, 2020

jsnellbaker commented Aug 21, 2020

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

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

lgtm-com bot commented Aug 24, 2020

Skylinar commented Aug 25, 2020

lgtm-com bot commented Aug 25, 2020

lgtm-com bot commented Aug 25, 2020

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

lgtm-com bot commented Aug 25, 2020

jsnellbaker commented Aug 25, 2020

Skylinar commented Aug 25, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Skylinar Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

Skylinar Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

Skylinar Aug 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smartclip-adtech commented May 20, 2020 •

edited

Loading

Skylinar Aug 28, 2020 •

edited

Loading

Skylinar Aug 28, 2020 •

edited

Loading

Skylinar Aug 28, 2020 •

edited

Loading