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

Fixes #2291 - user cust_params data lost with url option for dfpAdServerVideo module #2308

Merged
merged 1 commit into from
Apr 3, 2018

Conversation

jsnellbaker
Copy link
Collaborator

Type of change

  • Bugfix

Description of change

When user was attempting to pass their own cust_params data in a dfp video URL for the dfpAdServerVideo module, the module was dropping the original data and only setting the module's own data.

This fix preserves the original data and merges it with the module's data to make one unified encoded string for the cust_params field.

Note - the params route for the module was handling the merge of the user's cust_params and the module's data fine; this issue only applied to the url route for the module.

Other information

fix for issue #2291

@jsnellbaker jsnellbaker changed the title Fixes #2291 - original cust_params data lost with url option for dfpAdServerVideo module Fixes #2291 - user cust_params data lost with url option for dfpAdServerVideo module Mar 22, 2018
@jaiminpanchal27 jaiminpanchal27 requested review from harpere and mike-chowla and removed request for harpere March 30, 2018 17:27
@jaiminpanchal27 jaiminpanchal27 added LGTM needs 2nd review Core module updates require two approvals from the core team and removed needs review labels Mar 30, 2018
@mkendall07 mkendall07 merged commit d8c2a48 into master Apr 3, 2018
@mkendall07 mkendall07 deleted the bugfix/dfpVideoUrlCustParamsLost branch August 17, 2018 15:10
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.

4 participants