From 21b81e360501b15642ae31ef7149319726c6a61d Mon Sep 17 00:00:00 2001 From: Jeremy Friesen Date: Mon, 17 Jan 2022 16:57:03 -0500 Subject: [PATCH] Restoring tracking of custom impressions without GA (#16142) * Restoring tracking of custom impressions without GA In #15967 I (@jeremyf) introduced the "don't do tracking if Google Analytics ID is blank". However, we still likely want to trackCustomImpressions(). Further, I did some sleuthing and adding a couple of comments on whether we should record page views for articles in draft status or any views by the author. * Ensuring we don't track views of author or unpublished Prior to this commit, I was surprised to learn that we: 1) Tracked an author's view of their article. 2) Tracked views of an unpublished article. This came up from a Forem creator asking if they could reset the view counter. Or trigger the reset on publication. I think a general business logic policy of don't track views for the author and don't track views for unpublished articles is a reasonable default. Were we to pursue the clear views on publication, we'd need to consider something that went from unpublished -> published -> unpublished -> published. Without a more explicit state machine, triggering a busineiss logic behavior seems a bit unexpected. In other words, I wrote an article. There are 20 views when I realize that I need to unpublished it. I make the changes in the unpublished state, and re-publish. I'd assume that those 20 views would still be "recorded" and counted towards my article's view counts. * Revert "Ensuring we don't track views of author or unpublished" This reverts commit 321ecbed0ac4552e175d39bf17655a2fc93b265c. --- .../javascripts/initializers/initializeBaseTracking.js.erb | 2 +- app/workers/articles/update_page_views_worker.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/initializers/initializeBaseTracking.js.erb b/app/assets/javascripts/initializers/initializeBaseTracking.js.erb index 4de0846ae..75547b958 100644 --- a/app/assets/javascripts/initializers/initializeBaseTracking.js.erb +++ b/app/assets/javascripts/initializers/initializeBaseTracking.js.erb @@ -24,8 +24,8 @@ function initializeBaseTracking() { } }, 25); eventListening(); - trackCustomImpressions(); } + trackCustomImpressions(); } function fallbackActivityRecording() { diff --git a/app/workers/articles/update_page_views_worker.rb b/app/workers/articles/update_page_views_worker.rb index 574983d30..6f5d0f5ac 100644 --- a/app/workers/articles/update_page_views_worker.rb +++ b/app/workers/articles/update_page_views_worker.rb @@ -7,6 +7,8 @@ module Articles on_conflict: :replace, retry: false + # @todo Should we ever record a PageView for an article in draft status? (e.g., a preview) + # @todo Should we ever record a PageView for the author of the article? def perform(create_params) article = Article.find_by(id: create_params["article_id"]) return unless article