From a757c8419e5d2a58d76ca50ae68d62d5e95c3bf0 Mon Sep 17 00:00:00 2001
From: Avi Mehta <avimehta@google.com>
Date: Fri, 20 May 2016 14:32:07 -0700
Subject: [PATCH] [amp-analytics] Added ability to use variables in selectors.

This allows remote trigger config to specify a config like this:

Remote config:
```
{
  on: 'click',
  selector: '${likeButtonSelector}',
  request: 'foo',
}
```

In page config:
```
{
  vars: { likeButtonSelector: '#myLikeButton' }
}
```
---
 extensions/amp-analytics/0.1/amp-analytics.js | 14 ++++++++--
 .../0.1/test/test-amp-analytics.js            | 28 +++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/extensions/amp-analytics/0.1/amp-analytics.js b/extensions/amp-analytics/0.1/amp-analytics.js
index 3a4176d7b6da..37c11ef339a9 100644
--- a/extensions/amp-analytics/0.1/amp-analytics.js
+++ b/extensions/amp-analytics/0.1/amp-analytics.js
@@ -166,8 +166,18 @@ export class AmpAnalytics extends AMP.BaseElement {
           if (!result) {
             return;
           }
-          addListener(this.getWin(), trigger,
-              this.handleEvent_.bind(this, trigger));
+
+          if (trigger['selector']) {
+            // Expand the selector using variable expansion.
+            trigger['selector'] = this.expandTemplate_(trigger['selector'],
+                trigger);
+            addListener(this.getWin(), trigger, this.handleEvent_.bind(this,
+                  trigger));
+
+          } else {
+            addListener(this.getWin(), trigger,
+                this.handleEvent_.bind(this, trigger));
+          }
         }));
       }
     }
diff --git a/extensions/amp-analytics/0.1/test/test-amp-analytics.js b/extensions/amp-analytics/0.1/test/test-amp-analytics.js
index d1300284d56b..28fd235047ad 100644
--- a/extensions/amp-analytics/0.1/test/test-amp-analytics.js
+++ b/extensions/amp-analytics/0.1/test/test-amp-analytics.js
@@ -16,6 +16,7 @@
 
 import {ANALYTICS_CONFIG} from '../vendors';
 import {AmpAnalytics} from '../amp-analytics';
+import {instrumentationServiceFor} from '../instrumentation';
 import {
   installUserNotificationManager,
 } from '../../../amp-user-notification/0.1/amp-user-notification';
@@ -529,6 +530,33 @@ describe('amp-analytics', function() {
     });
   });
 
+  it('expands selector with config variable', () => {
+    const ins = instrumentationServiceFor(windowApi);
+    const addListenerSpy = sandbox.spy(ins, 'addListener');
+    const analytics = getAnalyticsTag({
+      requests: {foo: 'https://example.com/bar'},
+      triggers: [{on: 'click', selector: '${foo}', request: 'foo'}],
+      vars: {foo: 'bar'},
+    });
+    return waitForNoSendRequest(analytics).then(() => {
+      expect(addListenerSpy.callCount).to.equal(1);
+      expect(addListenerSpy.args[0][0]['selector']).to.equal('bar');
+    });
+  });
+
+  it('does not expands selector with platform variable', () => {
+    const ins = instrumentationServiceFor(windowApi);
+    const addListenerSpy = sandbox.spy(ins, 'addListener');
+    const analytics = getAnalyticsTag({
+      requests: {foo: 'https://example.com/bar'},
+      triggers: [{on: 'click', selector: '${title}', request: 'foo'}],
+    });
+    return waitForNoSendRequest(analytics).then(() => {
+      expect(addListenerSpy.callCount).to.equal(1);
+      expect(addListenerSpy.args[0][0]['selector']).to.equal('TITLE');
+    });
+  });
+
   it('respects optout', function() {
     const config = {
       'requests': {'foo': 'https://example.com/bar'},