diff --git a/app/services/dismiss_topics.rb b/app/services/dismiss_topics.rb index fc722f878da..eb2e05b44ca 100644 --- a/app/services/dismiss_topics.rb +++ b/app/services/dismiss_topics.rb @@ -17,7 +17,7 @@ class DismissTopics @rows ||= @topics_scope .joins("LEFT JOIN topic_users ON topic_users.topic_id = topics.id AND topic_users.user_id = #{@user.id}") .where("topics.created_at >= ?", since_date) - .where("topic_users.id IS NULL") + .where("topic_users.id IS NULL OR topic_users.last_read_post_number IS NULL") .where("topics.archetype <> ?", Archetype.private_message) .order("topics.created_at DESC") .limit(SiteSetting.max_new_topics).map do |topic| diff --git a/db/post_migrate/20210208022738_move_new_since_to_new_table.rb b/db/post_migrate/20210208022738_move_new_since_to_new_table.rb deleted file mode 100644 index 21e00a63ac2..00000000000 --- a/db/post_migrate/20210208022738_move_new_since_to_new_table.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -class MoveNewSinceToNewTable < ActiveRecord::Migration[6.0] - def up - sql = <<~SQL - INSERT INTO dismissed_topic_users (user_id, topic_id, created_at) - SELECT users.id, topics.id, user_stats.new_since - FROM user_stats - JOIN users ON users.id = user_stats.user_id - JOIN user_options ON user_options.user_id = users.id - LEFT JOIN topics ON topics.created_at <= user_stats.new_since - AND topics.archetype <> :private_message - AND topics.created_at >= GREATEST(CASE - WHEN COALESCE(user_options.new_topic_duration_minutes, :default_duration) = :always THEN users.created_at - WHEN COALESCE(user_options.new_topic_duration_minutes, :default_duration) = :last_visit THEN COALESCE(users.previous_visit_at,users.created_at) - ELSE (:now::timestamp - INTERVAL '1 MINUTE' * COALESCE(user_options.new_topic_duration_minutes, :default_duration)) - END, :min_date) - AND topics.id IN(SELECT id FROM topics ORDER BY created_at DESC LIMIT :max_new_topics) - LEFT JOIN topic_users ON topics.id = topic_users.topic_id AND users.id = topic_users.user_id - LEFT JOIN dismissed_topic_users ON dismissed_topic_users.topic_id = topics.id AND users.id = dismissed_topic_users.user_id - WHERE user_stats.new_since IS NOT NULL - AND topic_users.id IS NULL - AND topics.id IS NOT NULL - AND dismissed_topic_users.id IS NULL - ORDER BY topics.created_at DESC - SQL - DB.exec(sql, - now: DateTime.now, - last_visit: User::NewTopicDuration::LAST_VISIT, - always: User::NewTopicDuration::ALWAYS, - default_duration: SiteSetting.default_other_new_topic_duration_minutes, - min_date: Time.at(SiteSetting.min_new_topics_time).to_datetime, - private_message: Archetype.private_message, - max_new_topics: SiteSetting.max_new_topics) - end - - def down - raise ActiveRecord::IrreversibleMigration - end -end diff --git a/db/post_migrate/20210208022739_move_new_since_to_new_table.rb b/db/post_migrate/20210208022739_move_new_since_to_new_table.rb new file mode 100644 index 00000000000..a0efe780e30 --- /dev/null +++ b/db/post_migrate/20210208022739_move_new_since_to_new_table.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +class MoveNewSinceToNewTable < ActiveRecord::Migration[6.0] + disable_ddl_transaction! + BATCH_SIZE = 30_000 + + def up + offset = 0 + loop do + user_stat_user_ids = DB.query("SELECT user_id FROM user_stats LIMIT #{BATCH_SIZE} OFFSET :offset", offset: offset).map(&:user_id) + break if user_stat_user_ids.count.zero? + sql = <<~SQL + INSERT INTO dismissed_topic_users (user_id, topic_id, created_at) + SELECT users.id, topics.id, user_stats.new_since + FROM user_stats + JOIN users ON users.id = user_stats.user_id + JOIN user_options ON user_options.user_id = users.id + LEFT JOIN topics ON topics.created_at <= user_stats.new_since + AND topics.archetype <> :private_message + AND topics.created_at >= GREATEST(CASE + WHEN COALESCE(user_options.new_topic_duration_minutes, :default_duration) = :always THEN users.created_at + WHEN COALESCE(user_options.new_topic_duration_minutes, :default_duration) = :last_visit THEN COALESCE(users.previous_visit_at,users.created_at) + ELSE (:now::timestamp - INTERVAL '1 MINUTE' * COALESCE(user_options.new_topic_duration_minutes, :default_duration)) + END, :min_date) + AND topics.id IN(SELECT id FROM topics ORDER BY created_at DESC LIMIT :max_new_topics) + LEFT JOIN topic_users ON topics.id = topic_users.topic_id AND users.id = topic_users.user_id + LEFT JOIN dismissed_topic_users ON dismissed_topic_users.topic_id = topics.id AND users.id = dismissed_topic_users.user_id + WHERE user_stats.new_since IS NOT NULL + AND user_stats.user_id IN (:user_stat_user_ids) + AND topic_users.last_read_post_number IS NULL + AND topics.id IS NOT NULL + AND dismissed_topic_users.id IS NULL + ORDER BY topics.created_at DESC + ON CONFLICT DO NOTHING + SQL + DB.exec(sql, + now: DateTime.now, + last_visit: User::NewTopicDuration::LAST_VISIT, + always: User::NewTopicDuration::ALWAYS, + default_duration: SiteSetting.default_other_new_topic_duration_minutes, + min_date: Time.at(SiteSetting.min_new_topics_time).to_datetime, + private_message: Archetype.private_message, + user_stat_user_ids: user_stat_user_ids, + max_new_topics: SiteSetting.max_new_topics) + offset = offset + BATCH_SIZE + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/services/dismiss_topics_spec.rb b/spec/services/dismiss_topics_spec.rb index b61db8a2933..ba640ebb1ef 100644 --- a/spec/services/dismiss_topics_spec.rb +++ b/spec/services/dismiss_topics_spec.rb @@ -29,11 +29,17 @@ describe DismissTopics do end it 'respects seen topics' do - Fabricate(:topic_user, user: user, topic: topic1) - Fabricate(:topic_user, user: user, topic: topic2) + Fabricate(:topic_user, user: user, topic: topic1, last_read_post_number: 1) + Fabricate(:topic_user, user: user, topic: topic2, last_read_post_number: 1) expect { described_class.new(user, Topic.all).perform! }.to change { DismissedTopicUser.count }.by(0) end + it 'dismisses when topic user without last_read_post_number' do + Fabricate(:topic_user, user: user, topic: topic1, last_read_post_number: nil) + Fabricate(:topic_user, user: user, topic: topic2, last_read_post_number: nil) + expect { described_class.new(user, Topic.all).perform! }.to change { DismissedTopicUser.count }.by(2) + end + it 'respects new_topic_duration_minutes' do user.user_option.update!(new_topic_duration_minutes: 70)