diff --git a/app/jobs/scheduled/ensure_db_consistency.rb b/app/jobs/scheduled/ensure_db_consistency.rb index 6c21814fa01..c687cbec348 100644 --- a/app/jobs/scheduled/ensure_db_consistency.rb +++ b/app/jobs/scheduled/ensure_db_consistency.rb @@ -35,6 +35,9 @@ module Jobs UserStat.ensure_consistency!(13.hours.ago) measure(UserStat) + GroupUser.ensure_consistency!(13.hours.ago) + measure(GroupUser) + Rails.logger.debug(format_measure) nil end diff --git a/app/models/group_user.rb b/app/models/group_user.rb index 921641d4c64..d83b0606b5a 100644 --- a/app/models/group_user.rb +++ b/app/models/group_user.rb @@ -19,6 +19,52 @@ class GroupUser < ActiveRecord::Base NotificationLevels.all end + def self.ensure_consistency!(last_seen = 1.hour.ago) + update_first_unread_pm(last_seen) + end + + def self.update_first_unread_pm(last_seen, limit: 10_000) + DB.exec(<<~SQL, archetype: Archetype.private_message, last_seen: last_seen, limit: limit) + UPDATE group_users gu + SET first_unread_pm_at = Y.min_date + FROM ( + SELECT + X.group_id, + X.user_id, + X.min_date + FROM ( + SELECT + gu2.group_id, + gu2.user_id, + MIN(t.updated_at) min_date + FROM group_users gu2 + INNER JOIN topic_allowed_groups tag ON tag.group_id = gu2.group_id + INNER JOIN topics t ON t.id = tag.topic_id + INNER JOIN users u ON u.id = gu2.user_id + LEFT JOIN topic_users tu ON t.id = tu.topic_id AND tu.user_id = gu2.user_id + WHERE t.deleted_at IS NULL + AND t.archetype = :archetype + AND tu.last_read_post_number < CASE + WHEN u.admin OR u.moderator + THEN t.highest_staff_post_number + ELSE t.highest_post_number + END + AND (COALESCE(tu.notification_level, 1) >= 2) + GROUP BY gu2.user_id, gu2.group_id + ) AS X + WHERE X.user_id IN ( + SELECT id + FROM users + WHERE last_seen_at IS NOT NULL + AND last_seen_at > :last_seen + ORDER BY last_seen_at DESC + LIMIT :limit + ) + ) Y + WHERE gu.user_id = Y.user_id AND gu.group_id = Y.group_id + SQL + end + protected def set_notification_level diff --git a/app/models/post_timing.rb b/app/models/post_timing.rb index 1bf1c3f4149..8251d429074 100644 --- a/app/models/post_timing.rb +++ b/app/models/post_timing.rb @@ -75,7 +75,9 @@ class PostTiming < ActiveRecord::Base topic.posts.find_by(post_number: post_number).decrement!(:reads) - if !topic.private_message? + if topic.private_message? + set_minimum_first_unread_pm!(topic: topic, user_id: user.id, date: topic.updated_at) + else set_minimum_first_unread!(user_id: user.id, date: topic.updated_at) end end @@ -101,6 +103,27 @@ class PostTiming < ActiveRecord::Base end end + def self.set_minimum_first_unread_pm!(topic:, user_id:, date:) + if topic.topic_allowed_users.exists?(user_id: user_id) + UserStat.where("first_unread_pm_at > ? AND user_id = ?", date, user_id) + .update_all(first_unread_pm_at: date) + else + DB.exec(<<~SQL, date: date, user_id: user_id, topic_id: topic.id) + UPDATE group_users gu + SET first_unread_pm_at = :date + FROM ( + SELECT + gu2.user_id, + gu2.group_id + FROM group_users gu2 + INNER JOIN topic_allowed_groups tag ON tag.group_id = gu2.group_id AND tag.topic_id = :topic_id + WHERE gu2.user_id = :user_id + ) Y + WHERE gu.user_id = Y.user_id AND gu.group_id = Y.group_id + SQL + end + end + def self.set_minimum_first_unread!(user_id:, date:) DB.exec(<<~SQL, date: date, user_id: user_id) UPDATE user_stats diff --git a/app/models/user_stat.rb b/app/models/user_stat.rb index 881b14ae69a..bc2a9d251ca 100644 --- a/app/models/user_stat.rb +++ b/app/models/user_stat.rb @@ -12,10 +12,59 @@ class UserStat < ActiveRecord::Base update_distinct_badge_count update_view_counts(last_seen) update_first_unread(last_seen) + update_first_unread_pm(last_seen) end - def self.update_first_unread(last_seen, limit: 10_000) - DB.exec(<<~SQL, min_date: last_seen, limit: limit, now: 10.minutes.ago) + UPDATE_UNREAD_MINUTES_AGO = 10 + UPDATE_UNREAD_USERS_LIMIT = 10_000 + + def self.update_first_unread_pm(last_seen, limit: UPDATE_UNREAD_USERS_LIMIT) + DB.exec(<<~SQL, archetype: Archetype.private_message, now: UPDATE_UNREAD_MINUTES_AGO.minutes.ago, last_seen: last_seen, limit: limit) + UPDATE user_stats us + SET first_unread_pm_at = COALESCE(Z.min_date, :now) + FROM ( + SELECT + Y.user_id, + Y.min_date + FROM ( + SELECT + u1.id user_id, + X.min_date + FROM users u1 + LEFT JOIN ( + SELECT + tau.user_id, + MIN(t.updated_at) min_date + FROM topic_allowed_users tau + INNER JOIN topics t ON t.id = tau.topic_id + INNER JOIN users u ON u.id = tau.user_id + LEFT JOIN topic_users tu ON t.id = tu.topic_id AND tu.user_id = tau.user_id + WHERE t.deleted_at IS NULL + AND t.archetype = :archetype + AND tu.last_read_post_number < CASE + WHEN u.admin OR u.moderator + THEN t.highest_staff_post_number + ELSE t.highest_post_number + END + AND (COALESCE(tu.notification_level, 1) >= 2) + GROUP BY tau.user_id + ) AS X ON X.user_id = u1.id + ) AS Y + WHERE Y.user_id IN ( + SELECT id + FROM users + WHERE last_seen_at IS NOT NULL + AND last_seen_at > :last_seen + ORDER BY last_seen_at DESC + LIMIT :limit + ) + ) AS Z + WHERE us.user_id = Z.user_id + SQL + end + + def self.update_first_unread(last_seen, limit: UPDATE_UNREAD_USERS_LIMIT) + DB.exec(<<~SQL, min_date: last_seen, limit: limit, now: UPDATE_UNREAD_MINUTES_AGO.minutes.ago) UPDATE user_stats us SET first_unread_at = COALESCE(Y.min_date, :now) FROM ( diff --git a/db/migrate/20200902054531_add_first_unread_pm_a_to_group_user.rb b/db/migrate/20200902054531_add_first_unread_pm_a_to_group_user.rb new file mode 100644 index 00000000000..8cfb624a113 --- /dev/null +++ b/db/migrate/20200902054531_add_first_unread_pm_a_to_group_user.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddFirstUnreadPmAToGroupUser < ActiveRecord::Migration[6.0] + def up + add_column :group_users, :first_unread_pm_at, :datetime, null: false, default: -> { 'CURRENT_TIMESTAMP' } + + execute <<~SQL + UPDATE group_users gu + SET first_unread_pm_at = gu.created_at + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20200902082203_add_first_unread_pm_at_to_user_stats.rb b/db/migrate/20200902082203_add_first_unread_pm_at_to_user_stats.rb new file mode 100644 index 00000000000..6f4c8e812ac --- /dev/null +++ b/db/migrate/20200902082203_add_first_unread_pm_at_to_user_stats.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddFirstUnreadPmAtToUserStats < ActiveRecord::Migration[6.0] + def up + add_column :user_stats, :first_unread_pm_at, :datetime, null: false, default: -> { 'CURRENT_TIMESTAMP' } + + execute <<~SQL + UPDATE user_stats us + SET first_unread_pm_at = u.created_at + FROM users u + WHERE u.id = us.user_id + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/post_migrate/20200903045539_add_index_topics_on_timestamps_private.rb b/db/post_migrate/20200903045539_add_index_topics_on_timestamps_private.rb new file mode 100644 index 00000000000..9479dce40a8 --- /dev/null +++ b/db/post_migrate/20200903045539_add_index_topics_on_timestamps_private.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddIndexTopicsOnTimestampsPrivate < ActiveRecord::Migration[6.0] + disable_ddl_transaction! + + def up + execute <<~SQL + CREATE INDEX CONCURRENTLY IF NOT EXISTS + index_topics_on_timestamps_private + ON topics (bumped_at, created_at, updated_at) + WHERE deleted_at IS NULL AND archetype = 'private_message' + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index 9d2ee6ac82c..7e0a7b8786b 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -960,9 +960,19 @@ class TopicQuery def unread_messages(params) query = TopicQuery.unread_filter( messages_for_groups_or_user(params[:my_group_ids]), - @user&.id, - staff: @user&.staff?) - .limit(params[:count]) + @user.id, + staff: @user.staff? + ) + + first_unread_pm_at = + if params[:my_group_ids].present? + GroupUser.where(user_id: @user.id, group_id: params[:my_group_ids]).minimum(:first_unread_pm_at) + else + UserStat.where(user_id: @user.id).pluck(:first_unread_pm_at).first + end + + query = query.where("topics.updated_at >= ?", first_unread_pm_at) if first_unread_pm_at + query = query.limit(params[:count]) if params[:count] query end diff --git a/spec/models/group_user_spec.rb b/spec/models/group_user_spec.rb index 6b3029bab54..fca2c2c7e6d 100644 --- a/spec/models/group_user_spec.rb +++ b/spec/models/group_user_spec.rb @@ -160,4 +160,64 @@ describe GroupUser do expect(TagUser.lookup(user, :watching_first_post).pluck(:tag_id)).to eq([tag4.id]) end end + + describe '#ensure_consistency!' do + fab!(:group) { Fabricate(:group) } + + fab!(:pm_post) { Fabricate(:private_message_post) } + + fab!(:pm_topic) do + pm_post.topic.tap { |t| t.allowed_groups << group } + end + + fab!(:user) do + Fabricate(:user, last_seen_at: Time.zone.now).tap do |u| + group.add(u) + + TopicUser.change(u.id, pm_topic.id, + notification_level: TopicUser.notification_levels[:tracking], + last_read_post_number: pm_post.post_number + ) + end + end + + # User that is not tracking topic + fab!(:user_2) do + Fabricate(:user, last_seen_at: Time.zone.now).tap do |u| + group.add(u) + + TopicUser.change(u.id, pm_topic.id, + notification_level: TopicUser.notification_levels[:regular], + last_read_post_number: pm_post.post_number + ) + end + end + + # User that has not been seen + fab!(:user_3) do + Fabricate(:user).tap do |u| + group.add(u) + + TopicUser.change(u.id, pm_topic.id, + notification_level: TopicUser.notification_levels[:tracking], + last_read_post_number: pm_post.post_number + ) + end + end + + it 'updates first unread pm timestamp correctly' do + freeze_time 10.minutes.from_now + + post = create_post( + user: pm_topic.user, + topic_id: pm_topic.id + ) + + GroupUser.ensure_consistency! + + expect(group.group_users.find_by(user_id: user.id).first_unread_pm_at).to eq_time(post.topic.updated_at) + expect(group.group_users.find_by(user_id: user_2.id).first_unread_pm_at).to_not eq_time(post.topic.updated_at) + expect(group.group_users.find_by(user_id: user_3.id).first_unread_pm_at).to_not eq_time(post.topic.updated_at) + end + end end diff --git a/spec/models/post_timing_spec.rb b/spec/models/post_timing_spec.rb index cb18d50bbd1..43e3f051b65 100644 --- a/spec/models/post_timing_spec.rb +++ b/spec/models/post_timing_spec.rb @@ -184,4 +184,41 @@ describe PostTiming do expect(post.reload.reads).to eq initial_read_count end end + + describe '.destroy_last_for' do + it 'updates first unread for a user correctly when topic is public' do + post = Fabricate(:post) + post.topic.update!(updated_at: 10.minutes.ago) + PostTiming.process_timings(post.user, post.topic_id, 1, [[post.post_number, 100]]) + + PostTiming.destroy_last_for(post.user, post.topic_id) + + expect(post.user.user_stat.reload.first_unread_at).to eq_time(post.topic.updated_at) + end + + it 'updates first unread for a user correctly when topic is a pm' do + post = Fabricate(:private_message_post) + post.topic.update!(updated_at: 10.minutes.ago) + PostTiming.process_timings(post.user, post.topic_id, 1, [[post.post_number, 100]]) + + PostTiming.destroy_last_for(post.user, post.topic_id) + + expect(post.user.user_stat.reload.first_unread_pm_at).to eq_time(post.topic.updated_at) + end + + it 'updates first unread for a user correctly when topic is a group pm' do + topic = Fabricate(:private_message_topic, updated_at: 10.minutes.ago) + post = Fabricate(:post, topic: topic) + user = Fabricate(:user) + group = Fabricate(:group) + group.add(user) + topic.allowed_groups << group + PostTiming.process_timings(user, topic.id, 1, [[post.post_number, 100]]) + + PostTiming.destroy_last_for(user, topic.id) + + expect(GroupUser.find_by(user: user, group: group).first_unread_pm_at) + .to eq_time(post.topic.updated_at) + end + end end diff --git a/spec/models/user_stat_spec.rb b/spec/models/user_stat_spec.rb index 946e0fe3a14..ef0ddc3f62e 100644 --- a/spec/models/user_stat_spec.rb +++ b/spec/models/user_stat_spec.rb @@ -88,6 +88,46 @@ describe UserStat do post.user.user_stat.reload expect(post.user.user_stat.first_unread_at).to eq_time(Time.zone.now) end + + it 'updates first unread pm timestamp correctly' do + pm_topic = Fabricate(:private_message_topic) + user = pm_topic.user + user.update!(last_seen_at: Time.zone.now) + create_post(user: user, topic_id: pm_topic.id) + + TopicUser.change(user.id, pm_topic.id, + notification_level: TopicUser.notification_levels[:tracking] + ) + + # user that is not tracking PM topic + user_2 = Fabricate(:user, last_seen_at: Time.zone.now) + pm_topic.allowed_users << user_2 + + TopicUser.change(user_2.id, pm_topic.id, + notification_level: TopicUser.notification_levels[:regular] + ) + + # User that has not been seen + user_3 = Fabricate(:user) + pm_topic.allowed_users << user_3 + + TopicUser.change(user_3.id, pm_topic.id, + notification_level: TopicUser.notification_levels[:tracking] + ) + + freeze_time 10.minutes.from_now + + post = create_post( + user: Fabricate(:admin), + topic_id: pm_topic.id + ) + + UserStat.ensure_consistency! + + expect(user.user_stat.reload.first_unread_pm_at).to eq_time(post.topic.updated_at) + expect(user_2.user_stat.reload.first_unread_pm_at).to_not eq_time(post.topic.updated_at) + expect(user_3.user_stat.reload.first_unread_pm_at).to_not eq_time(post.topic.updated_at) + end end describe 'update_time_read!' do