FIX: Push notification delay should not be longer than specified (#20864)

When user.last_seen was less than push_notification_time_window_mins we
where delaying the notification for the whole
push_notification_time_window_mins PLUS the time the user was away from.

Originally reported in https://meta.discourse.org/t/-/259688
This commit is contained in:
Rafael dos Santos Silva 2023-03-28 13:22:54 -03:00 committed by GitHub
parent f8720a20f9
commit 2a7bdb2d66
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 6 deletions

View File

@ -55,12 +55,9 @@ class PostAlerter
if user.push_subscriptions.exists? if user.push_subscriptions.exists?
if user.seen_since?(SiteSetting.push_notification_time_window_mins.minutes.ago) if user.seen_since?(SiteSetting.push_notification_time_window_mins.minutes.ago)
Jobs.enqueue_in( delay =
SiteSetting.push_notification_time_window_mins.minutes, (SiteSetting.push_notification_time_window_mins - (Time.now - user.last_seen_at) / 60)
:send_push_notification, Jobs.enqueue_in(delay.minutes, :send_push_notification, user_id: user.id, payload: payload)
user_id: user.id,
payload: payload,
)
else else
Jobs.enqueue(:send_push_notification, user_id: user.id, payload: payload) Jobs.enqueue(:send_push_notification, user_id: user.id, payload: payload)
end end

View File

@ -1238,6 +1238,18 @@ RSpec.describe PostAlerter do
expect(Jobs::SendPushNotification.jobs[0]["at"]).not_to be_nil expect(Jobs::SendPushNotification.jobs[0]["at"]).not_to be_nil
end end
it "delays sending push notification for active online user for the correct delay ammount" do
evil_trout.update!(last_seen_at: 5.minutes.ago)
# SiteSetting.push_notification_time_window_mins is 10
# last_seen_at is 5 minutes ago
# 10 minutes from now - 5 minutes ago = 5 minutes
delay = 5.minutes.from_now.to_f
expect { mention_post }.to change { Jobs::SendPushNotification.jobs.count }
expect(Jobs::SendPushNotification.jobs[0]["at"]).to be_within(30.second).of(delay)
end
it "does not delay push notification for inactive offline user" do it "does not delay push notification for inactive offline user" do
evil_trout.update!(last_seen_at: 40.minutes.ago) evil_trout.update!(last_seen_at: 40.minutes.ago)