diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 2fed3fdae95..fd90472c004 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -410,6 +410,11 @@ class ActivityPub::ProcessStatusUpdateService < BaseService return unless poll.present? && poll.expires_at.present? && poll.votes.exists? + # Do not schedule notifications for already-expired polls, as the + # notification worker would run immediately and re-create notifications + # that users may have already dismissed (see #37948) + return if poll.expired? + PollExpirationNotifyWorker.remove_from_scheduled(poll.id) if @previous_expires_at.present? && @previous_expires_at > poll.expires_at PollExpirationNotifyWorker.perform_at(poll.expires_at + 5.minutes, poll.id) end diff --git a/app/services/vote_service.rb b/app/services/vote_service.rb index 878350388b8..3247ac08004 100644 --- a/app/services/vote_service.rb +++ b/app/services/vote_service.rb @@ -51,6 +51,10 @@ class VoteService < BaseService def queue_final_poll_check! return unless @poll.expires? + # Do not schedule if the poll has already expired, as the worker would + # run immediately and potentially re-create dismissed notifications (#37948) + return if @poll.expired? + PollExpirationNotifyWorker.perform_at(@poll.expires_at + 5.minutes, @poll.id) end diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 475c4b71f99..9dd1411c02d 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -136,6 +136,22 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do end end + context 'when the poll has already expired' do + let(:poll) { Fabricate(:poll, status: status, account: status.account, expires_at: 1.day.ago) } + + before do + poll.votes.create!(account: Fabricate(:account, domain: nil), choice: 0) + end + + it 'does not schedule a PollExpirationNotifyWorker' do + expect(PollExpirationNotifyWorker).to_not have_enqueued_sidekiq_job(poll.id) + + subject.call(status, activity_json, object_json) + + expect(PollExpirationNotifyWorker).to_not have_enqueued_sidekiq_job(poll.id) + end + end + context 'when the status changes a poll despite being not explicitly marked as updated' do let(:account) { Fabricate(:account, domain: 'example.com') } let!(:expiration) { 10.days.from_now.utc } diff --git a/spec/services/vote_service_spec.rb b/spec/services/vote_service_spec.rb index 88207b001c3..9e1dea8deae 100644 --- a/spec/services/vote_service_spec.rb +++ b/spec/services/vote_service_spec.rb @@ -24,6 +24,15 @@ RSpec.describe VoteService do end end + context 'when the remote poll has already expired' do + let(:poll) { Fabricate(:poll, account: account, options: %w(Foo Bar), expires_at: 1.day.ago) } + + it 'does not schedule a PollExpirationNotifyWorker' do + expect { subject.call(voter, poll, [0]) } + .to_not change { PollExpirationNotifyWorker.jobs.size } + end + end + context 'when the poll was created by a remote account' do let(:account) { Fabricate(:account, domain: 'host.example') } diff --git a/spec/workers/poll_expiration_notify_worker_spec.rb b/spec/workers/poll_expiration_notify_worker_spec.rb index 190630608c4..27acfceeaf0 100644 --- a/spec/workers/poll_expiration_notify_worker_spec.rb +++ b/spec/workers/poll_expiration_notify_worker_spec.rb @@ -54,5 +54,23 @@ RSpec.describe PollExpirationNotifyWorker do end end end + + context 'when a voter has already dismissed the notification' do + let(:voter) { Fabricate(:account, domain: nil) } + let(:poll) { Fabricate(:poll, account: status.account, status: status, expires_at: 1.day.ago) } + + before do + poll.votes.create!(account: voter, choice: 0) + # First run creates the notification + described_class.new.perform(poll.id) + # User dismisses the notification + Notification.where(account: voter, activity: poll, type: :poll).destroy_all + end + + it 'does not re-create the notification on a second run' do + expect { described_class.new.perform(poll.id) } + .to_not change { Notification.where(account: voter, activity: poll, type: :poll).count }.from(0) + end + end end end