Yieldmo Synthetic Inventory Module: add new module

[ym-abaranov]

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

New module. A link to a PR on docs repo here -- prebid/prebid.github.io#3322

[desidiver]

@ym-abaranov If google tag is undefined then this module won't work and we should throw some error. In the above line 5 we should do the same.

[ym-abaranov]

@desidiver we are defining an instance of googletag and pushing a callback, so later when the GPT will be initialized it will work okay. I've also added required params validation, now it's throwing an err.

[desidiver]

@Fawke any feedback on this PR? Thank you.

[ChrisHuie]

@desidiver it looks like this branch just needs to pull in recent commits to master to fix the errors. There was a linting error that briefly was merged before I could fix it and that must have been the master branch at the time of this pr. We rewrote all utils imports to only use the needed functions a couple of weeks ago and a few prs were still out there that had a .utils root.

[Fawke]

Hi @ym-abaranov,

The test coverage for this module is only 50%. Can you please make it at least 80% or more?

Also, I'm not sure about accessing googletag global object in the module. Adding @bretg for more clarification to see if we wanna allow it or not. Going through the Module Rules, I didn't find anything against this usage, but since I didn't find any other module accessing this object, I just want to confirm if this is allowed.

[ym-dlabuzov]

Hi @Fawke,
I've updated our tests, so now coverage should be > 85%

[bretg]

There are lots of files in the modules directory that refer to googletag, so in general this is fine.

However, glancing briefly at how this module is doing it, doesn't seem consistent with others.

There's a utils function called isGptPubadsDefined. This could be useful.

For instance, the Rubicon Analytics Adapter has what seems like a good way to handle it. Here it is generalized a bit:

        if (!gptFlag && isGptPubadsDefined()) {
          // Can do special GPT stuff right now!
          gptFlag = true;
        } else if (!gptFlag) {
          gptFlag = true;
          window.googletag = window.googletag || {};
          window.googletag.cmd = window.googletag.cmd || [];
          window.googletag.cmd.push(function () {
            // Can do special GPT stuff once googletag is loaded.
          });
        }

Adding @harpere to weigh in on whether whether this module's implementation of googletag is ok. My gut feeling is that assigning window.top.googletag without first checking that it exists is a problem.

Sounds like this might be another thing that needs to be checked by reviewers, but first we need consensus on the right approach.

[harpere]

@bretg Assuming we're ok with the module accessing GPT, I think the way it's being done is fine. If googletag already exists, this line reassigns it to itself. So only if it doesn't already exist does it assign it to {cmd: []}.

window.top.googletag = window.top.googletag || {cmd: []};

After that, all references to googletag are encapsulated in

googletag.cmd.push(() => {
    // some googletag code
});

So won't be executed until GPT has loaded, and will be executed immediately if it already has.

[ym-dlabuzov]

@Fawke, @bretg I've updated PR, so we are now using isGptPubadsDefined().
However, I don't know why 3rd party test is failing (locally tests are passed for me). Maybe rerunning the build will fix it.