From 20531d1e078a27116dea53ce21c1b32bf3774eca Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 25 Feb 2025 15:37:44 +0100 Subject: [PATCH] Fix `GET /api/v2/notifications/:id` and `POST /api/v2/notifications/:id/dismiss` for ungrouped notifications (#33990) --- .../api/v2/notifications_controller.rb | 4 +-- app/models/concerns/notification/groups.rb | 4 +++ spec/requests/api/v2/notifications_spec.rb | 25 +++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v2/notifications_controller.rb b/app/controllers/api/v2/notifications_controller.rb index cc38b95114..848c361cfc 100644 --- a/app/controllers/api/v2/notifications_controller.rb +++ b/app/controllers/api/v2/notifications_controller.rb @@ -46,7 +46,7 @@ class Api::V2::NotificationsController < Api::BaseController end def show - @notification = current_account.notifications.without_suspended.find_by!(group_key: params[:group_key]) + @notification = current_account.notifications.without_suspended.by_group_key(params[:group_key]).take! presenter = GroupedNotificationsPresenter.new(NotificationGroup.from_notifications([@notification])) render json: presenter, serializer: REST::DedupNotificationGroupSerializer end @@ -57,7 +57,7 @@ class Api::V2::NotificationsController < Api::BaseController end def dismiss - current_account.notifications.where(group_key: params[:group_key]).destroy_all + current_account.notifications.by_group_key(params[:group_key]).destroy_all render_empty end diff --git a/app/models/concerns/notification/groups.rb b/app/models/concerns/notification/groups.rb index 2f29bd9cea..914910e8f3 100644 --- a/app/models/concerns/notification/groups.rb +++ b/app/models/concerns/notification/groups.rb @@ -7,6 +7,10 @@ module Notification::Groups GROUPABLE_NOTIFICATION_TYPES = %i(favourite reblog follow).freeze MAXIMUM_GROUP_SPAN_HOURS = 12 + included do + scope :by_group_key, ->(group_key) { group_key&.start_with?('ungrouped-') ? where(id: group_key.delete_prefix('ungrouped-')) : where(group_key: group_key) } + end + def set_group_key! return if filtered? || GROUPABLE_NOTIFICATION_TYPES.exclude?(type) diff --git a/spec/requests/api/v2/notifications_spec.rb b/spec/requests/api/v2/notifications_spec.rb index b2f6d71b51..a7608e1419 100644 --- a/spec/requests/api/v2/notifications_spec.rb +++ b/spec/requests/api/v2/notifications_spec.rb @@ -365,6 +365,18 @@ RSpec.describe 'Notifications' do .to start_with('application/json') end + context 'with an ungrouped notification' do + let(:notification) { Fabricate(:notification, account: user.account, type: :favourite) } + + it 'returns http success' do + get "/api/v2/notifications/ungrouped-#{notification.id}", headers: headers + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + end + end + context 'when notification belongs to someone else' do let(:notification) { Fabricate(:notification, group_key: 'foobar') } @@ -396,6 +408,19 @@ RSpec.describe 'Notifications' do expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound) end + context 'with an ungrouped notification' do + let(:notification) { Fabricate(:notification, account: user.account, type: :favourite) } + + it 'destroys the notification' do + post "/api/v2/notifications/ungrouped-#{notification.id}/dismiss", headers: headers + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + expect { notification.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + context 'when notification belongs to someone else' do let(:notification) { Fabricate(:notification, group_key: 'foobar') }