FEATURE: Always track clicks using AJAX. (#7373)

app/assets/javascripts/discourse/components/discourse-topic.js.es6

@@ -3,7 +3,6 @@ import AddArchetypeClass from "discourse/mixins/add-archetype-class";
 import ClickTrack from "discourse/lib/click-track";
 import Scrolling from "discourse/mixins/scrolling";
 import MobileScrollDirection from "discourse/mixins/mobile-scroll-direction";
-import { selectedText } from "discourse/lib/utilities";
 import { observes } from "ember-addons/ember-computed-decorators";
 
 const MOBILE_SCROLL_DIRECTION_CHECK_THROTTLE = 300;
@@ -82,17 +81,9 @@ export default Ember.Component.extend(
       $(window).on("resize.discourse-on-scroll", () => this.scrolled());
 
       this.$().on(
-        "mouseup.discourse-redirect",
+        "click.discourse-redirect",
         ".cooked a, a.track-link",
         function(e) {
-          // bypass if we are selecting stuff
-          const selection = window.getSelection && window.getSelection();
-          if (selection.type === "Range" || selection.rangeCount > 0) {
-            if (selectedText() !== "") {
-              return true;
-            }
-          }
-
           const $target = $(e.target);
           if (
             $target.hasClass("mention") ||
@@ -116,7 +107,7 @@ export default Ember.Component.extend(
       $(window).unbind("resize.discourse-on-scroll");
 
       // Unbind link tracking
-      this.$().off("mouseup.discourse-redirect", ".cooked a, a.track-link");
+      this.$().off("click.discourse-redirect", ".cooked a, a.track-link");
 
       this.resetExamineDockCache();
 

app/assets/javascripts/discourse/components/links-redirect.js.es6

@@ -1,19 +1,10 @@
 import ClickTrack from "discourse/lib/click-track";
-import { selectedText } from "discourse/lib/utilities";
 
 export default Ember.Component.extend({
   didInsertElement() {
     this._super(...arguments);
 
-    this.$().on("mouseup.discourse-redirect", "#revisions a", function(e) {
+    this.$().on("click.discourse-redirect", "#revisions a", function(e) {
-      // bypass if we are selecting stuff
-      const selection = window.getSelection && window.getSelection();
-      if (selection.type === "Range" || selection.rangeCount > 0) {
-        if (selectedText() !== "") {
-          return true;
-        }
-      }
-
       const $target = $(e.target);
       if (
         $target.hasClass("mention") ||
@@ -28,6 +19,6 @@ export default Ember.Component.extend({
 
   willDestroyElement() {
     this._super(...arguments);
-    this.$().off("mouseup.discourse-redirect", "#revisions a");
+    this.$().off("click.discourse-redirect", "#revisions a");
   }
 });

app/assets/javascripts/discourse/components/user-stream.js.es6

@@ -1,6 +1,5 @@
 import LoadMore from "discourse/mixins/load-more";
 import ClickTrack from "discourse/lib/click-track";
-import { selectedText } from "discourse/lib/utilities";
 import Post from "discourse/models/post";
 import DiscourseURL from "discourse/lib/url";
 import Draft from "discourse/models/draft";
@@ -32,15 +31,7 @@ export default Ember.Component.extend(LoadMore, {
     $(window).on("resize.discourse-on-scroll", () => this.scrolled());
 
     this.$().on("click.details-disabled", "details.disabled", () => false);
-    this.$().on("mouseup.discourse-redirect", ".excerpt a", function(e) {
+    this.$().on("click.discourse-redirect", ".excerpt a", function(e) {
-      // bypass if we are selecting stuff
-      const selection = window.getSelection && window.getSelection();
-      if (selection.type === "Range" || selection.rangeCount > 0) {
-        if (selectedText() !== "") {
-          return true;
-        }
-      }
-
       const $target = $(e.target);
       if (
         $target.hasClass("mention") ||
@@ -60,7 +51,7 @@ export default Ember.Component.extend(LoadMore, {
     this.$().off("click.details-disabled", "details.disabled");
 
     // Unbind link tracking
-    this.$().off("mouseup.discourse-redirect", ".excerpt a");
+    this.$().off("click.discourse-redirect", ".excerpt a");
   }.on("willDestroyElement"),
 
   actions: {

app/assets/javascripts/discourse/lib/click-track.js.es6

@@ -4,6 +4,21 @@ import { wantsNewWindow } from "discourse/lib/intercept-click";
 import { selectedText } from "discourse/lib/utilities";
 
 export function isValidLink($link) {
+  // Do not track:
+  //  - lightboxes
+  //  - group mentions
+  //  - links with disabled tracking
+  //  - category links
+  //  - quote back button
+  if ($link.is(".lightbox, .mention-group, .no-track-link, .hashtag, .back")) {
+    return false;
+  }
+
+  // Do not track links in quotes or in elided part
+  if ($link.parents("aside.quote, .elided").length !== 0) {
+    return false;
+  }
+
   return (
     $link.hasClass("track-link") ||
     $link.closest(".hashtag, .badge-category, .onebox-result, .onebox-body")
@@ -18,34 +33,36 @@ export default {
       return true;
     }
 
-    // cancel click if triggered as part of selection.
+    // Cancel click if triggered as part of selection.
+    const selection = window.getSelection();
+    if (selection.type === "Range" || selection.rangeCount > 0) {
       if (selectedText() !== "") {
-      return false;
+        return true;
+      }
     }
 
     const $link = $(e.currentTarget);
-
+    if (!isValidLink($link)) {
-    // don't track
-    //   - lightboxes
-    //   - group mentions
-    //   - links with disabled tracking
-    //   - category links
-    //   - quote back button
-    if (
-      $link.is(".lightbox, .mention-group, .no-track-link, .hashtag, .back")
-    ) {
       return true;
     }
 
-    // don't track links in quotes or in elided part
+    if ($link.hasClass("attachment")) {
-    let tracking = $link.parents("aside.quote, .elided").length === 0;
+      // Warn the user if they cannot download the file.
-
+      if (
-    let href = $link.attr("href") || $link.data("href");
+        Discourse.SiteSettings.prevent_anons_from_downloading_files &&
-
+        !Discourse.User.current()
-    if (!href || href.trim().length === 0) {
+      ) {
+        bootbox.alert(I18n.t("post.errors.attachment_download_requires_login"));
         return false;
       }
-    if (href.indexOf("mailto:") === 0) {
+
+      return true;
+    }
+
+    let href = ($link.attr("href") || $link.data("href")).trim();
+    if (!href) {
+      return false;
+    } else if (href.indexOf("mailto:") === 0) {
       return true;
     }
 
@@ -57,26 +74,10 @@ export default {
     const userId = $link.data("user-id") || $article.data("user-id");
     const ownLink = userId && userId === Discourse.User.currentProp("id");
 
-    let destUrl = href;
+    // Update badge clicks unless it's our own.
-
-    if (tracking) {
-      destUrl = Discourse.getURL(
-        "/clicks/track?url=" + encodeURIComponent(href)
-      );
-
-      if (postId && !$link.data("ignore-post-id")) {
-        destUrl += "&post_id=" + encodeURI(postId);
-      }
-      if (topicId) {
-        destUrl += "&topic_id=" + encodeURI(topicId);
-      }
-
-      // Update badge clicks unless it's our own
     if (!ownLink) {
       const $badge = $("span.badge", $link);
       if ($badge.length === 1) {
-          // don't update counts in category badge nor in oneboxes (except when we force it)
-          if (isValidLink($link)) {
         const html = $badge.html();
         const key = `${new Date().toLocaleDateString()}-${postId}-${href}`;
         if (/^\d+$/.test(html) && !sessionStorage.getItem(key)) {
@@ -85,91 +86,52 @@ export default {
         }
       }
     }
-      }
-    }
 
-    // if they want to open in a new tab, do an AJAX request
+    const trackPromise = ajax("/clicks/track", {
-    if (tracking && wantsNewWindow(e)) {
-      ajax("/clicks/track", {
       data: {
         url: href,
         post_id: postId,
-          topic_id: topicId,
+        topic_id: topicId
-          redirect: false
+      }
-        },
-        dataType: "html"
     });
-      return true;
-    }
-
-    e.preventDefault();
-
-    // Remove the href, put it as a data attribute
-    if (!$link.data("href")) {
-      $link.addClass("no-href");
-      $link.data("href", $link.attr("href"));
-      $link.attr("href", null);
-      // Don't route to this URL
-      $link.data("auto-route", true);
-    }
-
-    // restore href
-    Ember.run.later(() => {
-      $link.removeClass("no-href");
-      $link.attr("href", $link.data("href"));
-      $link.data("href", null);
-    }, 50);
-
-    // warn the user if they can't download the file
-    if (
-      Discourse.SiteSettings.prevent_anons_from_downloading_files &&
-      $link.hasClass("attachment") &&
-      !Discourse.User.current()
-    ) {
-      bootbox.alert(I18n.t("post.errors.attachment_download_requires_login"));
-      return false;
-    }
 
     const isInternal = DiscourseURL.isInternal(href);
-
-    const modifierLeftClicked = (e.ctrlKey || e.metaKey) && e.which === 1;
-    const middleClicked = e.which === 2;
     const openExternalInNewTab = Discourse.User.currentProp(
       "external_links_in_new_tab"
     );
 
-    const openWindow =
+    if (!wantsNewWindow(e)) {
-      modifierLeftClicked ||
+      if (!isInternal && openExternalInNewTab) {
-      middleClicked ||
+        window.open(href, "_blank").focus();
-      (!isInternal && openExternalInNewTab);
 
-    // If we're on the same site, use the router and track via AJAX
+        // Hack to prevent changing current window.location.
-    if (isInternal && !$link.hasClass("attachment")) {
+        // e.preventDefault() does not work.
-      if (tracking) {
+        if (!$link.data("href")) {
-        ajax("/clicks/track", {
+          $link.addClass("no-href");
-          data: {
+          $link.data("href", $link.attr("href"));
-            url: href,
+          $link.attr("href", null);
-            post_id: postId,
+          $link.data("auto-route", true);
-            topic_id: topicId,
+
-            redirect: false
+          Ember.run.later(() => {
-          },
+            $link.removeClass("no-href");
-          dataType: "html"
+            $link.attr("href", $link.data("href"));
+            $link.data("href", null);
+            $link.data("auto-route", null);
+          }, 50);
+        }
+      } else {
+        trackPromise.finally(() => {
+          if (isInternal) {
+            DiscourseURL.routeTo(href);
+          } else {
+            DiscourseURL.redirectTo(href);
+          }
         });
       }
-      if (openWindow) {
+
-        window.open(destUrl, "_blank").focus();
-      } else {
-        DiscourseURL.routeTo(href);
-      }
       return false;
     }
 
-    if (openWindow) {
+    return true;
-      window.open(destUrl, "_blank").focus();
-    } else {
-      DiscourseURL.redirectTo(destUrl);
-    }
-
-    return false;
   }
 };

app/controllers/clicks_controller.rb

@@ -1,39 +1,18 @@
 class ClicksController < ApplicationController
-  layout "no_ember"
-
   skip_before_action :check_xhr, :preload_json
 
   def track
-    raise Discourse::NotFound unless params[:url]
+    params.require([:url, :post_id, :topic_id])
 
-    params = track_params.merge(ip: request.remote_ip)
+    TopicLinkClick.create_from(
+      url: params[:url],
+      post_id: params[:post_id],
+      topic_id: params[:topic_id],
+      ip: request.remote_ip,
+      user_id: current_user&.id
+    )
 
-    if params[:topic_id].present? || params[:post_id].present?
+    render json: success_json
-      params.merge!(user_id: current_user.id) if current_user.present?
-      @redirect_url = TopicLinkClick.create_from(params)
-    end
-
-    # links in whispers aren't extracted, just allow the redirection to staff
-    if @redirect_url.blank? && current_user&.staff? && params[:post_id].present?
-      @redirect_url = params[:url] if Post.exists?(id: params[:post_id], post_type: Post.types[:whisper])
-    end
-
-    # Sometimes we want to record a link without a 302.
-    # Since XHR has to load the redirected URL we want it to not return a 302 in those cases.
-    if params[:redirect] == "false"
-      render body: nil
-    elsif @redirect_url.present?
-      redirect_to(@redirect_url)
-    else
-      render
-    end
-  end
-
-  private
-
-  def track_params
-    params.require(:url)
-    params.permit(:url, :post_id, :topic_id, :redirect)
   end
 
 end

app/views/clicks/track.html.erb

@@ -1,4 +0,0 @@
-<div class="container redirection-warning">
-  <p><%= I18n.t("redirect_warning") %></p>
-  <p><%= sanitize link_to params[:url], params[:url] %></p>
-</div>

spec/requests/clicks_controller_spec.rb

@@ -1,61 +1,18 @@
 require 'rails_helper'
 
 describe ClicksController do
-  context 'create' do
-    context 'missing params' do
-      it 'raises a 404 without a url' do
-        get "/clicks/track", params: { post_id: 123 }
-        expect(response).to be_not_found
-      end
-    end
 
-    context 'correct params' do
   let(:url) { "https://discourse.org/" }
   let(:headers) { { REMOTE_ADDR: "192.168.0.1" } }
   let(:post) { create_post(raw: "this is a post with a link #{url}") }
 
-      context "with a made up url" do
+  context '#track' do
-        it "doesn't redirect" do
+    it "creates a TopicLinkClick" do
-          get "/clicks/track", params: { url: 'https://fakewebsite.com', post_id: post.id }, headers: headers
+      sign_in(Fabricate(:user))
-          expect(response).not_to be_redirect
-          expect(response.body).to include(I18n.t("redirect_warning"))
-        end
-      end
 
-      context "with a valid url" do
+      expect {
-        it "redirects" do
+        get "/clicks/track", params: { url: url, post_id: post.id, topic_id: post.topic_id }, headers: headers
-          get "/clicks/track", params: { url: 'https://discourse.org/?hello=123', post_id: post.id }, headers: headers
+      }.to change { TopicLinkClick.count }.by(1)
-          expect(response).to redirect_to("https://discourse.org/?hello=123")
-        end
-      end
-
-      context 'with a post_id' do
-        it 'redirects' do
-          get "/clicks/track", params: { url: url, post_id: post.id }, headers: headers
-          expect(response).to redirect_to(url)
-        end
-
-        it "redirects links in whispers to staff members" do
-          sign_in(Fabricate(:admin))
-          whisper = Fabricate(:post, post_type: Post.types[:whisper])
-
-          get "/clicks/track", params: { url: url, post_id: whisper.id }, headers: headers
-
-          expect(response).to redirect_to(url)
-        end
-
-        it "doesn't redirect with the redirect=false param" do
-          get "/clicks/track", params: { url: url, post_id: post.id, redirect: 'false' }, headers: headers
-          expect(response).not_to be_redirect
-        end
-      end
-
-      context 'with a topic_id' do
-        it 'redirects' do
-          get "/clicks/track", params: { url: url, topic_id: post.topic.id }, headers: headers
-          expect(response).to redirect_to(url)
-        end
-      end
     end
   end
 end

test/javascripts/lib/click-track-edit-history-test.js.es6

@@ -1,16 +1,20 @@
 import DiscourseURL from "discourse/lib/url";
 import ClickTrack from "discourse/lib/click-track";
-
+import { logIn } from "helpers/qunit-helpers";
-var windowOpen, win, redirectTo;
 
 QUnit.module("lib:click-track-edit-history", {
   beforeEach() {
-    // Prevent any of these tests from navigating away
+    logIn();
-    win = { focus: function() {} };
+
-    redirectTo = sandbox.stub(DiscourseURL, "redirectTo");
+    let win = { focus: function() {} };
-    windowOpen = sandbox.stub(window, "open").returns(win);
+    sandbox.stub(window, "open").returns(win);
     sandbox.stub(win, "focus");
 
+    sandbox.stub(DiscourseURL, "routeTo");
+    sandbox.stub(DiscourseURL, "redirectTo");
+
+    sessionStorage.clear();
+
     fixture().html(
       `<div id="topic" data-topic-id="1337">
        </div>
@@ -48,132 +52,63 @@ QUnit.module("lib:click-track-edit-history", {
 
 var track = ClickTrack.trackClick;
 
-// test
+function generateClickEventOn(selector) {
-var generateClickEventOn = function(selector) {
+  return $.Event("click", { currentTarget: fixture(selector).first() });
-  return $.Event("click", { currentTarget: fixture(selector)[0] });
+}
-};
 
-QUnit.test("does not track clicks on lightboxes", assert => {
+QUnit.test("tracks internal URLs", async assert => {
-  var clickEvent = generateClickEventOn(".lightbox");
+  assert.expect(2);
-  sandbox.stub(clickEvent, "preventDefault");
+  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-  assert.ok(track(clickEvent));
-  assert.ok(!clickEvent.preventDefault.calledOnce);
-});
-
-QUnit.test("it calls preventDefault when clicking on an a", assert => {
-  var clickEvent = generateClickEventOn("a");
-  sandbox.stub(clickEvent, "preventDefault");
-  track(clickEvent);
-  assert.ok(clickEvent.preventDefault.calledOnce);
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-QUnit.test("does not track clicks when forcibly disabled", assert => {
-  assert.ok(track(generateClickEventOn(".no-track-link")));
-});
-
-QUnit.test("does not track clicks on back buttons", assert => {
-  assert.ok(track(generateClickEventOn(".back")));
-});
-
-QUnit.test("does not track clicks on category badges", assert => {
-  assert.ok(track(generateClickEventOn(".hashtag")));
-});
-
-QUnit.test("removes the href and put it as a data attribute", assert => {
-  track(generateClickEventOn("a"));
-
-  var $link = fixture("a").first();
-  assert.ok($link.hasClass("no-href"));
-  assert.equal($link.data("href"), "http://www.google.com");
-  assert.blank($link.attr("href"));
-  assert.ok($link.data("auto-route"));
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-asyncTestDiscourse("restores the href after a while", function(assert) {
-  assert.expect(1);
-
-  track(generateClickEventOn("a"));
 
   const done = assert.async();
-  setTimeout(function() {
+  /* global server */
+  server.get("/clicks/track", request => {
+    assert.ok(
+      request.url.indexOf(
+        "url=http%3A%2F%2Fdiscuss.domain.com&post_id=42&topic_id=1337"
+      ) !== -1
+    );
     done();
-    assert.equal(fixture("a").attr("href"), "http://www.google.com");
-  }, 75);
   });
 
-var testOpenInANewTab = function(description, clickEventModifier) {
+  assert.notOk(track(generateClickEventOn("#same-site")));
-  test(description, function(assert) {
-    var clickEvent = generateClickEventOn("a");
-    clickEventModifier(clickEvent);
-    sandbox.stub(clickEvent, "preventDefault");
-    assert.ok(track(clickEvent));
-    assert.ok(!clickEvent.preventDefault.calledOnce);
-  });
-};
-
-testOpenInANewTab("it opens in a new tab when pressing shift", function(
-  clickEvent
-) {
-  clickEvent.shiftKey = true;
 });
 
-testOpenInANewTab("it opens in a new tab when pressing meta", function(
+QUnit.test("tracks external URLs", async assert => {
-  clickEvent
+  assert.expect(2);
-) {
-  clickEvent.metaKey = true;
-});
 
-testOpenInANewTab("it opens in a new tab when pressing ctrl", function(
+  const done = assert.async();
-  clickEvent
+  /* global server */
-) {
+  server.get("/clicks/track", request => {
-  clickEvent.ctrlKey = true;
-});
-
-testOpenInANewTab("it opens in a new tab on middle click", function(
-  clickEvent
-) {
-  clickEvent.button = 2;
-});
-
-QUnit.test("tracks via AJAX if we're on the same site", assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  assert.ok(!track(generateClickEventOn("#same-site")));
-  assert.ok(DiscourseURL.routeTo.calledOnce);
-});
-
-QUnit.test("does not track via AJAX for attachments", assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  assert.ok(!track(generateClickEventOn(".attachment")));
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-QUnit.test("tracks custom urls when opening in another window", assert => {
-  var clickEvent = generateClickEventOn("a");
-  sandbox
-    .stub(Discourse.User, "currentProp")
-    .withArgs("external_links_in_new_tab")
-    .returns(true);
-  assert.ok(!track(clickEvent));
     assert.ok(
-    windowOpen.calledWith(
+      request.url.indexOf(
-      "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337",
+        "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
-      "_blank"
+      ) !== -1
-    )
     );
+    done();
   });
 
-QUnit.test("tracks custom urls when opening in another window", assert => {
+  assert.notOk(track(generateClickEventOn("a")));
-  var clickEvent = generateClickEventOn("a");
-  assert.ok(!track(clickEvent));
-  assert.ok(
-    redirectTo.calledWith(
-      "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
-    )
-  );
 });
+
+QUnit.test(
+  "tracks external URLs when opening in another window",
+  async assert => {
+    assert.expect(3);
+    Discourse.User.currentProp("external_links_in_new_tab", true);
+
+    const done = assert.async();
+    /* global server */
+    server.get("/clicks/track", request => {
+      assert.ok(
+        request.url.indexOf(
+          "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
+        ) !== -1
+      );
+      done();
+    });
+
+    assert.notOk(track(generateClickEventOn("a")));
+    assert.ok(window.open.calledWith("http://www.google.com", "_blank"));
+  }
+);

test/javascripts/lib/click-track-profile-page-test.js.es6

@@ -1,16 +1,20 @@
 import DiscourseURL from "discourse/lib/url";
 import ClickTrack from "discourse/lib/click-track";
-
+import { logIn } from "helpers/qunit-helpers";
-var windowOpen, win, redirectTo;
 
 QUnit.module("lib:click-track-profile-page", {
   beforeEach() {
-    // Prevent any of these tests from navigating away
+    logIn();
-    win = { focus: function() {} };
+
-    redirectTo = sandbox.stub(DiscourseURL, "redirectTo");
+    let win = { focus: function() {} };
-    windowOpen = sandbox.stub(window, "open").returns(win);
+    sandbox.stub(window, "open").returns(win);
     sandbox.stub(win, "focus");
 
+    sandbox.stub(DiscourseURL, "routeTo");
+    sandbox.stub(DiscourseURL, "redirectTo");
+
+    sessionStorage.clear();
+
     fixture().html(
       `<p class="excerpt first" data-post-id="42" data-topic-id="1337" data-user-id="3141">
         <a href="http://www.google.com">google.com</a>
@@ -42,163 +46,56 @@ QUnit.module("lib:click-track-profile-page", {
 
 var track = ClickTrack.trackClick;
 
-// test
+function generateClickEventOn(selector) {
-var generateClickEventOn = function(selector) {
+  return $.Event("click", { currentTarget: fixture(selector).first() });
-  return $.Event("click", { currentTarget: fixture(selector)[0] });
+}
-};
 
-QUnit.test("does not track clicks on lightboxes", assert => {
+QUnit.test("tracks internal URLs", async assert => {
-  var clickEvent = generateClickEventOn(".lightbox");
+  assert.expect(2);
-  sandbox.stub(clickEvent, "preventDefault");
+  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-  assert.ok(track(clickEvent));
-  assert.ok(!clickEvent.preventDefault.calledOnce);
-});
-
-QUnit.test("it calls preventDefault when clicking on an a", assert => {
-  var clickEvent = generateClickEventOn("a");
-  sandbox.stub(clickEvent, "preventDefault");
-  track(clickEvent);
-  assert.ok(clickEvent.preventDefault.calledOnce);
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-QUnit.test("does not track clicks when forcibly disabled", assert => {
-  assert.ok(track(generateClickEventOn(".no-track-link")));
-});
-
-QUnit.test("does not track clicks on back buttons", assert => {
-  assert.ok(track(generateClickEventOn(".back")));
-});
-
-QUnit.test("does not track clicks on category badges", assert => {
-  assert.ok(track(generateClickEventOn(".hashtag")));
-});
-
-QUnit.test("removes the href and put it as a data attribute", assert => {
-  track(generateClickEventOn("a"));
-
-  var $link = fixture("a").first();
-  assert.ok($link.hasClass("no-href"));
-  assert.equal($link.data("href"), "http://www.google.com");
-  assert.blank($link.attr("href"));
-  assert.ok($link.data("auto-route"));
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-asyncTestDiscourse("restores the href after a while", function(assert) {
-  assert.expect(1);
-
-  track(generateClickEventOn("a"));
 
   const done = assert.async();
-  setTimeout(function() {
+  /* global server */
+  server.get("/clicks/track", request => {
+    assert.ok(
+      request.url.indexOf("url=http%3A%2F%2Fdiscuss.domain.com") !== -1
+    );
     done();
-    assert.equal(fixture("a").attr("href"), "http://www.google.com");
-  }, 75);
   });
 
-var testOpenInANewTab = function(description, clickEventModifier) {
+  assert.notOk(track(generateClickEventOn("#same-site")));
-  test(description, function(assert) {
-    var clickEvent = generateClickEventOn("a");
-    clickEventModifier(clickEvent);
-    sandbox.stub(clickEvent, "preventDefault");
-    assert.ok(track(clickEvent));
-    assert.ok(!clickEvent.preventDefault.calledOnce);
-  });
-};
-
-testOpenInANewTab("it opens in a new tab when pressing shift", function(
-  clickEvent
-) {
-  clickEvent.shiftKey = true;
 });
 
-testOpenInANewTab("it opens in a new tab when pressing meta", function(
+QUnit.test("tracks external URLs", async assert => {
-  clickEvent
+  assert.expect(2);
-) {
-  clickEvent.metaKey = true;
-});
 
-testOpenInANewTab("it opens in a new tab when pressing ctrl", function(
+  const done = assert.async();
-  clickEvent
+  /* global server */
-) {
+  server.get("/clicks/track", request => {
-  clickEvent.ctrlKey = true;
-});
-
-testOpenInANewTab("it opens in a new tab on middle click", function(
-  clickEvent
-) {
-  clickEvent.button = 2;
-});
-
-QUnit.test("tracks via AJAX if we're on the same site", assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  assert.ok(!track(generateClickEventOn("#same-site")));
-  assert.ok(DiscourseURL.routeTo.calledOnce);
-});
-
-QUnit.test("does not track via AJAX for attachments", assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  assert.ok(!track(generateClickEventOn(".attachment")));
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-QUnit.test("tracks custom urls when opening in another window", assert => {
-  var clickEvent = generateClickEventOn("a");
-  sandbox
-    .stub(Discourse.User, "currentProp")
-    .withArgs("external_links_in_new_tab")
-    .returns(true);
-  assert.ok(!track(clickEvent));
     assert.ok(
-    windowOpen.calledWith(
+      request.url.indexOf(
-      "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337",
+        "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
-      "_blank"
+      ) !== -1
-    )
     );
+    done();
   });
 
-QUnit.test(
+  assert.notOk(track(generateClickEventOn("a")));
-  "tracks custom urls on second excerpt when opening in another window",
-  assert => {
-    var clickEvent = generateClickEventOn(".second a");
-    sandbox
-      .stub(Discourse.User, "currentProp")
-      .withArgs("external_links_in_new_tab")
-      .returns(true);
-    assert.ok(!track(clickEvent));
-    assert.ok(
-      windowOpen.calledWith(
-        "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=24&topic_id=7331",
-        "_blank"
-      )
-    );
-  }
-);
-
-QUnit.test("tracks custom urls when opening in another window", assert => {
-  var clickEvent = generateClickEventOn("a");
-  assert.ok(!track(clickEvent));
-  assert.ok(
-    redirectTo.calledWith(
-      "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
-    )
-  );
 });
 
-QUnit.test(
+QUnit.test("tracks external URLs in other posts", async assert => {
-  "tracks custom urls on second excerpt when opening in another window",
+  assert.expect(2);
-  assert => {
+
-    var clickEvent = generateClickEventOn(".second a");
+  const done = assert.async();
-    assert.ok(!track(clickEvent));
+  /* global server */
+  server.get("/clicks/track", request => {
     assert.ok(
-      redirectTo.calledWith(
+      request.url.indexOf(
-        "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=24&topic_id=7331"
+        "url=http%3A%2F%2Fwww.google.com&post_id=24&topic_id=7331"
-      )
+      ) !== -1
-    );
-  }
     );
+    done();
+  });
+
+  assert.notOk(track(generateClickEventOn(".second a")));
+});

test/javascripts/lib/click-track-test.js.es6

@@ -1,16 +1,18 @@
 import DiscourseURL from "discourse/lib/url";
 import ClickTrack from "discourse/lib/click-track";
-
+import { logIn } from "helpers/qunit-helpers";
-var windowOpen, win, redirectTo;
 
 QUnit.module("lib:click-track", {
   beforeEach() {
-    // Prevent any of these tests from navigating away
+    logIn();
-    win = { focus: function() {} };
+
-    redirectTo = sandbox.stub(DiscourseURL, "redirectTo");
+    let win = { focus: function() {} };
-    windowOpen = sandbox.stub(window, "open").returns(win);
+    sandbox.stub(window, "open").returns(win);
     sandbox.stub(win, "focus");
 
+    sandbox.stub(DiscourseURL, "routeTo");
+    sandbox.stub(DiscourseURL, "redirectTo");
+
     sessionStorage.clear();
 
     fixture().html(
@@ -41,100 +43,125 @@ QUnit.module("lib:click-track", {
 
 var track = ClickTrack.trackClick;
 
-// test
+function generateClickEventOn(selector) {
-var generateClickEventOn = function(selector) {
+  return $.Event("click", { currentTarget: fixture(selector).first() });
-  return $.Event("click", { currentTarget: fixture(selector)[0] });
+}
-};
 
-QUnit.test("does not track clicks on lightboxes", assert => {
+QUnit.test("tracks internal URLs", async assert => {
+  assert.expect(2);
+  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
+
+  const done = assert.async();
+  /* global server */
+  server.get("/clicks/track", request => {
+    assert.ok(
+      request.url.indexOf(
+        "url=http%3A%2F%2Fdiscuss.domain.com&post_id=42&topic_id=1337"
+      ) !== -1
+    );
+    done();
+  });
+
+  assert.notOk(track(generateClickEventOn("#same-site")));
+});
+
+QUnit.test("does not track attachments", async assert => {
+  assert.expect(1);
+  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
+
+  /* global server */
+  server.get("/clicks/track", () => assert.ok(false));
+
+  assert.ok(track(generateClickEventOn(".attachment")));
+});
+
+QUnit.test("tracks external URLs", async assert => {
+  assert.expect(2);
+
+  const done = assert.async();
+  /* global server */
+  server.get("/clicks/track", request => {
+    assert.ok(
+      request.url.indexOf(
+        "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
+      ) !== -1
+    );
+    done();
+  });
+
+  assert.notOk(track(generateClickEventOn("a")));
+});
+
+QUnit.test(
+  "tracks external URLs when opening in another window",
+  async assert => {
+    assert.expect(3);
+    Discourse.User.currentProp("external_links_in_new_tab", true);
+
+    const done = assert.async();
+    /* global server */
+    server.get("/clicks/track", request => {
+      assert.ok(
+        request.url.indexOf(
+          "url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
+        ) !== -1
+      );
+      done();
+    });
+
+    assert.notOk(track(generateClickEventOn("a")));
+    assert.ok(window.open.calledWith("http://www.google.com", "_blank"));
+  }
+);
+
+QUnit.test("does not track clicks on lightboxes", async assert => {
   var clickEvent = generateClickEventOn(".lightbox");
-  sandbox.stub(clickEvent, "preventDefault");
   assert.ok(track(clickEvent));
-  assert.ok(!clickEvent.preventDefault.calledOnce);
 });
 
-QUnit.test("it calls preventDefault when clicking on an a", assert => {
+QUnit.test("does not track clicks when forcibly disabled", async assert => {
-  var clickEvent = generateClickEventOn("a");
-  sandbox.stub(clickEvent, "preventDefault");
-  track(clickEvent);
-  assert.ok(clickEvent.preventDefault.calledOnce);
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-QUnit.test("does not track clicks when forcibly disabled", assert => {
   assert.ok(track(generateClickEventOn(".no-track-link")));
 });
 
-QUnit.test("does not track clicks on back buttons", assert => {
+QUnit.test("does not track clicks on back buttons", async assert => {
   assert.ok(track(generateClickEventOn(".back")));
 });
 
-QUnit.test("does not track right clicks inside quotes", assert => {
+QUnit.test("does not track right clicks inside quotes", async assert => {
   const event = generateClickEventOn(".quote a:first-child");
   event.which = 3;
   assert.ok(track(event));
 });
 
-QUnit.test("does not track clicks to internal links in quotes", assert => {
+QUnit.test("does not track clicks links in quotes", async assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
+  assert.ok(track(generateClickEventOn(".quote a:last-child")));
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  track(generateClickEventOn(".quote a:first-child"));
-  assert.ok(
-    DiscourseURL.routeTo.calledWith(
-      "https://discuss.domain.com/t/welcome-to-meta-discourse-org/1/30"
-    )
-  );
 });
 
-QUnit.test("can open links inside quotes in new window", assert => {
+QUnit.test("does not track clicks on category badges", async assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  track(
-    $.Event("click", {
-      currentTarget: fixture(".quote a:first-child")[0],
-      ctrlKey: true,
-      which: 1
-    })
-  );
-
-  assert.ok(
-    window.open.calledWith(
-      "https://discuss.domain.com/t/welcome-to-meta-discourse-org/1/30",
-      "_blank"
-    )
-  );
-});
-
-QUnit.test("does not track clicks to external links in quotes", assert => {
-  track(generateClickEventOn(".quote a:last-child"));
-  assert.ok(DiscourseURL.redirectTo.calledWith("https://google.com"));
-});
-
-QUnit.test("does not track clicks on category badges", assert => {
   assert.ok(track(generateClickEventOn(".hashtag")));
 });
 
-QUnit.test("does not track clicks on mailto", assert => {
+QUnit.test("does not track clicks on mailto", async assert => {
   assert.ok(track(generateClickEventOn(".mailto")));
 });
 
-QUnit.test("removes the href and put it as a data attribute", assert => {
+QUnit.test("removes the href and put it as a data attribute", async assert => {
-  track(generateClickEventOn("a"));
+  Discourse.User.currentProp("external_links_in_new_tab", true);
+
+  assert.notOk(track(generateClickEventOn("a")));
 
   var $link = fixture("a").first();
   assert.ok($link.hasClass("no-href"));
   assert.equal($link.data("href"), "http://www.google.com");
   assert.blank($link.attr("href"));
   assert.ok($link.data("auto-route"));
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
+  assert.ok(window.open.calledWith("http://www.google.com", "_blank"));
 });
 
-asyncTestDiscourse("restores the href after a while", assert => {
+asyncTestDiscourse("restores the href after a while", async assert => {
-  assert.expect(1);
+  assert.expect(2);
 
-  track(generateClickEventOn("a"));
+  assert.notOk(track(generateClickEventOn("a")));
 
   const done = assert.async();
   setTimeout(function() {
@@ -143,13 +170,13 @@ asyncTestDiscourse("restores the href after a while", assert => {
   }, 75);
 });
 
-var badgeClickCount = function(assert, id, expected) {
+function badgeClickCount(assert, id, expected) {
   track(generateClickEventOn("#" + id));
   var $badge = $("span.badge", fixture("#" + id).first());
   assert.equal(parseInt($badge.html(), 10), expected);
-};
+}
 
-QUnit.test("does not update badge clicks on my own link", assert => {
+QUnit.test("does not update badge clicks on my own link", async assert => {
   sandbox
     .stub(Discourse.User, "currentProp")
     .withArgs("id")
@@ -157,7 +184,7 @@ QUnit.test("does not update badge clicks on my own link", assert => {
   badgeClickCount(assert, "with-badge", 1);
 });
 
-QUnit.test("does not update badge clicks in my own post", assert => {
+QUnit.test("does not update badge clicks in my own post", async assert => {
   sandbox
     .stub(Discourse.User, "currentProp")
     .withArgs("id")
@@ -165,87 +192,33 @@ QUnit.test("does not update badge clicks in my own post", assert => {
   badgeClickCount(assert, "with-badge-but-not-mine", 1);
 });
 
-QUnit.test("updates badge counts correctly", assert => {
+QUnit.test("updates badge counts correctly", async assert => {
   badgeClickCount(assert, "inside-onebox", 1);
   badgeClickCount(assert, "inside-onebox-forced", 2);
   badgeClickCount(assert, "with-badge", 2);
 });
 
-var testOpenInANewTab = function(description, clickEventModifier) {
+function testOpenInANewTab(description, clickEventModifier) {
-  test(description, assert => {
+  test(description, async assert => {
     var clickEvent = generateClickEventOn("a");
     clickEventModifier(clickEvent);
-    sandbox.stub(clickEvent, "preventDefault");
     assert.ok(track(clickEvent));
-    assert.ok(!clickEvent.preventDefault.calledOnce);
+    assert.notOk(clickEvent.defaultPrevented);
   });
-};
+}
 
-testOpenInANewTab("it opens in a new tab when pressing shift", function(
+testOpenInANewTab("it opens in a new tab when pressing shift", clickEvent => {
-  clickEvent
-) {
   clickEvent.shiftKey = true;
 });
 
-testOpenInANewTab("it opens in a new tab when pressing meta", function(
+testOpenInANewTab("it opens in a new tab when pressing meta", clickEvent => {
-  clickEvent
-) {
   clickEvent.metaKey = true;
 });
 
-testOpenInANewTab("it opens in a new tab when pressing ctrl", function(
+testOpenInANewTab("it opens in a new tab when pressing ctrl", clickEvent => {
-  clickEvent
-) {
   clickEvent.ctrlKey = true;
 });
 
-testOpenInANewTab("it opens in a new tab on middle click", function(
+testOpenInANewTab("it opens in a new tab on middle click", clickEvent => {
-  clickEvent
-) {
   clickEvent.button = 2;
 });
-
-QUnit.test("tracks via AJAX if we're on the same site", assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  assert.ok(!track(generateClickEventOn("#same-site")));
-  assert.ok(DiscourseURL.routeTo.calledOnce);
-});
-
-QUnit.test("does not track via AJAX for attachments", assert => {
-  sandbox.stub(DiscourseURL, "routeTo");
-  sandbox.stub(DiscourseURL, "origin").returns("http://discuss.domain.com");
-
-  assert.ok(!track(generateClickEventOn(".attachment")));
-  assert.ok(DiscourseURL.redirectTo.calledOnce);
-});
-
-QUnit.test("tracks custom urls when opening in another window", function(
-  assert
-) {
-  var clickEvent = generateClickEventOn("a");
-  sandbox
-    .stub(Discourse.User, "currentProp")
-    .withArgs("external_links_in_new_tab")
-    .returns(true);
-  assert.ok(!track(clickEvent));
-  assert.ok(
-    windowOpen.calledWith(
-      "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337",
-      "_blank"
-    )
-  );
-});
-
-QUnit.test("tracks custom urls when opening in another window", function(
-  assert
-) {
-  var clickEvent = generateClickEventOn("a");
-  assert.ok(!track(clickEvent));
-  assert.ok(
-    redirectTo.calledWith(
-      "/clicks/track?url=http%3A%2F%2Fwww.google.com&post_id=42&topic_id=1337"
-    )
-  );
-});