Skip to content

Commit

Permalink
Add notification grouping for follow notifications (mastodon#32085)
Browse files Browse the repository at this point in the history
  • Loading branch information
renchap authored Sep 25, 2024
1 parent 3dc4ddc commit d6f5ee7
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 22 deletions.
9 changes: 8 additions & 1 deletion app/javascript/mastodon/actions/notification_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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()),
}),
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/javascript/mastodon/api/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 28 additions & 1 deletion app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class Notification < ApplicationRecord
self.inheritance_column = nil

include Paginable
include Redisable

LEGACY_TYPE_CLASS_MAP = {
'Mention' => :mention,
Expand All @@ -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 = {
Expand Down Expand Up @@ -123,6 +126,30 @@ def target_status
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?
Expand Down
21 changes: 1 addition & 20 deletions app/services/notify_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -216,7 +214,7 @@ def call(recipient, type, activity)
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
Expand All @@ -236,23 +234,6 @@ def call(recipient, type, activity)

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
Expand Down

0 comments on commit d6f5ee7

Please sign in to comment.