forum: optimize unread forum topics on forum index.

Avoid doing one SQL query per topic when checking for new topics on the
forum index.

This also changes it so that forum topics aren't always marked as new
for anonymous users.

app/controllers/forum_topics_controller.rb

@@ -24,6 +24,8 @@ class ForumTopicsController < ApplicationController
     params[:limit] ||= 40
 
     @forum_topics = ForumTopic.paginated_search(params)
+    @read_forum_topics = @forum_topics.read_by_user(CurrentUser.user)
+
     @forum_topics = @forum_topics.includes(:creator, :updater).load if request.format.html?
     @forum_topics = @forum_topics.includes(:creator, :original_post).load if request.format.atom?
 

app/helpers/forum_topics_helper.rb

@@ -6,4 +6,8 @@ module ForumTopicsHelper
   def available_min_user_levels
     ForumTopic::MIN_LEVELS.select { |name, level| level <= CurrentUser.level }.to_a
   end
+
+  def new_forum_topic?(topic, read_forum_topics)
+    !read_forum_topics.map(&:id).include?(topic.id)
+  end
 end

app/models/forum_topic.rb

@@ -14,8 +14,10 @@ class ForumTopic < ApplicationRecord
   belongs_to :creator, class_name: "User"
   belongs_to_updater
   has_many :posts, -> {order("forum_posts.id asc")}, :class_name => "ForumPost", :foreign_key => "topic_id", :dependent => :destroy
+  has_many :forum_topic_visits
   has_many :moderation_reports, through: :posts
   has_one :original_post, -> {order("forum_posts.id asc")}, class_name: "ForumPost", foreign_key: "topic_id", inverse_of: :topic
+
   before_validation :initialize_is_deleted, :on => :create
   validates_presence_of :title
   validates_associated :original_post
@@ -55,6 +57,11 @@ class ForumTopic < ApplicationRecord
       where("min_level <= ?", CurrentUser.level)
     end
 
+    def read_by_user(user)
+      return none if user.last_forum_read_at.nil? || user.last_forum_read_at < '2000-01-01'
+      merge(user.visited_forum_topics.where("forum_topic_visits.last_read_at >= forum_topics.updated_at OR ? >= forum_topics.updated_at", user.last_forum_read_at))
+    end
+
     def sticky_first
       order(is_sticky: :desc, updated_at: :desc)
     end
@@ -89,16 +96,6 @@ class ForumTopic < ApplicationRecord
   end
 
   module VisitMethods
-    def read_by?(user = nil)
-      user ||= CurrentUser.user
-
-      if user.last_forum_read_at && updated_at <= user.last_forum_read_at
-        return true
-      end
-
-      ForumTopicVisit.where("user_id = ? and forum_topic_id = ? and last_read_at >= ?", user.id, id, updated_at).exists?
-    end
-
     def mark_as_read!(user = CurrentUser.user)
       return if user.is_anonymous?
 

app/models/user.rb

@@ -94,6 +94,8 @@ class User < ApplicationRecord
   has_many :wiki_page_versions, foreign_key: :updater_id
   has_many :feedback, :class_name => "UserFeedback", :dependent => :destroy
   has_many :forum_post_votes, dependent: :destroy, foreign_key: :creator_id
+  has_many :forum_topic_visits, dependent: :destroy
+  has_many :visited_forum_topics, through: :forum_topic_visits, source: :forum_topic
   has_many :moderation_reports, as: :model
   has_many :pools, foreign_key: :creator_id
   has_many :posts, :foreign_key => "uploader_id"

app/views/admin/dashboards/show.html.erb

@@ -19,7 +19,7 @@
 
     <div class="section">
       <h2>Forum Topics</h2>
-      <%= render "forum_topics/listing", :forum_topics => @dashboard.forum_topics %>
+      <%= render "forum_topics/listing", forum_topics: @dashboard.forum_topics, read_forum_topics: @dashboard.forum_topics.read_by_user(CurrentUser.user) %>
     </div>
   </div>
 </div>

app/views/forum_topics/_listing.html.erb

@@ -4,7 +4,7 @@
       <span class="sticky">Sticky:</span>
     <% end %>
 
-    <% unless topic.read_by?(CurrentUser.user) %>
+    <% if CurrentUser.is_member? && new_forum_topic?(topic, read_forum_topics) %>
       <span class="new">NEW</span>
     <% end %>
 

app/views/forum_topics/index.html.erb

@@ -11,7 +11,7 @@
       <%= ForumTopic::CATEGORIES.map {|id, name| link_to_unless_current(name, forum_topics_path(:search => {:category_id => id}))}.join(", ").html_safe %>
     </p>
 
-    <%= render "listing", :forum_topics => @forum_topics %>
+    <%= render "listing", forum_topics: @forum_topics, read_forum_topics: @read_forum_topics %>
 
     <%= numbered_paginator(@forum_topics) %>
   </div>

test/unit/forum_topic_test.rb

@@ -15,7 +15,7 @@ class ForumTopicTest < ActiveSupport::TestCase
       CurrentUser.ip_addr = nil
     end
 
-    context "#read_by?" do
+    context "read_by_user" do
       context "with a populated @user.last_forum_read_at" do
         setup do
           @user.update_attribute(:last_forum_read_at, Time.now)
@@ -26,8 +26,8 @@ class ForumTopicTest < ActiveSupport::TestCase
             @topic.update_column(:updated_at, 1.day.from_now)
           end
 
-          should "return false" do
-            assert_equal(false, @topic.read_by?(@user))
+          should "return nothing" do
+            assert_equal([], ForumTopic.read_by_user(@user).map(&:id))
           end
         end
 
@@ -41,8 +41,8 @@ class ForumTopicTest < ActiveSupport::TestCase
               FactoryBot.create(:forum_topic_visit, user: @user, forum_topic: @topic, last_read_at: 16.hours.from_now)
             end
 
-            should "return false" do
-              assert_equal(false, @topic.read_by?(@user))
+            should "return nothing" do
+              assert_equal([], ForumTopic.read_by_user(@user).map(&:id))
             end
           end
 
@@ -51,8 +51,8 @@ class ForumTopicTest < ActiveSupport::TestCase
               FactoryBot.create(:forum_topic_visit, user: @user, forum_topic: @topic, last_read_at: 2.days.from_now)
             end
 
-            should "return true" do
-              assert_equal(true, @topic.read_by?(@user))
+            should "return the topic" do
+              assert_equal([@topic.id], ForumTopic.read_by_user(@user).map(&:id))
             end
           end
         end
@@ -60,8 +60,8 @@ class ForumTopicTest < ActiveSupport::TestCase
 
       context "with a blank @user.last_forum_read_at" do
         context "and no visits" do
-          should "return false" do
-            assert_equal(false, @topic.read_by?(@user))
+          should "return nothing" do
+            assert_equal([], ForumTopic.read_by_user(@user).map(&:id))
           end
         end
 
@@ -71,8 +71,8 @@ class ForumTopicTest < ActiveSupport::TestCase
               FactoryBot.create(:forum_topic_visit, user: @user, forum_topic: @topic, last_read_at: 1.day.ago)
             end
 
-            should "return false" do
-              assert_equal(false, @topic.read_by?(@user))
+            should "return nothing" do
+              assert_equal([], ForumTopic.read_by_user(@user).map(&:id))
             end
           end
 
@@ -81,8 +81,8 @@ class ForumTopicTest < ActiveSupport::TestCase
               FactoryBot.create(:forum_topic_visit, user: @user, forum_topic: @topic, last_read_at: 1.day.from_now)
             end
 
-            should "return true" do
-              assert_equal(true, @topic.read_by?(@user))
+            should "return nothing" do
+              assert_equal([], ForumTopic.read_by_user(@user).map(&:id))
             end
           end
         end