PERF: Updating first unread PM for user not respecting limits. (#15056)
Inb8c8909a9d
, we introduced a regression where users may have had their `UserStat.first_unread_pm_at` set incorrectly. This commit introduces a migration to reset `UserStat.first_unread_pm_at` back to `User#created_at`. Follow-up tob8c8909a9d
.
This commit is contained in:
parent
db4c52ca26
commit
8226ab1099
|
@ -53,6 +53,14 @@ class UserStat < ActiveRecord::Base
|
||||||
)
|
)
|
||||||
GROUP BY tau.user_id
|
GROUP BY tau.user_id
|
||||||
) AS X ON X.user_id = u1.id
|
) AS X ON X.user_id = u1.id
|
||||||
|
WHERE u1.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
|
) AS Z
|
||||||
WHERE us.user_id = Z.user_id
|
WHERE us.user_id = Z.user_id
|
||||||
SQL
|
SQL
|
||||||
|
|
|
@ -0,0 +1,16 @@
|
||||||
|
# frozen_string_literal: true
|
||||||
|
|
||||||
|
class ResetFirstUnreadPmAtOnUserStat < ActiveRecord::Migration[6.1]
|
||||||
|
def up
|
||||||
|
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
|
|
@ -90,6 +90,8 @@ describe UserStat do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'updates first unread pm timestamp correctly' do
|
it 'updates first unread pm timestamp correctly' do
|
||||||
|
freeze_time
|
||||||
|
|
||||||
pm_topic = Fabricate(:private_message_topic)
|
pm_topic = Fabricate(:private_message_topic)
|
||||||
user = pm_topic.user
|
user = pm_topic.user
|
||||||
user.update!(last_seen_at: Time.zone.now)
|
user.update!(last_seen_at: Time.zone.now)
|
||||||
|
@ -107,26 +109,57 @@ describe UserStat do
|
||||||
notification_level: TopicUser.notification_levels[:regular]
|
notification_level: TopicUser.notification_levels[:regular]
|
||||||
)
|
)
|
||||||
|
|
||||||
# User that has not been seen
|
# User that has not been seen recently
|
||||||
user_3 = Fabricate(:user)
|
user_3 = Fabricate(:user, last_seen_at: 1.year.ago)
|
||||||
pm_topic.allowed_users << user_3
|
pm_topic.allowed_users << user_3
|
||||||
|
|
||||||
TopicUser.change(user_3.id, pm_topic.id,
|
TopicUser.change(user_3.id, pm_topic.id,
|
||||||
notification_level: TopicUser.notification_levels[:tracking]
|
notification_level: TopicUser.notification_levels[:tracking]
|
||||||
)
|
)
|
||||||
|
|
||||||
freeze_time 10.minutes.from_now
|
user_3_orig_first_unread_pm_at = user_3.user_stat.first_unread_pm_at
|
||||||
|
|
||||||
post = create_post(
|
# User that is not related to the PM
|
||||||
user: Fabricate(:admin),
|
user_4 = Fabricate(:user, last_seen_at: Time.zone.now)
|
||||||
topic_id: pm_topic.id
|
user_4_orig_first_unread_pm_at = user_4.user_stat.first_unread_pm_at
|
||||||
|
|
||||||
|
# User for another PM topic
|
||||||
|
pm_topic_2 = Fabricate(:private_message_topic)
|
||||||
|
user_5 = pm_topic_2.user
|
||||||
|
user_5.update!(last_seen_at: Time.zone.now)
|
||||||
|
create_post(user: user_5, topic_id: pm_topic_2.id)
|
||||||
|
|
||||||
|
TopicUser.change(user_5.id, pm_topic_2.id,
|
||||||
|
notification_level: TopicUser.notification_levels[:tracking]
|
||||||
)
|
)
|
||||||
|
|
||||||
UserStat.ensure_consistency!
|
# User out of last seen limit
|
||||||
|
user_6 = Fabricate(:user, last_seen_at: Time.zone.now)
|
||||||
|
pm_topic.allowed_users << user_6
|
||||||
|
create_post(user: user_6, topic_id: pm_topic.id)
|
||||||
|
|
||||||
expect(user.user_stat.reload.first_unread_pm_at).to eq_time(post.topic.updated_at)
|
TopicUser.change(user_6.id, pm_topic.id,
|
||||||
expect(user_2.user_stat.reload.first_unread_pm_at).to_not eq_time(post.topic.updated_at)
|
notification_level: TopicUser.notification_levels[:tracking]
|
||||||
expect(user_3.user_stat.reload.first_unread_pm_at).to_not eq_time(post.topic.updated_at)
|
)
|
||||||
|
|
||||||
|
user_6_orig_first_unread_pm_at = user_6.user_stat.first_unread_pm_at
|
||||||
|
|
||||||
|
admin = Fabricate(:admin)
|
||||||
|
create_post(user: admin, topic_id: pm_topic.id)
|
||||||
|
create_post(user: admin, topic_id: pm_topic_2.id)
|
||||||
|
pm_topic.update!(updated_at: 10.minutes.from_now)
|
||||||
|
pm_topic_2.update!(updated_at: 20.minutes.from_now)
|
||||||
|
|
||||||
|
stub_const(UserStat, "UPDATE_UNREAD_USERS_LIMIT", 4) do
|
||||||
|
UserStat.ensure_consistency!
|
||||||
|
end
|
||||||
|
|
||||||
|
expect(user.user_stat.reload.first_unread_pm_at).to eq_time(pm_topic.reload.updated_at)
|
||||||
|
expect(user_2.user_stat.reload.first_unread_pm_at).to eq_time(UserStat::UPDATE_UNREAD_MINUTES_AGO.minutes.ago)
|
||||||
|
expect(user_3.user_stat.reload.first_unread_pm_at).to eq_time(user_3_orig_first_unread_pm_at)
|
||||||
|
expect(user_4.user_stat.reload.first_unread_pm_at).to eq_time(UserStat::UPDATE_UNREAD_MINUTES_AGO.minutes.ago)
|
||||||
|
expect(user_5.user_stat.reload.first_unread_pm_at).to eq_time(pm_topic_2.reload.updated_at)
|
||||||
|
expect(user_6.user_stat.reload.first_unread_pm_at).to eq_time(user_6_orig_first_unread_pm_at)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue