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
Add TGT macro for targeting data in json #12707
Conversation
@@ -741,6 +741,10 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A { | |||
return { | |||
PAGEVIEWID: () => Services.documentInfoForDoc(this.element).pageViewId, | |||
HREF: () => this.win.location.href, | |||
TGT: () => { | |||
const json = tryParseJson(this.element.getAttribute('data-json')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TGT: () => (tryParseJson(this.element.getAttribute('data-json')) || {})['targeting'] || '',
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -294,6 +297,7 @@ describes.realWin('DoubleClick Fast Fetch RTC', {amp: true}, env => { | |||
'data-multi-size-validation': macros['data-multi-size-validation'], | |||
'data-override-width': macros['data-OVERRIDE-width'], | |||
'data-override-height': macros['data-override-HEIGHT'], | |||
'data-json': JSON.stringify(json), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does your test coverage handle the case where data-json exists but without targeting key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added test
Hey, @bradfrizzell |
@monish-chhadwa Good catch, follow up PR to fix here: #12841 |
* Add TGT macro for targeting data in json * Respond to feedback * golf * Fix * fix check-types * fix
* Add TGT macro for targeting data in json * Respond to feedback * golf * Fix * fix check-types * fix
See #12374