Add option to request partial accounts in grouped notifications API (#31299)
This commit is contained in:
parent
5d890ebc57
commit
438dac99d6
4 changed files with 88 additions and 9 deletions
|
@ -16,10 +16,10 @@ class Api::V2Alpha::NotificationsController < Api::BaseController
|
||||||
@group_metadata = load_group_metadata
|
@group_metadata = load_group_metadata
|
||||||
@grouped_notifications = load_grouped_notifications
|
@grouped_notifications = load_grouped_notifications
|
||||||
@relationships = StatusRelationshipsPresenter.new(target_statuses_from_notifications, current_user&.account_id)
|
@relationships = StatusRelationshipsPresenter.new(target_statuses_from_notifications, current_user&.account_id)
|
||||||
@sample_accounts = @grouped_notifications.flat_map(&:sample_accounts)
|
@presenter = GroupedNotificationsPresenter.new(@grouped_notifications, expand_accounts: expand_accounts_param)
|
||||||
|
|
||||||
# Preload associations to avoid N+1s
|
# Preload associations to avoid N+1s
|
||||||
ActiveRecord::Associations::Preloader.new(records: @sample_accounts, associations: [:account_stat, { user: :role }]).call
|
ActiveRecord::Associations::Preloader.new(records: @presenter.accounts, associations: [:account_stat, { user: :role }]).call
|
||||||
end
|
end
|
||||||
|
|
||||||
MastodonOTELTracer.in_span('Api::V2Alpha::NotificationsController#index rendering') do |span|
|
MastodonOTELTracer.in_span('Api::V2Alpha::NotificationsController#index rendering') do |span|
|
||||||
|
@ -27,14 +27,14 @@ class Api::V2Alpha::NotificationsController < Api::BaseController
|
||||||
|
|
||||||
span.add_attributes(
|
span.add_attributes(
|
||||||
'app.notification_grouping.count' => @grouped_notifications.size,
|
'app.notification_grouping.count' => @grouped_notifications.size,
|
||||||
'app.notification_grouping.sample_account.count' => @sample_accounts.size,
|
'app.notification_grouping.account.count' => @presenter.accounts.size,
|
||||||
'app.notification_grouping.sample_account.unique_count' => @sample_accounts.pluck(:id).uniq.size,
|
'app.notification_grouping.partial_account.count' => @presenter.partial_accounts.size,
|
||||||
'app.notification_grouping.status.count' => statuses.size,
|
'app.notification_grouping.status.count' => statuses.size,
|
||||||
'app.notification_grouping.status.unique_count' => statuses.uniq.size
|
'app.notification_grouping.status.unique_count' => statuses.uniq.size,
|
||||||
|
'app.notification_grouping.expand_accounts_param' => expand_accounts_param
|
||||||
)
|
)
|
||||||
|
|
||||||
presenter = GroupedNotificationsPresenter.new(@grouped_notifications)
|
render json: @presenter, serializer: REST::DedupNotificationGroupSerializer, relationships: @relationships, group_metadata: @group_metadata, expand_accounts: expand_accounts_param
|
||||||
render json: presenter, serializer: REST::DedupNotificationGroupSerializer, relationships: @relationships, group_metadata: @group_metadata
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -131,4 +131,15 @@ class Api::V2Alpha::NotificationsController < Api::BaseController
|
||||||
def pagination_params(core_params)
|
def pagination_params(core_params)
|
||||||
params.slice(:limit, :types, :exclude_types, :include_filtered).permit(:limit, :include_filtered, types: [], exclude_types: []).merge(core_params)
|
params.slice(:limit, :types, :exclude_types, :include_filtered).permit(:limit, :include_filtered, types: [], exclude_types: []).merge(core_params)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def expand_accounts_param
|
||||||
|
case params[:expand_accounts]
|
||||||
|
when nil, 'full'
|
||||||
|
'full'
|
||||||
|
when 'partial_avatars'
|
||||||
|
'partial_avatars'
|
||||||
|
else
|
||||||
|
raise Mastodon::InvalidParameterError, "Invalid value for 'expand_accounts': '#{params[:expand_accounts]}', allowed values are 'full' and 'partial_avatars'"
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,10 +1,11 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class GroupedNotificationsPresenter < ActiveModelSerializers::Model
|
class GroupedNotificationsPresenter < ActiveModelSerializers::Model
|
||||||
def initialize(grouped_notifications)
|
def initialize(grouped_notifications, options = {})
|
||||||
super()
|
super()
|
||||||
|
|
||||||
@grouped_notifications = grouped_notifications
|
@grouped_notifications = grouped_notifications
|
||||||
|
@options = options
|
||||||
end
|
end
|
||||||
|
|
||||||
def notification_groups
|
def notification_groups
|
||||||
|
@ -16,6 +17,28 @@ class GroupedNotificationsPresenter < ActiveModelSerializers::Model
|
||||||
end
|
end
|
||||||
|
|
||||||
def accounts
|
def accounts
|
||||||
|
@accounts ||= begin
|
||||||
|
if partial_avatars?
|
||||||
|
@grouped_notifications.map { |group| group.sample_accounts.first }.uniq(&:id)
|
||||||
|
else
|
||||||
@grouped_notifications.flat_map(&:sample_accounts).uniq(&:id)
|
@grouped_notifications.flat_map(&:sample_accounts).uniq(&:id)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def partial_accounts
|
||||||
|
@partial_accounts ||= begin
|
||||||
|
if partial_avatars?
|
||||||
|
@grouped_notifications.flat_map { |group| group.sample_accounts[1...] }.uniq(&:id).filter { |account| accounts.exclude?(account) }
|
||||||
|
else
|
||||||
|
[]
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def partial_avatars?
|
||||||
|
@options[:expand_accounts] == 'partial_avatars'
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
|
@ -1,7 +1,22 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class REST::DedupNotificationGroupSerializer < ActiveModel::Serializer
|
class REST::DedupNotificationGroupSerializer < ActiveModel::Serializer
|
||||||
|
class PartialAccountSerializer < REST::AccountSerializer
|
||||||
|
# This is a hack to reset ActiveModel::Serializer internals and only expose the attributes
|
||||||
|
# we care about.
|
||||||
|
self._attributes_data = {}
|
||||||
|
self._reflections = []
|
||||||
|
self._links = []
|
||||||
|
|
||||||
|
attributes :id, :acct, :locked, :bot, :url, :avatar, :avatar_static
|
||||||
|
end
|
||||||
|
|
||||||
has_many :accounts, serializer: REST::AccountSerializer
|
has_many :accounts, serializer: REST::AccountSerializer
|
||||||
|
has_many :partial_accounts, serializer: PartialAccountSerializer, if: :return_partial_accounts?
|
||||||
has_many :statuses, serializer: REST::StatusSerializer
|
has_many :statuses, serializer: REST::StatusSerializer
|
||||||
has_many :notification_groups, serializer: REST::NotificationGroupSerializer
|
has_many :notification_groups, serializer: REST::NotificationGroupSerializer
|
||||||
|
|
||||||
|
def return_partial_accounts?
|
||||||
|
instance_options[:expand_accounts] == 'partial_avatars'
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -160,6 +160,36 @@ RSpec.describe 'Notifications' do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when requesting stripped-down accounts' do
|
||||||
|
let(:params) { { expand_accounts: 'partial_avatars' } }
|
||||||
|
|
||||||
|
let(:recent_account) { Fabricate(:account) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
FavouriteService.new.call(recent_account, user.account.statuses.first)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns an account in "partial_accounts", with the expected keys', :aggregate_failures do
|
||||||
|
subject
|
||||||
|
|
||||||
|
expect(response).to have_http_status(200)
|
||||||
|
expect(body_as_json[:partial_accounts].size).to be > 0
|
||||||
|
expect(body_as_json[:partial_accounts][0].keys).to contain_exactly(:acct, :avatar, :avatar_static, :bot, :id, :locked, :url)
|
||||||
|
expect(body_as_json[:partial_accounts].pluck(:id)).to_not include(recent_account.id.to_s)
|
||||||
|
expect(body_as_json[:accounts].pluck(:id)).to include(recent_account.id.to_s)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when passing an invalid value for "expand_accounts"' do
|
||||||
|
let(:params) { { expand_accounts: 'unknown_foobar' } }
|
||||||
|
|
||||||
|
it 'returns http bad request' do
|
||||||
|
subject
|
||||||
|
|
||||||
|
expect(response).to have_http_status(400)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def body_json_types
|
def body_json_types
|
||||||
body_as_json[:notification_groups].pluck(:type)
|
body_as_json[:notification_groups].pluck(:type)
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue