From 7e07b874a46923692b6671ddb222f32a20e2f1dd Mon Sep 17 00:00:00 2001 From: r888888888 Date: Wed, 12 Mar 2014 18:22:15 -0700 Subject: [PATCH] implementation for #1469 This reverts commit 18edc937fd7a96453a36131b088a4395d2a059e2. --- app/controllers/forum_topics_controller.rb | 20 +++++++------ app/logical/session_creator.rb | 1 - app/logical/session_loader.rb | 12 ++++++++ app/models/forum_topic.rb | 26 +++++++++++++++++ app/views/forum_topics/index.html.erb | 2 +- test/unit/forum_topic_test.rb | 34 ++++++++++++++++++++++ 6 files changed, 84 insertions(+), 11 deletions(-) diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb index 7d299601a..d865be07e 100644 --- a/app/controllers/forum_topics_controller.rb +++ b/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 diff --git a/app/logical/session_creator.rb b/app/logical/session_creator.rb index 4ce44e334..a0aff070c 100644 --- a/app/logical/session_creator.rb +++ b/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] = { diff --git a/app/logical/session_loader.rb b/app/logical/session_loader.rb index 737a99a5a..bbaeebdbf 100644 --- a/app/logical/session_loader.rb +++ b/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 diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index 5df02c2d3..d2dafbad3 100644 --- a/app/models/forum_topic.rb +++ b/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 diff --git a/app/views/forum_topics/index.html.erb b/app/views/forum_topics/index.html.erb index c14969da5..cacf5555d 100644 --- a/app/views/forum_topics/index.html.erb +++ b/app/views/forum_topics/index.html.erb @@ -24,7 +24,7 @@ Sticky: <% end %> - <% if topic.updated_at > (CurrentUser.last_forum_read_at || Time.now) %> + <% unless topic.read_by?(CurrentUser.user, @read_forum_topic_ids) %> NEW <% end %> diff --git a/test/unit/forum_topic_test.rb b/test/unit/forum_topic_test.rb index 4c1826bbe..b9cc9551c 100644 --- a/test/unit/forum_topic_test.rb +++ b/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