From e33b15e1e5ed203be1910704ea0bad1c304c1fdd Mon Sep 17 00:00:00 2001 From: Daniel Uber Date: Fri, 27 Aug 2021 08:03:43 -0500 Subject: [PATCH] Add counts for number of views when showing referrers (#14600) * Add counts for number of views when showing referrers We're sampling unauthenticated page views (every 10th visit is recorded) and using the "counts_for_number_of_views" attribute to mark the 10x weight of these sampled events. The referrers section of the statistics page however was counting the number of page view rows (without regard to the weight) and it looked like external referrers (like a google search) were counting 10 times on the statistics page. Update the analytics service to show the sum of the weights, rather than the count of events, on the "Traffic Source Summary" of the article stats page. This is loaded from /api/analytics/referrers?article_id=:id on the front end. Needs a test that demonstrates this (currently all specs for this stayed green suggesting the coverage didn't include anything that represents this situation). * Avoid calling sum twice by reusing the value alias the .sum(:column) method in this chain creates an alias 'select sum(column) as sum_column' which can be used here to avoid recalculating the sum once for sorting and again for return value. Sort by the return value's alias. * Update the test to include weighted page views Since we want to demonstrate that the count shown for visits matches the sum of the weighted page views, add weighting to the top url visits and expect the sum of the weights to be presented. --- app/services/analytics_service.rb | 6 +++++- spec/services/analytics_service_spec.rb | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/services/analytics_service.rb b/app/services/analytics_service.rb index da70c268d..2860eef44 100644 --- a/app/services/analytics_service.rb +++ b/app/services/analytics_service.rb @@ -54,7 +54,11 @@ class AnalyticsService # Returns the list of referrers def referrers(top: 20) # count_all is the name of the field autogenerated by Rails with COUNT(*) - counts = page_view_data.group(:domain).order(count_all: :desc).limit(top).count + counts = page_view_data + .group(:domain) + .order(Arel.sql("sum_counts_for_number_of_views DESC")) + .limit(top) + .sum("counts_for_number_of_views") # we transform this in a list of hashes in case we need to add more keys domains = counts.map { |domain, count| { domain: domain, count: count } } diff --git a/spec/services/analytics_service_spec.rb b/spec/services/analytics_service_spec.rb index aef1c3df1..cb59e5135 100644 --- a/spec/services/analytics_service_spec.rb +++ b/spec/services/analytics_service_spec.rb @@ -410,14 +410,14 @@ RSpec.describe AnalyticsService, type: :service do expect(analytics_service.referrers[:domains]).to be_empty end - it "returns the domains ordered by count" do + it "returns the domains ordered by number of views" do other_url = Faker::Internet.url create_list(:page_view, 2, user: user, article: article, referrer: other_url) top_url = Faker::Internet.url - create_list(:page_view, 3, user: user, article: article, referrer: top_url) + create_list(:page_view, 3, user: user, article: article, referrer: top_url, counts_for_number_of_views: 10) expected_result = [ - { domain: Addressable::URI.parse(top_url).domain, count: 3 }, + { domain: Addressable::URI.parse(top_url).domain, count: 30 }, { domain: Addressable::URI.parse(other_url).domain, count: 2 }, ] expect(analytics_service.referrers[:domains]).to eq(expected_result)