From 549ab089eed07ee3ad90288ee5e134b4cfdc1e76 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 31 Jul 2024 12:50:13 +0200 Subject: [PATCH] Change grouped notifications API shape (take 2) (#31214) --- .../api/v2_alpha/notifications_controller.rb | 6 ++++-- .../mastodon/actions/notification_groups.ts | 12 +++++------ app/javascript/mastodon/api/notifications.ts | 13 +++++++++--- .../mastodon/api_types/notifications.ts | 8 ++++++- .../mastodon/models/notification_group.ts | 5 ++--- .../grouped_notifications_presenter.rb | 21 +++++++++++++++++++ .../dedup_notification_group_serializer.rb | 7 +++++++ .../rest/notification_group_serializer.rb | 12 +++++++++-- .../api/v2_alpha/notifications_spec.rb | 6 +++--- 9 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 app/presenters/grouped_notifications_presenter.rb create mode 100644 app/serializers/rest/dedup_notification_group_serializer.rb diff --git a/app/controllers/api/v2_alpha/notifications_controller.rb b/app/controllers/api/v2_alpha/notifications_controller.rb index d1126baaf4..837499e898 100644 --- a/app/controllers/api/v2_alpha/notifications_controller.rb +++ b/app/controllers/api/v2_alpha/notifications_controller.rb @@ -33,7 +33,8 @@ class Api::V2Alpha::NotificationsController < Api::BaseController 'app.notification_grouping.status.unique_count' => statuses.uniq.size ) - render json: @grouped_notifications, each_serializer: REST::NotificationGroupSerializer, relationships: @relationships, group_metadata: @group_metadata + presenter = GroupedNotificationsPresenter.new(@grouped_notifications) + render json: presenter, serializer: REST::DedupNotificationGroupSerializer, relationships: @relationships, group_metadata: @group_metadata end end @@ -47,7 +48,8 @@ class Api::V2Alpha::NotificationsController < Api::BaseController def show @notification = current_account.notifications.without_suspended.find_by!(group_key: params[:id]) - render json: NotificationGroup.from_notification(@notification), serializer: REST::NotificationGroupSerializer + presenter = GroupedNotificationsPresenter.new([NotificationGroup.from_notification(@notification)]) + render json: presenter, serializer: REST::DedupNotificationGroupSerializer end def clear diff --git a/app/javascript/mastodon/actions/notification_groups.ts b/app/javascript/mastodon/actions/notification_groups.ts index 8fdec6e48b..fc5807c8c0 100644 --- a/app/javascript/mastodon/actions/notification_groups.ts +++ b/app/javascript/mastodon/actions/notification_groups.ts @@ -38,10 +38,6 @@ function dispatchAssociatedRecords( const fetchedStatuses: ApiStatusJSON[] = []; notifications.forEach((notification) => { - if ('sample_accounts' in notification) { - fetchedAccounts.push(...notification.sample_accounts); - } - if (notification.type === 'admin.report') { fetchedAccounts.push(notification.report.target_account); } @@ -75,7 +71,9 @@ export const fetchNotifications = createDataLoadingThunk( : excludeAllTypesExcept(activeFilter), }); }, - ({ notifications }, { dispatch }) => { + ({ notifications, accounts, statuses }, { dispatch }) => { + dispatch(importFetchedAccounts(accounts)); + dispatch(importFetchedStatuses(statuses)); dispatchAssociatedRecords(dispatch, notifications); const payload: (ApiNotificationGroupJSON | NotificationGap)[] = notifications; @@ -95,7 +93,9 @@ export const fetchNotificationsGap = createDataLoadingThunk( async (params: { gap: NotificationGap }) => apiFetchNotifications({ max_id: params.gap.maxId }), - ({ notifications }, { dispatch }) => { + ({ notifications, accounts, statuses }, { dispatch }) => { + dispatch(importFetchedAccounts(accounts)); + dispatch(importFetchedStatuses(statuses)); dispatchAssociatedRecords(dispatch, notifications); return { notifications }; diff --git a/app/javascript/mastodon/api/notifications.ts b/app/javascript/mastodon/api/notifications.ts index c1ab6f70ca..ed187da5ec 100644 --- a/app/javascript/mastodon/api/notifications.ts +++ b/app/javascript/mastodon/api/notifications.ts @@ -1,17 +1,24 @@ import api, { apiRequest, getLinks } from 'mastodon/api'; -import type { ApiNotificationGroupJSON } from 'mastodon/api_types/notifications'; +import type { ApiNotificationGroupsResultJSON } from 'mastodon/api_types/notifications'; export const apiFetchNotifications = async (params?: { exclude_types?: string[]; max_id?: string; }) => { - const response = await api().request({ + const response = await api().request({ method: 'GET', url: '/api/v2_alpha/notifications', params, }); - return { notifications: response.data, links: getLinks(response) }; + const { statuses, accounts, notification_groups } = response.data; + + return { + statuses, + accounts, + notifications: notification_groups, + links: getLinks(response), + }; }; export const apiClearNotifications = () => diff --git a/app/javascript/mastodon/api_types/notifications.ts b/app/javascript/mastodon/api_types/notifications.ts index d7cbbca73b..fba0b7941b 100644 --- a/app/javascript/mastodon/api_types/notifications.ts +++ b/app/javascript/mastodon/api_types/notifications.ts @@ -51,7 +51,7 @@ export interface BaseNotificationGroupJSON { group_key: string; notifications_count: number; type: NotificationType; - sample_accounts: ApiAccountJSON[]; + sample_account_ids: string[]; latest_page_notification_at: string; // FIXME: This will only be present if the notification group is returned in a paginated list, not requested directly most_recent_notification_id: string; page_min_id?: string; @@ -143,3 +143,9 @@ export type ApiNotificationGroupJSON = | AccountRelationshipSeveranceNotificationGroupJSON | NotificationGroupWithStatusJSON | ModerationWarningNotificationGroupJSON; + +export interface ApiNotificationGroupsResultJSON { + accounts: ApiAccountJSON[]; + statuses: ApiStatusJSON[]; + notification_groups: ApiNotificationGroupJSON[]; +} diff --git a/app/javascript/mastodon/models/notification_group.ts b/app/javascript/mastodon/models/notification_group.ts index 5fe1e6f2e4..330bba88a8 100644 --- a/app/javascript/mastodon/models/notification_group.ts +++ b/app/javascript/mastodon/models/notification_group.ts @@ -14,7 +14,7 @@ import type { ApiReportJSON } from 'mastodon/api_types/reports'; export const NOTIFICATIONS_GROUP_MAX_AVATARS = 8; interface BaseNotificationGroup - extends Omit { + extends Omit { sampleAccountIds: string[]; } @@ -115,8 +115,7 @@ function createAccountRelationshipSeveranceEventFromJSON( export function createNotificationGroupFromJSON( groupJson: ApiNotificationGroupJSON, ): NotificationGroup { - const { sample_accounts, ...group } = groupJson; - const sampleAccountIds = sample_accounts.map((account) => account.id); + const { sample_account_ids: sampleAccountIds, ...group } = groupJson; switch (group.type) { case 'favourite': diff --git a/app/presenters/grouped_notifications_presenter.rb b/app/presenters/grouped_notifications_presenter.rb new file mode 100644 index 0000000000..f01acfd41c --- /dev/null +++ b/app/presenters/grouped_notifications_presenter.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class GroupedNotificationsPresenter < ActiveModelSerializers::Model + def initialize(grouped_notifications) + super() + + @grouped_notifications = grouped_notifications + end + + def notification_groups + @grouped_notifications + end + + def statuses + @grouped_notifications.filter_map(&:target_status).uniq(&:id) + end + + def accounts + @grouped_notifications.flat_map(&:sample_accounts).uniq(&:id) + end +end diff --git a/app/serializers/rest/dedup_notification_group_serializer.rb b/app/serializers/rest/dedup_notification_group_serializer.rb new file mode 100644 index 0000000000..fd0c6b3c41 --- /dev/null +++ b/app/serializers/rest/dedup_notification_group_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class REST::DedupNotificationGroupSerializer < ActiveModel::Serializer + has_many :accounts, serializer: REST::AccountSerializer + has_many :statuses, serializer: REST::StatusSerializer + has_many :notification_groups, serializer: REST::NotificationGroupSerializer +end diff --git a/app/serializers/rest/notification_group_serializer.rb b/app/serializers/rest/notification_group_serializer.rb index 749f717754..b855f1cba9 100644 --- a/app/serializers/rest/notification_group_serializer.rb +++ b/app/serializers/rest/notification_group_serializer.rb @@ -8,12 +8,20 @@ class REST::NotificationGroupSerializer < ActiveModel::Serializer attribute :page_max_id, if: :paginated? attribute :latest_page_notification_at, if: :paginated? - has_many :sample_accounts, serializer: REST::AccountSerializer - belongs_to :target_status, key: :status, if: :status_type?, serializer: REST::StatusSerializer + attribute :sample_account_ids + attribute :status_id, if: :status_type? belongs_to :report, if: :report_type?, serializer: REST::ReportSerializer belongs_to :account_relationship_severance_event, key: :event, if: :relationship_severance_event?, serializer: REST::AccountRelationshipSeveranceEventSerializer belongs_to :account_warning, key: :moderation_warning, if: :moderation_warning_event?, serializer: REST::AccountWarningSerializer + def sample_account_ids + object.sample_accounts.pluck(:id).map(&:to_s) + end + + def status_id + object.target_status&.id&.to_s + end + def status_type? [:favourite, :reblog, :status, :mention, :poll, :update].include?(object.type) end diff --git a/spec/requests/api/v2_alpha/notifications_spec.rb b/spec/requests/api/v2_alpha/notifications_spec.rb index 381987e7e7..fc1daef43f 100644 --- a/spec/requests/api/v2_alpha/notifications_spec.rb +++ b/spec/requests/api/v2_alpha/notifications_spec.rb @@ -135,7 +135,7 @@ RSpec.describe 'Notifications' do expect(response).to have_http_status(200) expect(body_json_types.uniq).to eq ['mention'] - expect(body_as_json[0][:page_min_id]).to_not be_nil + expect(body_as_json.dig(:notification_groups, 0, :page_min_id)).to_not be_nil end end @@ -147,7 +147,7 @@ RSpec.describe 'Notifications' do notifications = user.account.notifications - expect(body_as_json.size) + expect(body_as_json[:notification_groups].size) .to eq(params[:limit]) expect(response) @@ -161,7 +161,7 @@ RSpec.describe 'Notifications' do end def body_json_types - body_as_json.pluck(:type) + body_as_json[:notification_groups].pluck(:type) end end