From 7cc5da08fe23be0d18068547e24bfc8c2b2610ac Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Fri, 24 Jan 2014 15:19:20 -0500
Subject: [PATCH] Track how many posts a user reads each day in user_visits

---
 app/models/leader_requirements.rb             |  3 +--
 app/models/topic_user.rb                      |  4 ++++
 app/models/user.rb                            | 20 +++++++++++++----
 app/models/user_visit.rb                      |  2 +-
 ...124202427_add_posts_read_to_user_visits.rb | 12 ++++++++++
 spec/models/user_spec.rb                      | 22 +++++++++++++++++++
 6 files changed, 56 insertions(+), 7 deletions(-)
 create mode 100644 db/migrate/20140124202427_add_posts_read_to_user_visits.rb

diff --git a/app/models/leader_requirements.rb b/app/models/leader_requirements.rb
index 349ad3685a1..d8a2c504d20 100644
--- a/app/models/leader_requirements.rb
+++ b/app/models/leader_requirements.rb
@@ -24,8 +24,7 @@ class LeaderRequirements
   end
 
   def days_visited
-    # This is naive. The user may have visited the site, but not read any posts.
-    @user.user_visits.where("visited_at > ?", time_period.days.ago).count
+    @user.user_visits.where("visited_at > ? and posts_read > 0", time_period.days.ago).count
   end
 
   def min_days_visited
diff --git a/app/models/topic_user.rb b/app/models/topic_user.rb
index e069ccf66ec..b69216b800a 100644
--- a/app/models/topic_user.rb
+++ b/app/models/topic_user.rb
@@ -199,7 +199,9 @@ SQL
         before_last_read = rows[0][2].to_i
 
         if before_last_read < post_number
+          # The user read at least one new post
           TopicTrackingState.publish_read(topic_id, post_number, user.id)
+          user.update_posts_read!(post_number - before_last_read)
         end
 
         if before != after
@@ -208,7 +210,9 @@ SQL
       end
 
       if rows.length == 0
+        # The user read at least one post in a topic that they haven't viewed before.
         TopicTrackingState.publish_read(topic_id, post_number, user.id)
+        user.update_posts_read!(post_number)
 
         args[:tracking] = notification_levels[:tracking]
         args[:regular] = notification_levels[:regular]
diff --git a/app/models/user.rb b/app/models/user.rb
index d58f3925d42..fad8ffd587d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -281,14 +281,21 @@ class User < ActiveRecord::Base
     last_seen_at.present?
   end
 
-  def has_visit_record?(date)
+  def visit_record_for(date)
     user_visits.where(visited_at: date).first
   end
 
   def update_visit_record!(date)
-    unless has_visit_record?(date)
-      user_stat.update_column(:days_visited, user_stat.days_visited + 1)
-      user_visits.create!(visited_at: date)
+    create_visit_record!(date) unless visit_record_for(date)
+  end
+
+  def update_posts_read!(num_posts, now=Time.zone.now)
+    if user_visit = visit_record_for(now.to_date)
+      user_visit.posts_read += num_posts
+      user_visit.save
+      user_visit
+    else
+      create_visit_record!(now.to_date, num_posts)
     end
   end
 
@@ -556,6 +563,11 @@ class User < ActiveRecord::Base
     email_tokens.create(email: email)
   end
 
+  def create_visit_record!(date, posts_read=0)
+    user_stat.update_column(:days_visited, user_stat.days_visited + 1)
+    user_visits.create!(visited_at: date, posts_read: posts_read)
+  end
+
   def ensure_password_is_hashed
     if @raw_password
       self.salt = SecureRandom.hex(16)
diff --git a/app/models/user_visit.rb b/app/models/user_visit.rb
index 737fa9f058a..4683bd7be74 100644
--- a/app/models/user_visit.rb
+++ b/app/models/user_visit.rb
@@ -1,6 +1,6 @@
 class UserVisit < ActiveRecord::Base
 
-  # A list of visits in the last month by day
+  # A count of visits in the last month by day
   def self.by_day(sinceDaysAgo=30)
     where("visited_at >= ?", sinceDaysAgo.days.ago).group(:visited_at).order(:visited_at).count
   end
diff --git a/db/migrate/20140124202427_add_posts_read_to_user_visits.rb b/db/migrate/20140124202427_add_posts_read_to_user_visits.rb
new file mode 100644
index 00000000000..5315f07ba7b
--- /dev/null
+++ b/db/migrate/20140124202427_add_posts_read_to_user_visits.rb
@@ -0,0 +1,12 @@
+class AddPostsReadToUserVisits < ActiveRecord::Migration
+  def up
+    add_column :user_visits, :posts_read, :integer, default: 0
+
+    # Can't accurately back-fill this column. Assume everyone read at least one post per visit.
+    execute "UPDATE user_visits SET posts_read = 1"
+  end
+
+  def down
+    remove_column :user_visits, :posts_read
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 0d2f4274cd9..7d09fdbc0bb 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1028,4 +1028,26 @@ describe User do
 
   end
 
+  describe "update_posts_read!" do
+    context "with a UserVisit record" do
+      let!(:user) { Fabricate(:user) }
+      let!(:now)  { Time.zone.now }
+      before { user.update_last_seen!(now) }
+
+      it "with existing UserVisit record, increments the posts_read value" do
+        expect {
+          user_visit = user.update_posts_read!(2)
+          user_visit.posts_read.should == 2
+        }.to_not change { UserVisit.count }
+      end
+
+      it "with no existing UserVisit record, creates a new UserVisit record and increments the posts_read count" do
+        expect {
+          user_visit = user.update_posts_read!(3, 5.days.ago)
+          user_visit.posts_read.should == 3
+        }.to change { UserVisit.count }.by(1)
+      end
+    end
+  end
+
 end