From 38cbca3f67ca2e218bbf3bd2bf5d8ea9e4021c19 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Fri, 18 Feb 2022 14:47:56 +0200 Subject: [PATCH] FIX: Count clicks on links with query params (#15969) This did not work sometimes if a topic had the same URL with and without query params because it did not try to select the best matching URL. --- app/models/topic_link_click.rb | 8 ++++---- spec/models/topic_link_click_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/app/models/topic_link_click.rb b/app/models/topic_link_click.rb index ff0d0daa2fb..8770567997d 100644 --- a/app/models/topic_link_click.rb +++ b/app/models/topic_link_click.rb @@ -70,17 +70,17 @@ class TopicLinkClick < ActiveRecord::Base end end - link = TopicLink.select([:id, :user_id]) - # test for all possible URLs - link = link.where(Array.new(urls.count, "url = ?").join(" OR "), *urls) + link = TopicLink.where(url: urls) # Find the forum topic link link = link.where(post_id: args[:post_id]) if args[:post_id].present? # If we don't have a post, just find the first occurrence of the link link = link.where(topic_id: args[:topic_id]) if args[:topic_id].present? - link = link.first + + # select the TopicLink associated to first url + link = link.order("array_position(ARRAY[#{urls.map { |s| "#{ActiveRecord::Base.connection.quote(s)}" }.join(',')}], url::text)").first # If no link is found... unless link.present? diff --git a/spec/models/topic_link_click_spec.rb b/spec/models/topic_link_click_spec.rb index 543d92b6188..b2f32439b14 100644 --- a/spec/models/topic_link_click_spec.rb +++ b/spec/models/topic_link_click_spec.rb @@ -238,6 +238,27 @@ describe TopicLinkClick do end end + context 'same base URL with different query' do + it 'are handled differently' do + post = Fabricate(:post, raw: <<~RAW) + no query param: http://example.com/a + with query param: http://example.com/a?b=c + with two query params: http://example.com/a?b=c&d=e + RAW + + TopicLink.extract_from(post) + + TopicLinkClick.create_from(url: 'http://example.com/a', post_id: post.id, ip: '127.0.0.1', user: Fabricate(:user)) + TopicLinkClick.create_from(url: 'http://example.com/a?b=c', post_id: post.id, ip: '127.0.0.2', user: Fabricate(:user)) + TopicLinkClick.create_from(url: 'http://example.com/a?b=c&d=e', post_id: post.id, ip: '127.0.0.3', user: Fabricate(:user)) + TopicLinkClick.create_from(url: 'http://example.com/a?b=c', post_id: post.id, ip: '127.0.0.4', user: Fabricate(:user)) + + expect(TopicLink.where("url LIKE '%example.com%'").pluck(:url, :clicks)).to contain_exactly( + ['http://example.com/a', 1], ['http://example.com/a?b=c', 2], ['http://example.com/a?b=c&d=e', 1] + ) + end + end + context 'with a google analytics tracking code and a hash' do before do @url = TopicLinkClick.create_from(url: 'http://discourse.org?_ga=1.16846778.221554446.1071987018#faq',