From 598ae4f2da86029b1c3c3e35e64b89873037b598 Mon Sep 17 00:00:00 2001 From: Claire Date: Tue, 30 Jul 2024 10:39:11 +0200 Subject: [PATCH] Add endpoints for unread notifications count (#31191) --- app/controllers/api/base_controller.rb | 4 +- .../api/v1/notifications_controller.rb | 14 ++++ .../api/v2_alpha/notifications_controller.rb | 14 ++++ config/routes/api.rb | 2 + spec/requests/api/v1/notifications_spec.rb | 77 +++++++++++++++++++ .../api/v2_alpha/notifications_spec.rb | 77 +++++++++++++++++++ 6 files changed, 186 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index c1a5e43f88..0980e0ebbc 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -30,10 +30,10 @@ class Api::BaseController < ApplicationController protected - def limit_param(default_limit) + def limit_param(default_limit, max_limit = nil) return default_limit unless params[:limit] - [params[:limit].to_i.abs, default_limit * 2].min + [params[:limit].to_i.abs, max_limit || (default_limit * 2)].min end def params_slice(*keys) diff --git a/app/controllers/api/v1/notifications_controller.rb b/app/controllers/api/v1/notifications_controller.rb index 1d0aa10d2e..13919b400d 100644 --- a/app/controllers/api/v1/notifications_controller.rb +++ b/app/controllers/api/v1/notifications_controller.rb @@ -7,6 +7,8 @@ class Api::V1::NotificationsController < Api::BaseController after_action :insert_pagination_headers, only: :index DEFAULT_NOTIFICATIONS_LIMIT = 40 + DEFAULT_NOTIFICATIONS_COUNT_LIMIT = 100 + MAX_NOTIFICATIONS_COUNT_LIMIT = 1_000 def index with_read_replica do @@ -17,6 +19,14 @@ class Api::V1::NotificationsController < Api::BaseController render json: @notifications, each_serializer: REST::NotificationSerializer, relationships: @relationships end + def unread_count + limit = limit_param(DEFAULT_NOTIFICATIONS_COUNT_LIMIT, MAX_NOTIFICATIONS_COUNT_LIMIT) + + with_read_replica do + render json: { count: browserable_account_notifications.paginate_by_min_id(limit, notification_marker&.last_read_id).count } + end + end + def show @notification = current_account.notifications.without_suspended.find(params[:id]) render json: @notification, serializer: REST::NotificationSerializer @@ -54,6 +64,10 @@ class Api::V1::NotificationsController < Api::BaseController ) end + def notification_marker + current_user.markers.find_by(timeline: 'notifications') + end + def target_statuses_from_notifications @notifications.reject { |notification| notification.target_status.nil? }.map(&:target_status) end diff --git a/app/controllers/api/v2_alpha/notifications_controller.rb b/app/controllers/api/v2_alpha/notifications_controller.rb index 83d40a0886..d1126baaf4 100644 --- a/app/controllers/api/v2_alpha/notifications_controller.rb +++ b/app/controllers/api/v2_alpha/notifications_controller.rb @@ -7,6 +7,8 @@ class Api::V2Alpha::NotificationsController < Api::BaseController after_action :insert_pagination_headers, only: :index DEFAULT_NOTIFICATIONS_LIMIT = 40 + DEFAULT_NOTIFICATIONS_COUNT_LIMIT = 100 + MAX_NOTIFICATIONS_COUNT_LIMIT = 1_000 def index with_read_replica do @@ -35,6 +37,14 @@ class Api::V2Alpha::NotificationsController < Api::BaseController end end + def unread_count + limit = limit_param(DEFAULT_NOTIFICATIONS_COUNT_LIMIT, MAX_NOTIFICATIONS_COUNT_LIMIT) + + with_read_replica do + render json: { count: browserable_account_notifications.paginate_groups_by_min_id(limit, min_id: notification_marker&.last_read_id).count } + end + end + def show @notification = current_account.notifications.without_suspended.find_by!(group_key: params[:id]) render json: NotificationGroup.from_notification(@notification), serializer: REST::NotificationGroupSerializer @@ -92,6 +102,10 @@ class Api::V2Alpha::NotificationsController < Api::BaseController ) end + def notification_marker + current_user.markers.find_by(timeline: 'notifications') + end + def target_statuses_from_notifications @notifications.filter_map(&:target_status) end diff --git a/config/routes/api.rb b/config/routes/api.rb index 9011916697..e488bb9187 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -167,6 +167,7 @@ namespace :api, format: false do resources :notifications, only: [:index, :show] do collection do post :clear + get :unread_count end member do @@ -336,6 +337,7 @@ namespace :api, format: false do resources :notifications, only: [:index, :show] do collection do post :clear + get :unread_count end member do diff --git a/spec/requests/api/v1/notifications_spec.rb b/spec/requests/api/v1/notifications_spec.rb index c9034c17dc..3d1e8a4787 100644 --- a/spec/requests/api/v1/notifications_spec.rb +++ b/spec/requests/api/v1/notifications_spec.rb @@ -8,6 +8,83 @@ RSpec.describe 'Notifications' do let(:scopes) { 'read:notifications write:notifications' } let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + describe 'GET /api/v1/notifications/unread_count', :inline_jobs do + subject do + get '/api/v1/notifications/unread_count', headers: headers, params: params + end + + let(:params) { {} } + + before do + first_status = PostStatusService.new.call(user.account, text: 'Test') + ReblogService.new.call(Fabricate(:account), first_status) + PostStatusService.new.call(Fabricate(:account), text: 'Hello @alice') + FavouriteService.new.call(Fabricate(:account), first_status) + FavouriteService.new.call(Fabricate(:account), first_status) + FollowService.new.call(Fabricate(:account), user.account) + end + + it_behaves_like 'forbidden for wrong scope', 'write write:notifications' + + context 'with no options' do + it 'returns expected notifications count' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 5 + end + end + + context 'with a read marker' do + before do + id = user.account.notifications.browserable.order(id: :desc).offset(2).first.id + user.markers.create!(timeline: 'notifications', last_read_id: id) + end + + it 'returns expected notifications count' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 2 + end + end + + context 'with exclude_types param' do + let(:params) { { exclude_types: %w(mention) } } + + it 'returns expected notifications count' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 4 + end + end + + context 'with a user-provided limit' do + let(:params) { { limit: 2 } } + + it 'returns a capped value' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 2 + end + end + + context 'when there are more notifications than the limit' do + before do + stub_const('Api::V1::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT', 2) + end + + it 'returns a capped value' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq Api::V1::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT + end + end + end + describe 'GET /api/v1/notifications', :inline_jobs do subject do get '/api/v1/notifications', headers: headers, params: params diff --git a/spec/requests/api/v2_alpha/notifications_spec.rb b/spec/requests/api/v2_alpha/notifications_spec.rb index 104651ebe3..381987e7e7 100644 --- a/spec/requests/api/v2_alpha/notifications_spec.rb +++ b/spec/requests/api/v2_alpha/notifications_spec.rb @@ -8,6 +8,83 @@ RSpec.describe 'Notifications' do let(:scopes) { 'read:notifications write:notifications' } let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + describe 'GET /api/v2_alpha/notifications/unread_count', :inline_jobs do + subject do + get '/api/v2_alpha/notifications/unread_count', headers: headers, params: params + end + + let(:params) { {} } + + before do + first_status = PostStatusService.new.call(user.account, text: 'Test') + ReblogService.new.call(Fabricate(:account), first_status) + PostStatusService.new.call(Fabricate(:account), text: 'Hello @alice') + FavouriteService.new.call(Fabricate(:account), first_status) + FavouriteService.new.call(Fabricate(:account), first_status) + FollowService.new.call(Fabricate(:account), user.account) + end + + it_behaves_like 'forbidden for wrong scope', 'write write:notifications' + + context 'with no options' do + it 'returns expected notifications count' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 4 + end + end + + context 'with a read marker' do + before do + id = user.account.notifications.browserable.order(id: :desc).offset(2).first.id + user.markers.create!(timeline: 'notifications', last_read_id: id) + end + + it 'returns expected notifications count' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 2 + end + end + + context 'with exclude_types param' do + let(:params) { { exclude_types: %w(mention) } } + + it 'returns expected notifications count' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 3 + end + end + + context 'with a user-provided limit' do + let(:params) { { limit: 2 } } + + it 'returns a capped value' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq 2 + end + end + + context 'when there are more notifications than the limit' do + before do + stub_const('Api::V2Alpha::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT', 2) + end + + it 'returns a capped value' do + subject + + expect(response).to have_http_status(200) + expect(body_as_json[:count]).to eq Api::V2Alpha::NotificationsController::DEFAULT_NOTIFICATIONS_COUNT_LIMIT + end + end + end + describe 'GET /api/v2_alpha/notifications', :inline_jobs do subject do get '/api/v2_alpha/notifications', headers: headers, params: params