mirror of https://github.com/tootsuite/mastodon
Do not dismiss notification requests for ever.
"Dismiss" now only applies to a single notification request. If the same sender mentions the recipient again, a new notification request is created.fixes/dismissing-notification-requests-dismisses-too-much
parent
36592d10aa
commit
c17851bc7e
|
@ -235,7 +235,7 @@ class NotifyService < BaseService
|
|||
def update_notification_request!
|
||||
return unless @notification.type == :mention
|
||||
|
||||
notification_request = NotificationRequest.find_or_initialize_by(account_id: @recipient.id, from_account_id: @notification.from_account_id)
|
||||
notification_request = NotificationRequest.find_or_initialize_by(account_id: @recipient.id, from_account_id: @notification.from_account_id, dismissed: false)
|
||||
notification_request.last_status_id = @notification.target_status.id
|
||||
notification_request.save
|
||||
end
|
||||
|
|
|
@ -0,0 +1,10 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class ChangeUniqueIndexOnNotificationRequests < ActiveRecord::Migration[7.1]
|
||||
disable_ddl_transaction!
|
||||
|
||||
def change
|
||||
remove_index :notification_requests, [:account_id, :from_account_id], unique: true
|
||||
add_index :notification_requests, [:account_id, :from_account_id, :last_status_id], unique: true, algorithm: :concurrently
|
||||
end
|
||||
end
|
|
@ -10,7 +10,7 @@
|
|||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema[7.1].define(version: 2024_06_07_094856) do
|
||||
ActiveRecord::Schema[7.1].define(version: 2024_07_10_153313) do
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "plpgsql"
|
||||
|
||||
|
@ -709,7 +709,7 @@ ActiveRecord::Schema[7.1].define(version: 2024_06_07_094856) do
|
|||
t.boolean "dismissed", default: false, null: false
|
||||
t.datetime "created_at", null: false
|
||||
t.datetime "updated_at", null: false
|
||||
t.index ["account_id", "from_account_id"], name: "index_notification_requests_on_account_id_and_from_account_id", unique: true
|
||||
t.index ["account_id", "from_account_id", "last_status_id"], name: "idx_on_account_id_from_account_id_last_status_id_bbe7648aec", unique: true
|
||||
t.index ["account_id", "id"], name: "index_notification_requests_on_account_id_and_id", order: { id: :desc }, where: "(dismissed = false)"
|
||||
t.index ["from_account_id"], name: "index_notification_requests_on_from_account_id"
|
||||
t.index ["last_status_id"], name: "index_notification_requests_on_last_status_id"
|
||||
|
|
|
@ -129,6 +129,51 @@ RSpec.describe NotifyService do
|
|||
end
|
||||
end
|
||||
|
||||
context 'with filtered notifications' do
|
||||
let(:unknown) { Fabricate(:account, username: 'unknown') }
|
||||
let(:status) { Fabricate(:status, account: unknown) }
|
||||
let(:activity) { Fabricate(:mention, account: recipient, status: status) }
|
||||
let(:type) { :mention }
|
||||
|
||||
before do
|
||||
Fabricate(:notification_policy, account: recipient, filter_not_following: true)
|
||||
end
|
||||
|
||||
it 'creates a filtered notification' do
|
||||
expect { subject }.to change(Notification, :count)
|
||||
expect(Notification.last).to be_filtered
|
||||
end
|
||||
|
||||
context 'when no notification request exists' do
|
||||
it 'creates a notification request' do
|
||||
expect { subject }.to change(NotificationRequest, :count)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a notification request exists' do
|
||||
let!(:notification_request) do
|
||||
Fabricate(:notification_request, account: recipient, from_account: unknown, last_status: Fabricate(:status, account: unknown), dismissed: dismissed)
|
||||
end
|
||||
|
||||
context 'when it is not dismissed' do
|
||||
let(:dismissed) { false }
|
||||
|
||||
it 'updates the existing notification request' do
|
||||
expect { subject }.to_not change(NotificationRequest, :count)
|
||||
expect(notification_request.reload.last_status).to eq status
|
||||
end
|
||||
end
|
||||
|
||||
context 'when it is dismissed' do
|
||||
let(:dismissed) { true }
|
||||
|
||||
it 'creates a new notification request' do
|
||||
expect { subject }.to change(NotificationRequest, :count)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe NotifyService::DismissCondition do
|
||||
subject { described_class.new(notification) }
|
||||
|
||||
|
|
Loading…
Reference in New Issue