From d6f5ee75ab91eba676e4e200d4e6a98a5aed91ef Mon Sep 17 00:00:00 2001 From: Renaud Chaput Date: Wed, 25 Sep 2024 15:36:19 +0200 Subject: [PATCH] Add notification grouping for follow notifications (#32085) --- .../mastodon/actions/notification_groups.ts | 9 +++++- app/javascript/mastodon/api/notifications.ts | 1 + app/models/notification.rb | 29 ++++++++++++++++++- app/services/notify_service.rb | 21 +------------- 4 files changed, 38 insertions(+), 22 deletions(-) diff --git a/app/javascript/mastodon/actions/notification_groups.ts b/app/javascript/mastodon/actions/notification_groups.ts index 9d3fc0d42..b40b04f8c 100644 --- a/app/javascript/mastodon/actions/notification_groups.ts +++ b/app/javascript/mastodon/actions/notification_groups.ts @@ -68,10 +68,15 @@ function dispatchAssociatedRecords( dispatch(importFetchedStatuses(fetchedStatuses)); } +const supportedGroupedNotificationTypes = ['favourite', 'reblog']; + export const fetchNotifications = createDataLoadingThunk( 'notificationGroups/fetch', async (_params, { getState }) => - apiFetchNotificationGroups({ exclude_types: getExcludedTypes(getState()) }), + apiFetchNotificationGroups({ + grouped_types: supportedGroupedNotificationTypes, + exclude_types: getExcludedTypes(getState()), + }), ({ notifications, accounts, statuses }, { dispatch }) => { dispatch(importFetchedAccounts(accounts)); dispatch(importFetchedStatuses(statuses)); @@ -93,6 +98,7 @@ export const fetchNotificationsGap = createDataLoadingThunk( 'notificationGroups/fetchGap', async (params: { gap: NotificationGap }, { getState }) => apiFetchNotificationGroups({ + grouped_types: supportedGroupedNotificationTypes, max_id: params.gap.maxId, exclude_types: getExcludedTypes(getState()), }), @@ -109,6 +115,7 @@ export const pollRecentNotifications = createDataLoadingThunk( 'notificationGroups/pollRecentNotifications', async (_params, { getState }) => { return apiFetchNotificationGroups({ + grouped_types: supportedGroupedNotificationTypes, max_id: undefined, exclude_types: getExcludedTypes(getState()), // In slow mode, we don't want to include notifications that duplicate the already-displayed ones diff --git a/app/javascript/mastodon/api/notifications.ts b/app/javascript/mastodon/api/notifications.ts index 92863ac5c..813e2f3a1 100644 --- a/app/javascript/mastodon/api/notifications.ts +++ b/app/javascript/mastodon/api/notifications.ts @@ -31,6 +31,7 @@ export const apiFetchNotifications = async ( export const apiFetchNotificationGroups = async (params?: { url?: string; + grouped_types?: string[]; exclude_types?: string[]; max_id?: string; since_id?: string; diff --git a/app/models/notification.rb b/app/models/notification.rb index 44a43d2ec..695f39a31 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -20,6 +20,7 @@ class Notification < ApplicationRecord self.inheritance_column = nil include Paginable + include Redisable LEGACY_TYPE_CLASS_MAP = { 'Mention' => :mention, @@ -30,7 +31,9 @@ class Notification < ApplicationRecord 'Poll' => :poll, }.freeze - GROUPABLE_NOTIFICATION_TYPES = %i(favourite reblog).freeze + # `set_group_key!` needs to be updated if this list changes + GROUPABLE_NOTIFICATION_TYPES = %i(favourite reblog follow).freeze + MAXIMUM_GROUP_SPAN_HOURS = 12 # Please update app/javascript/api_types/notification.ts if you change this PROPERTIES = { @@ -123,6 +126,30 @@ class Notification < ApplicationRecord end end + def set_group_key! + return if filtered? || Notification::GROUPABLE_NOTIFICATION_TYPES.exclude?(type) + + type_prefix = case type + when :favourite, :reblog + [type, target_status&.id].join('-') + when :follow + type + else + raise NotImplementedError + end + redis_key = "notif-group/#{account.id}/#{type_prefix}" + hour_bucket = activity.created_at.utc.to_i / 1.hour.to_i + + # Reuse previous group if it does not span too large an amount of time + previous_bucket = redis.get(redis_key).to_i + hour_bucket = previous_bucket if hour_bucket < previous_bucket + MAXIMUM_GROUP_SPAN_HOURS + + # We do not concern ourselves with race conditions since we use hour buckets + redis.set(redis_key, hour_bucket, ex: MAXIMUM_GROUP_SPAN_HOURS.hours.to_i) + + self.group_key = "#{type_prefix}-#{hour_bucket}" + end + class << self def browserable(types: [], exclude_types: [], from_account_id: nil, include_filtered: false) requested_types = if types.empty? diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index 97eee0548..9aebab787 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -3,8 +3,6 @@ class NotifyService < BaseService include Redisable - MAXIMUM_GROUP_SPAN_HOURS = 12 - # TODO: the severed_relationships type probably warrants email notifications NON_EMAIL_TYPES = %i( admin.report @@ -216,7 +214,7 @@ class NotifyService < BaseService return if drop? @notification.filtered = filter? - @notification.group_key = notification_group_key + @notification.set_group_key! @notification.save! # It's possible the underlying activity has been deleted @@ -236,23 +234,6 @@ class NotifyService < BaseService private - def notification_group_key - return nil if @notification.filtered || Notification::GROUPABLE_NOTIFICATION_TYPES.exclude?(@notification.type) - - type_prefix = "#{@notification.type}-#{@notification.target_status.id}" - redis_key = "notif-group/#{@recipient.id}/#{type_prefix}" - hour_bucket = @notification.activity.created_at.utc.to_i / 1.hour.to_i - - # Reuse previous group if it does not span too large an amount of time - previous_bucket = redis.get(redis_key).to_i - hour_bucket = previous_bucket if hour_bucket < previous_bucket + MAXIMUM_GROUP_SPAN_HOURS - - # We do not concern ourselves with race conditions since we use hour buckets - redis.set(redis_key, hour_bucket, ex: MAXIMUM_GROUP_SPAN_HOURS.hours.to_i) - - "#{type_prefix}-#{hour_bucket}" - end - def drop? DropCondition.new(@notification).drop? end