Ensure transaction ID is always unique

[naegelin]

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

Transaction ID implicitly suggests that this is a unique identifier per transaction. However in the case of a refresh / re-use of an adunit the transaction ID remains the same causing undesirable bidding with certain SSPs.

[naegelin]

linter is failing using the standard github inline editor. Leaving this as is for the time being .

[jsnellbaker]

@naegelin The Prebid.js project uses the gulp-eslint plugin to maintain the styling rules for the project. Can you please make the necessary updates in your file to conform to the linting rules? You can locally check the linting status/errors by running gulp lint in your Prebid.js project.

[naegelin]

The tests for transaction id validation are now failing (obviously). I'm leaving this as is for discussion first @mkendall07 @jsnellbaker as a decision needs to be made regarding what the intended behavior should actually be here.

[mkendall07]

It looks like this was changed with #2706
Including @snapwich here for comment. I think we do want to always assign a new transactionId though.

[snapwich]

I'm not sure on what the business logic of transactionId should be, maybe @bretg has input?

In #2706 I specifically tried to copy the existing business logic of having a persistent transactionId assigned to each adUnit. Before #2706, transactionIds were added when you called pbjs.addAdUnit() but they were not refreshed if you were to pbjs.requestBids() multiple times. I kept that same business logic in #2706, however I moved the transactionId creation inside of pbjs.requestBids() to ensure that all adUnits had a transactionId, including ones passed directly to pbjs.requestBids() without the use of pbjs.addAdUnit().

[mkendall07]

ah interesting, thanks Rich. @jaiminpanchal27 do you have any idea on this?

[bretg]

The definition of transactionID is in http://prebid.org/dev-docs/bidder-adaptor.html#bidrequest-parameters:

Transaction ID is unique for each ad unit with a call to requestBids, but same across bidders. This is the ID that DSPs need to recognize the same impression coming in from different supply sources.

So if transactionId is not currently changing on a refresh, I agree with @naegelin that this is a bug.

[snapwich]

So the only other concern I could possibly think of with this pull-request was if an implementor wanted to generate their own transactionIds and pass them through then this would overwrite their ids. Is that something that people do? @bretg I know we generated our own transactionIds as part of hpv2, but I can't remember if that was just a temporary fix because #2706 hadn't happened yet.

If it's no big deal then this LGTM. (other than the tests which need to be updated as well).

[bretg]

I think we found that transaction ID wasn't being added because we weren't using pbjs.addAdUnit. Our code checks:

                    if (!adUnit.transactionId) {
                        adUnit.transactionId = generateUUID();
                    }

So yes - I think #2706 should have resolved that issue. So I think we're good here. Except that the unit tests are failing.

[mkendall07]

@naegelin
Please update the test and we can merge this. Thanks!

[snapwich]

LGTM

[harpere]

LGTM

[naegelin]

Thanks for updating guys - was on vacation and just saw this :)