implementation for #1469

This reverts commit 18edc937fd.

app/controllers/forum_topics_controller.rb

@@ -2,7 +2,6 @@ class ForumTopicsController < ApplicationController
   respond_to :html, :xml, :json
   before_filter :member_only, :except => [:index, :show]
   before_filter :normalize_search, :only => :index
-  after_filter :update_last_forum_read_at, :only => [:show]
 
   def new
     @forum_topic = ForumTopic.new
@@ -17,8 +16,10 @@ class ForumTopicsController < ApplicationController
   end
 
   def index
+    # session[:read_forum_topics] = ""
     @query = ForumTopic.active.search(params[:search])
     @forum_topics = @query.order("is_sticky DESC, updated_at DESC").paginate(params[:page], :limit => params[:limit], :search_count => params[:search])
+    @read_forum_topic_ids = read_forum_topic_ids
     respond_with(@forum_topics) do |format|
       format.xml do
         render :xml => @forum_topics.to_xml(:root => "forum-topics")
@@ -31,6 +32,7 @@ class ForumTopicsController < ApplicationController
     @forum_posts = ForumPost.search(:topic_id => @forum_topic.id).order("forum_posts.id").paginate(params[:page])
     @forum_posts.all
     respond_with(@forum_topic)
+    session[:read_forum_topics] = @forum_topic.mark_as_read(read_forum_topic_ids)
   end
 
   def create
@@ -67,14 +69,6 @@ class ForumTopicsController < ApplicationController
   end
 
 private
-  def update_last_forum_read_at
-    return if CurrentUser.is_anonymous?
-
-    if CurrentUser.last_forum_read_at.nil? || CurrentUser.last_forum_read_at < @forum_topic.updated_at
-      CurrentUser.update_column(:last_forum_read_at, @forum_topic.updated_at)
-    end
-  end
-
   def normalize_search
     if params[:title_matches]
       params[:search] ||= {}
@@ -87,6 +81,14 @@ private
     end
   end
 
+  def read_forum_topics
+    session[:read_forum_topics].to_s
+  end
+
+  def read_forum_topic_ids
+    read_forum_topics.scan(/(\S+) (\S+)/)
+  end
+
   def check_privilege(forum_topic)
     if !forum_topic.editable_by?(CurrentUser.user)
       raise User::PrivilegeError

app/logical/session_creator.rb

@@ -13,7 +13,6 @@ class SessionCreator
   def authenticate
     if User.authenticate(name, password)
       user = User.find_by_name(name)
-      user.update_column(:last_logged_in_at, Time.now)
 
       if remember.present?
         cookies.permanent.signed[:user_name] = {

app/logical/session_loader.rb

@@ -13,6 +13,10 @@ class SessionLoader
       load_session_user
     elsif cookie_password_hash_valid?
       load_cookie_user
+
+      # this means a new session is being created, so assume
+      # all the old forum topics can be marked as read
+      update_forum_last_read_at
     else
       load_session_for_api
     end
@@ -72,6 +76,7 @@ private
   def load_cookie_user
     CurrentUser.user = User.find_by_name(cookies.signed[:user_name])
     CurrentUser.ip_addr = request.remote_ip
+    session[:user_id] = CurrentUser.user.id
   end
 
   def ban_expired?
@@ -99,5 +104,12 @@ private
   def set_time_zone
     Time.zone = CurrentUser.user.time_zone
   end
+
+  def update_forum_last_read_at
+    unless CurrentUser.user.is_anonymous?
+      CurrentUser.user.update_column(:last_forum_read_at, CurrentUser.user.last_logged_in_at)
+      CurrentUser.user.update_column(:last_logged_in_at, Time.now)
+    end
+  end
 end
 

app/models/forum_topic.rb

@@ -104,4 +104,30 @@ class ForumTopic < ActiveRecord::Base
   def hidden_attributes
     super + [:text_index]
   end
+
+  def read_by?(user, read_forum_topic_ids)
+    if read_forum_topic_ids.any? {|topic_id, timestamp| id.to_s == topic_id && updated_at.to_i > timestamp.to_i}
+      return false
+    end
+    if read_forum_topic_ids.any? {|topic_id, timestamp| id.to_s == topic_id && updated_at.to_i <= timestamp.to_i}
+      return true
+    end
+    return false if user.last_forum_read_at.nil?
+    return true if updated_at < user.last_forum_read_at
+    return false
+  end
+
+  def mark_as_read(read_forum_topic_ids)
+    hash = read_forum_topic_ids.inject({}) do |hash, x|
+      hash[x[0].to_s] = x[1].to_s
+      hash
+    end
+    hash[id.to_s] = updated_at.to_i.to_s
+    result = hash.to_a.flatten.join(" ")
+    if result.size > 3000
+      ids = result.scan(/\S+/)
+      result = ids[(ids.size / 2)..-1].join(" ")
+    end
+    result
+  end
 end

app/views/forum_topics/index.html.erb

@@ -24,7 +24,7 @@
                 <span class="sticky">Sticky:</span>
               <% end %>
 
-              <% if topic.updated_at > (CurrentUser.last_forum_read_at || Time.now) %>
+              <% unless topic.read_by?(CurrentUser.user, @read_forum_topic_ids) %>
                 <span class="new">NEW</span>
               <% end %>
 

test/unit/forum_topic_test.rb

@@ -14,6 +14,40 @@ class ForumTopicTest < ActiveSupport::TestCase
       CurrentUser.ip_addr = nil
     end
 
+    context "#read_by?" do
+      context "for a topic that was never read by the user" do
+        should "return false" do
+          assert_equal(false, @topic.read_by?(@user, [[(@topic.id + 1).to_s, "1"]]))
+        end
+      end
+
+      context "for a topic that was read by the user but has been updated since then" do
+        should "return false" do
+          assert_equal(false, @topic.read_by?(@user, [["#{@topic.id}", "#{1.day.ago.to_i}"]]))
+        end
+      end
+
+      context "for a topic that was read by the user and has not been updated since" do
+        should "return true" do
+          assert_equal(true, @topic.read_by?(@user, [["#{@topic.id}", "#{1.day.from_now.to_i}"]]))
+        end
+      end
+    end
+
+    context "#mark_as_read" do
+      should "include the topic id and updated_at timestamp" do
+        plus_one = @topic.id + 1
+        result = @topic.mark_as_read([["#{plus_one}", "2"]])
+        assert_equal("#{plus_one} 2 #{@topic.id} #{@topic.updated_at.to_i}", result)
+      end
+
+      should "prune the string if it gets too long" do
+        array = (1..1_000).to_a.map(&:to_s).in_groups_of(2)
+        result = @topic.mark_as_read(array)
+        assert_equal("502 503", result[0..6])
+      end
+    end
+
     context "constructed with nested attributes for its original post" do
       should "create a matching forum post" do
         assert_difference(["ForumTopic.count", "ForumPost.count"], 1) do