diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb index 9aef4ea80..89536b629 100644 --- a/app/controllers/forum_topics_controller.rb +++ b/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? diff --git a/app/helpers/forum_topics_helper.rb b/app/helpers/forum_topics_helper.rb index 98b9ad8ed..f96eac9c9 100644 --- a/app/helpers/forum_topics_helper.rb +++ b/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 diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index 8d35b984c..b34f28c2e 100644 --- a/app/models/forum_topic.rb +++ b/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? diff --git a/app/models/user.rb b/app/models/user.rb index 7510ff332..f4db96924 100644 --- a/app/models/user.rb +++ b/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" diff --git a/app/views/admin/dashboards/show.html.erb b/app/views/admin/dashboards/show.html.erb index e45999905..9adf1441c 100644 --- a/app/views/admin/dashboards/show.html.erb +++ b/app/views/admin/dashboards/show.html.erb @@ -19,7 +19,7 @@

Forum Topics

- <%= 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) %>
diff --git a/app/views/forum_topics/_listing.html.erb b/app/views/forum_topics/_listing.html.erb index 2a6075ee3..ce25f1b4c 100644 --- a/app/views/forum_topics/_listing.html.erb +++ b/app/views/forum_topics/_listing.html.erb @@ -4,7 +4,7 @@ Sticky: <% end %> - <% unless topic.read_by?(CurrentUser.user) %> + <% if CurrentUser.is_member? && new_forum_topic?(topic, read_forum_topics) %> NEW <% end %> diff --git a/app/views/forum_topics/index.html.erb b/app/views/forum_topics/index.html.erb index f59fd4af6..a457b13e0 100644 --- a/app/views/forum_topics/index.html.erb +++ b/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 %>

- <%= render "listing", :forum_topics => @forum_topics %> + <%= render "listing", forum_topics: @forum_topics, read_forum_topics: @read_forum_topics %> <%= numbered_paginator(@forum_topics) %> diff --git a/test/unit/forum_topic_test.rb b/test/unit/forum_topic_test.rb index 1dc61b35c..f4371659f 100644 --- a/test/unit/forum_topic_test.rb +++ b/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