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

DigiTrust Facade init GH Issue 3911 #3918

Merged

Conversation

goosemanjack
Copy link
Contributor

Fix to init the DigiTrust facade object if the userId framework would not normally execute the call due to an ID being found.

Type of change

  • [X ] Bugfix

Description of change

This executes code to build the DigiTrust facade after a time if it has not already been built.

  • [X ] official adapter submission

Other information

Fixes issue 3911

Fix to init the DigiTrust facade object if the userId framework would not normally execute the call due to an ID being found.
@mkendall07
Copy link
Member

This pull request introduces 1 alert when merging 1185a02 into 842cc19 - view on LGTM.com

new alerts:

  • 1 for Superfluous trailing arguments

This project has automated code review enabled, but doesn't use the LGTM GitHub App. Migrate over by installing the app. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

@idettman idettman self-requested a review June 18, 2019 17:34
@goosemanjack goosemanjack changed the title GH Issue 3911 DigiTrust Facade init GH Issue 3911 Jun 19, 2019
@idettman

This comment has been minimized.

Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

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

Latest code LGTM, but a unit test for this scenario should be added

@goosemanjack

This comment has been minimized.

@mkendall07
Copy link
Member

This pull request introduces 1 alert when merging c9713db into 2bd04a1 - view on LGTM.com

new alerts:

  • 1 for Superfluous trailing arguments

This project has automated code review enabled, but doesn't use the LGTM GitHub App. Migrate over by installing the app. Read about the benefits of migrating to GitHub Apps in the blog.


Comment posted by LGTM.com

@goosemanjack

This comment has been minimized.

@bretg bretg removed the request for review from robertrmartinez July 16, 2019 15:00
@bretg bretg assigned idettman and unassigned robertrmartinez Jul 16, 2019
Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

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

Looks good with a small change to remove the import and usage of attachIdSystem, since it was replaced by submodule.


fallbackTimer = setTimeout(fallbackInit, fallbackTimeout);

attachIdSystem(digiTrustIdSubmodule);
Copy link
Contributor

Choose a reason for hiding this comment

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

delete this line (attachIdSystem was replaced by submodule)

modules/digiTrustIdSystem.js Outdated Show resolved Hide resolved
Copy link
Contributor

@idettman idettman left a comment

Choose a reason for hiding this comment

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

LGTM

@idettman idettman merged commit 4a3372d into prebid:master Jul 23, 2019
leonardlabat pushed a commit to criteo-forks/Prebid.js that referenced this pull request Jul 30, 2019
* GH Issue 3911
Fix to init the DigiTrust facade object if the userId framework would not normally execute the call due to an ID being found.

* Fixed superfluous trailing arg issue.

* Addition of unit test for digiTrustIdSystem. Fix init error in facade callback.

* Uncommenting init code.

* Reverting package-lock.json file.

* Removed extraneous function parameter.

* Removing unused code. Fixing file casing issue causing unit test to fail.

* Adding support for SameSite=none to cookie

* Tweaking unit test.

* Removing Promise from unit test as IE doesn't like it.

* Cleanup of unused code lines

* Minor comment changes.

* Commenting out unit tests to see if this fixes Safari timeout issue.

* Reenable test. Fixing where done was not being called.

* Whitespace changes for lint

* Capture and clear fallback timer to fix unit tests.

* Removing unused function

* Comment improvements. Retry CircleCI for unassociated failure.

* Removed old call to attachIdSystem.
VideoReach pushed a commit to VideoReach/Prebid.js that referenced this pull request Aug 1, 2019
* GH Issue 3911
Fix to init the DigiTrust facade object if the userId framework would not normally execute the call due to an ID being found.

* Fixed superfluous trailing arg issue.

* Addition of unit test for digiTrustIdSystem. Fix init error in facade callback.

* Uncommenting init code.

* Reverting package-lock.json file.

* Removed extraneous function parameter.

* Removing unused code. Fixing file casing issue causing unit test to fail.

* Adding support for SameSite=none to cookie

* Tweaking unit test.

* Removing Promise from unit test as IE doesn't like it.

* Cleanup of unused code lines

* Minor comment changes.

* Commenting out unit tests to see if this fixes Safari timeout issue.

* Reenable test. Fixing where done was not being called.

* Whitespace changes for lint

* Capture and clear fallback timer to fix unit tests.

* Removing unused function

* Comment improvements. Retry CircleCI for unassociated failure.

* Removed old call to attachIdSystem.
sa1omon pushed a commit to gamoshi/Prebid.js that referenced this pull request Nov 28, 2019
* GH Issue 3911
Fix to init the DigiTrust facade object if the userId framework would not normally execute the call due to an ID being found.

* Fixed superfluous trailing arg issue.

* Addition of unit test for digiTrustIdSystem. Fix init error in facade callback.

* Uncommenting init code.

* Reverting package-lock.json file.

* Removed extraneous function parameter.

* Removing unused code. Fixing file casing issue causing unit test to fail.

* Adding support for SameSite=none to cookie

* Tweaking unit test.

* Removing Promise from unit test as IE doesn't like it.

* Cleanup of unused code lines

* Minor comment changes.

* Commenting out unit tests to see if this fixes Safari timeout issue.

* Reenable test. Fixing where done was not being called.

* Whitespace changes for lint

* Capture and clear fallback timer to fix unit tests.

* Removing unused function

* Comment improvements. Retry CircleCI for unassociated failure.

* Removed old call to attachIdSystem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants