FEATURE: topic_view_stats table with daily fidelity (#27197)

This gives us daily fidelity of topic view stats

New table stores a row per topic viewed per day tracking
anonymous and logged on views

We also have a new endpoint `/t/ID/views-stats.json` to get the statistics for the topic.

app/controllers/topic_view_stats_controller.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class TopicViewStatsController < ApplicationController
+  MAX_STATS_PER_API_REQUEST = 300
+
+  def index
+    topic = Topic.find(params[:topic_id].to_i)
+    guardian.ensure_can_see!(topic)
+
+    from = 30.days.ago.to_date
+    to = Date.today
+
+    begin
+      from = params[:from].to_date if params[:from].present?
+      to = params[:to].to_date if params[:to].present?
+    rescue Date::Error
+      render_json_error(I18n.t("topic_view_stats.invalid_date"), status: 422)
+      return
+    end
+
+    stats =
+      TopicViewStat
+        .where(topic_id: topic.id, viewed_at: from..to)
+        .order(viewed_at: :desc)
+        .limit(MAX_STATS_PER_API_REQUEST)
+
+    rows = []
+
+    stats.each do |stat|
+      rows << { viewed_at: stat.viewed_at, views: stat.anonymous_views + stat.logged_in_views }
+    end
+
+    render json: { topic_id: topic.id, stats: rows.reverse }
+  end
+end

app/models/topic.rb

@@ -262,6 +262,7 @@ class Topic < ActiveRecord::Base
 
   has_many :group_archived_messages, dependent: :destroy
   has_many :user_archived_messages, dependent: :destroy
+  has_many :topic_view_stats, dependent: :destroy
 
   has_many :allowed_groups, through: :topic_allowed_groups, source: :group
   has_many :allowed_group_users, through: :allowed_groups, source: :users
@@ -862,7 +863,7 @@ class Topic < ActiveRecord::Base
         FROM posts
         WHERE deleted_at IS NULL AND post_type <> 4
         GROUP BY topic_id
-      )       
+      )
       UPDATE topics
       SET
         highest_staff_post_number = X.highest_post_number,

app/models/topic_view_item.rb

@@ -24,13 +24,14 @@ class TopicViewItem < ActiveRecord::Base
 
       TopicViewItem.transaction do
         # this is called real frequently, working hard to avoid exceptions
-        sql =
+        sql = <<~SQL
-          "INSERT INTO topic_views (topic_id, ip_address, viewed_at, user_id)
+          INSERT INTO topic_views (topic_id, ip_address, viewed_at, user_id)
-               SELECT :topic_id, :ip_address, :viewed_at, :user_id
+          SELECT :topic_id, :ip_address, :viewed_at, :user_id
-               WHERE NOT EXISTS (
+          WHERE NOT EXISTS (
-                 SELECT 1 FROM topic_views
+            SELECT 1 FROM topic_views
-                 /*where*/
+            /*where*/
-               )"
+          )
+        SQL
 
         builder = DB.build(sql)
 
@@ -43,15 +44,20 @@ class TopicViewItem < ActiveRecord::Base
 
         result = builder.exec(topic_id: topic_id, ip_address: ip, viewed_at: at, user_id: user_id)
 
-        Topic.where(id: topic_id).update_all "views = views + 1"
-
         if result > 0
           if user_id
             UserStat.where(user_id: user_id).update_all "topics_entered = topics_entered + 1"
           end
         end
 
-        # Update the views count in the parent, if it exists.
+        Topic.where(id: topic_id).update_all "views = views + 1"
+
+        TopicViewStat.add(
+          topic_id: topic_id,
+          date: at,
+          anonymous_views: user_id ? 0 : 1,
+          logged_in_views: user_id ? 1 : 0,
+        )
       end
     end
   end

app/models/topic_view_stat.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+class TopicViewStat < ActiveRecord::Base
+  belongs_to :topic
+
+  def self.add(topic_id:, date:, anonymous_views:, logged_in_views:)
+    sql = <<~SQL
+          INSERT INTO topic_view_stats (topic_id, viewed_at, anonymous_views, logged_in_views)
+          VALUES (:topic_id, :viewed_at, :anon_views, :logged_in_views)
+          ON CONFLICT (topic_id, viewed_at)
+          DO UPDATE SET
+            anonymous_views = topic_view_stats.anonymous_views + :anon_views,
+            logged_in_views = topic_view_stats.logged_in_views + :logged_in_views
+        SQL
+
+    DB.exec(
+      sql,
+      topic_id: topic_id,
+      viewed_at: date,
+      anon_views: anonymous_views,
+      logged_in_views: logged_in_views,
+    )
+  end
+end
+
+# == Schema Information
+#
+# Table name: topic_view_stats
+#
+#  id              :bigint           not null, primary key
+#  topic_id        :integer          not null
+#  viewed_at       :date             not null
+#  anonymous_views :integer          default(0), not null
+#  logged_in_views :integer          default(0), not null
+#
+# Indexes
+#
+#  index_topic_view_stats_on_topic_id_and_viewed_at  (topic_id,viewed_at) UNIQUE
+#  index_topic_view_stats_on_viewed_at_and_topic_id  (viewed_at,topic_id)
+#

config/locales/server.en.yml

@@ -836,6 +836,9 @@ en:
 
   admin_quick_start_title: "Admin Guide: Getting Started"
 
+  topic_view_stats:
+    invalid_date: "Date is in an invalid format, use YYYY-MM-DD"
+
   category:
     topic_prefix: "About the %{category} category"
     replace_paragraph: "(Replace this first paragraph with a brief description of your new category. This guidance will appear in the category selection area, so try to keep it below 200 characters.)"

config/routes.rb

@@ -1583,6 +1583,9 @@ Discourse::Application.routes.draw do
            constraints: HomePageConstraint.new("#{filter}"),
            as: "list_#{filter}"
     end
+
+    get "/t/:topic_id/view-stats.json" => "topic_view_stats#index"
+
     # special case for categories
     root to: "categories#index",
          constraints: HomePageConstraint.new("categories"),

db/migrate/20240527015009_add_topic_view_stats.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class AddTopicViewStats < ActiveRecord::Migration[7.0]
+  def change
+    create_table :topic_view_stats do |t|
+      t.integer :topic_id, null: false
+      t.date :viewed_at, null: false
+      t.integer :anonymous_views, default: 0, null: false
+      t.integer :logged_in_views, default: 0, null: false
+    end
+
+    add_index :topic_view_stats, %i[topic_id viewed_at], unique: true
+    add_index :topic_view_stats, %i[viewed_at topic_id]
+  end
+end

spec/models/topic_view_item_spec.rb

@@ -6,6 +6,33 @@ RSpec.describe TopicViewItem do
     TopicViewItem.add(topic_id, ip, user_id, nil, skip_redis)
   end
 
+  it "correctly increase topic view stats" do
+    topic = Fabricate(:topic)
+
+    freeze_time "2021-01-01 12:00"
+
+    add(topic.id, "1.1.1.1", nil)
+    add(topic.id, "1.1.1.1", nil)
+
+    stat = TopicViewStat.find_by(topic_id: topic.id, viewed_at: Date.today)
+    expect(stat.anonymous_views).to eq(2)
+    expect(stat.logged_in_views).to eq(0)
+
+    add(topic.id, "1.1.1.1", topic.user.id)
+    stat.reload
+
+    expect(stat.anonymous_views).to eq(2)
+    expect(stat.logged_in_views).to eq(1)
+
+    freeze_time(1.day.from_now)
+
+    add(topic.id, "1.1.1.1", nil)
+    stat = TopicViewStat.find_by(topic_id: topic.id, viewed_at: Date.today)
+
+    expect(stat.anonymous_views).to eq(1)
+    expect(stat.logged_in_views).to eq(0)
+  end
+
   it "raises nothing for dupes" do
     add(2, "1.1.1.1")
     add(2, "1.1.1.1", 1)

spec/requests/topic_view_stats_controller_spec.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+describe TopicViewStatsController do
+  fab!(:topic)
+
+  it "will error if accessed on require login sites" do
+    SiteSetting.login_required = true
+    get "/t/#{topic.id}/view-stats.json"
+    expect(response.status).to eq(403)
+  end
+
+  it "will not allow access to private topics" do
+    topic.category.update!(read_restricted: true)
+
+    get "/t/#{topic.id}/view-stats.json"
+    expect(response.status).to eq(403)
+  end
+
+  it "will raise correct errors if any param is invalid" do
+    get "/t/999999999999999999999999999999990000009/view-stats.json"
+    expect(response.status).to eq(404)
+  end
+
+  it "will return an error if from and to are not valid dates" do
+    get "/t/#{topic.id}/view-stats.json?from=abc&to=xxx"
+
+    expect(response.status).to eq(422)
+  end
+
+  it "will return view stats for public topics" do
+    freeze_time "2021-01-01 12:00"
+
+    TopicViewStat.create!(
+      topic_id: topic.id,
+      viewed_at: Date.yesterday,
+      anonymous_views: 2,
+      logged_in_views: 3,
+    )
+
+    TopicViewStat.create!(
+      topic_id: topic.id,
+      viewed_at: Date.today,
+      anonymous_views: 1,
+      logged_in_views: 2,
+    )
+
+    get "/t/#{topic.id}/view-stats.json"
+    expect(response.status).to eq(200)
+
+    expected = {
+      "topic_id" => topic.id,
+      "stats" => [
+        { "viewed_at" => "2020-12-31", "views" => 5 },
+        { "viewed_at" => "2021-01-01", "views" => 3 },
+      ],
+    }
+
+    expect(response.parsed_body).to eq(expected)
+
+    get "/t/#{topic.id}/view-stats.json?from=2019-12-31&to=2019-12-31"
+    expect(response.parsed_body).to eq({ "topic_id" => topic.id, "stats" => [] })
+
+    get "/t/#{topic.id}/view-stats.json?from=2000-12-31&to=2020-12-31"
+    expected = {
+      "topic_id" => topic.id,
+      "stats" => [{ "viewed_at" => "2020-12-31", "views" => 5 }],
+    }
+    expect(response.parsed_body).to eq(expected)
+
+    get "/t/#{topic.id}/view-stats.json?from=2020-12-31&to=2020-12-31"
+    expect(response.parsed_body).to eq(expected)
+  end
+end