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

Consent Management module bug fix. #2588

Merged
merged 26 commits into from
May 23, 2018
Merged

Consent Management module bug fix. #2588

merged 26 commits into from
May 23, 2018

Conversation

anand-venkatraman
Copy link
Contributor

Type of change

  • Bugfix

Description of change

Consent Management module has a bug when the CMP module responds back with String message instead of Object. The code fragment that checks for existence of substring cmpReturn, makes the check using array.includes, instead of string.includes from the core-js lib.

Also the test, was not really testing the code part for iframed pages. Since the existing test created a stub for window.__cmp, the test was not really running the code branch for "iframed" - callCmpWhileInIframe. Updated the test to verify behavior with both Object message format and String message format.

* ET-1691: Adding pulsepoint analytics and tests for pulsepoint adapter

* ET-1691: Adding pulsepoint analytics and tests for pulsepoint adapter

* ET-1691: cleanup

* ET-1691: minor

* ET-1691: revert package.json change
@anand-venkatraman anand-venkatraman changed the title Gdpr bug fix Consent Management module bug fix. May 22, 2018
Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

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

LGTM thanks for putting this together.

@jsnellbaker jsnellbaker added LGTM needs 2nd review Core module updates require two approvals from the core team labels May 23, 2018
@mkendall07 mkendall07 merged commit fc95a52 into prebid:master May 23, 2018
jsnellbaker added a commit that referenced this pull request May 23, 2018
jsnellbaker added a commit that referenced this pull request May 23, 2018
@jsnellbaker jsnellbaker mentioned this pull request May 23, 2018
1 task
mkendall07 pushed a commit that referenced this pull request May 23, 2018
* Revert "Consent Management module bug fix. (#2588)"

This reverts commit fc95a52.

* use new file for string includes
@jsnellbaker
Copy link
Collaborator

@anand-venkatraman As you may have seen from the additional updates on this PR, we had to partially revert the changes to fix the build on master. The unit test updates were causing some test failures on Safari browsers (see here https://travis-ci.org/prebid/Prebid.js/builds/382804970).

If you want to re-include the unit test enhancements, can you please take a look into these errors and create another PR with the updated changes?

@anand-venkatraman
Copy link
Contributor Author

@jsnellbaker didnt notice it before, but will check this and create another PR for you. Thanks.

dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* ET-1691: Pulsepoint Analytics adapter for Prebid. (#1)

* ET-1691: Adding pulsepoint analytics and tests for pulsepoint adapter

* ET-1691: Adding pulsepoint analytics and tests for pulsepoint adapter

* ET-1691: cleanup

* ET-1691: minor

* ET-1691: revert package.json change

* Adding bidRequest to bidFactory.createBid method as per prebid#509

* ET-1765: Adding support for additional params in PulsePoint adapter (#2)

* ET-1850: Fixing prebid#866

* Minor fix

* Adding mandatory parameters to Bid

* GDPR Bug Fix with String response

* minor
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* Revert "Consent Management module bug fix. (prebid#2588)"

This reverts commit fc95a52.

* use new file for string includes
ghost pushed a commit to devunrulymedia/Prebid.js that referenced this pull request Jan 30, 2019
* ET-1691: Pulsepoint Analytics adapter for Prebid. (prebid#1)

* ET-1691: Adding pulsepoint analytics and tests for pulsepoint adapter

* ET-1691: Adding pulsepoint analytics and tests for pulsepoint adapter

* ET-1691: cleanup

* ET-1691: minor

* ET-1691: revert package.json change

* Adding bidRequest to bidFactory.createBid method as per prebid#509

* ET-1765: Adding support for additional params in PulsePoint adapter (prebid#2)

* ET-1850: Fixing prebid#866

* Minor fix

* Adding mandatory parameters to Bid

* GDPR Bug Fix with String response

* minor
ghost pushed a commit to devunrulymedia/Prebid.js that referenced this pull request Jan 30, 2019
* Revert "Consent Management module bug fix. (prebid#2588)"

This reverts commit fc95a52.

* use new file for string includes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM needs 2nd review Core module updates require two approvals from the core team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants