Add notifications when a reblogged status has been updated (#17404)

* Add notifications when a reblogged status has been updated

* Change wording to say "edit" instead of "update" and add missing controls

* Replace previous update notifications with the most up-to-date one
This commit is contained in:
Eugen Rochko 2022-02-11 22:20:19 +01:00 committed by GitHub
parent d0fcf07436
commit 8f03b7a2fb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 103 additions and 8 deletions

View File

@ -26,6 +26,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController
mention: alerts_enabled, mention: alerts_enabled,
poll: alerts_enabled, poll: alerts_enabled,
status: alerts_enabled, status: alerts_enabled,
update: alerts_enabled,
}, },
} }
@ -61,6 +62,15 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController
end end
def data_params def data_params
@data_params ||= params.require(:data).permit(:policy, alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) @data_params ||= params.require(:data).permit(:policy, alerts: [
:follow,
:follow_request,
:favourite,
:reblog,
:mention,
:poll,
:status,
:update,
])
end end
end end

View File

@ -34,7 +34,6 @@ export const NOTIFICATIONS_LOAD_PENDING = 'NOTIFICATIONS_LOAD_PENDING';
export const NOTIFICATIONS_MOUNT = 'NOTIFICATIONS_MOUNT'; export const NOTIFICATIONS_MOUNT = 'NOTIFICATIONS_MOUNT';
export const NOTIFICATIONS_UNMOUNT = 'NOTIFICATIONS_UNMOUNT'; export const NOTIFICATIONS_UNMOUNT = 'NOTIFICATIONS_UNMOUNT';
export const NOTIFICATIONS_MARK_AS_READ = 'NOTIFICATIONS_MARK_AS_READ'; export const NOTIFICATIONS_MARK_AS_READ = 'NOTIFICATIONS_MARK_AS_READ';
export const NOTIFICATIONS_SET_BROWSER_SUPPORT = 'NOTIFICATIONS_SET_BROWSER_SUPPORT'; export const NOTIFICATIONS_SET_BROWSER_SUPPORT = 'NOTIFICATIONS_SET_BROWSER_SUPPORT';
@ -124,7 +123,17 @@ export function updateNotifications(notification, intlMessages, intlLocale) {
const excludeTypesFromSettings = state => state.getIn(['settings', 'notifications', 'shows']).filter(enabled => !enabled).keySeq().toJS(); const excludeTypesFromSettings = state => state.getIn(['settings', 'notifications', 'shows']).filter(enabled => !enabled).keySeq().toJS();
const excludeTypesFromFilter = filter => { const excludeTypesFromFilter = filter => {
const allTypes = ImmutableList(['follow', 'follow_request', 'favourite', 'reblog', 'mention', 'poll']); const allTypes = ImmutableList([
'follow',
'follow_request',
'favourite',
'reblog',
'mention',
'poll',
'status',
'update',
]);
return allTypes.filterNot(item => item === filter).toJS(); return allTypes.filterNot(item => item === filter).toJS();
}; };

View File

@ -153,6 +153,17 @@ export default class ColumnSettings extends React.PureComponent {
<SettingToggle prefix='notifications' settings={settings} settingPath={['sounds', 'status']} onChange={onChange} label={soundStr} /> <SettingToggle prefix='notifications' settings={settings} settingPath={['sounds', 'status']} onChange={onChange} label={soundStr} />
</div> </div>
</div> </div>
<div role='group' aria-labelledby='notifications-update'>
<span id='notifications-status' className='column-settings__section'><FormattedMessage id='notifications.column_settings.update' defaultMessage='Edits:' /></span>
<div className='column-settings__row'>
<SettingToggle disabled={browserPermission === 'denied'} prefix='notifications_desktop' settings={settings} settingPath={['alerts', 'update']} onChange={onChange} label={alertStr} />
{showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingPath={['alerts', 'update']} onChange={this.onPushChange} label={pushStr} />}
<SettingToggle prefix='notifications' settings={settings} settingPath={['shows', 'update']} onChange={onChange} label={showStr} />
<SettingToggle prefix='notifications' settings={settings} settingPath={['sounds', 'update']} onChange={onChange} label={soundStr} />
</div>
</div>
</div> </div>
); );
} }

View File

@ -19,6 +19,7 @@ const messages = defineMessages({
poll: { id: 'notification.poll', defaultMessage: 'A poll you have voted in has ended' }, poll: { id: 'notification.poll', defaultMessage: 'A poll you have voted in has ended' },
reblog: { id: 'notification.reblog', defaultMessage: '{name} boosted your status' }, reblog: { id: 'notification.reblog', defaultMessage: '{name} boosted your status' },
status: { id: 'notification.status', defaultMessage: '{name} just posted' }, status: { id: 'notification.status', defaultMessage: '{name} just posted' },
update: { id: 'notification.update', defaultMessage: '{name} edited a post' },
}); });
const notificationForScreenReader = (intl, message, timestamp) => { const notificationForScreenReader = (intl, message, timestamp) => {
@ -273,6 +274,38 @@ class Notification extends ImmutablePureComponent {
); );
} }
renderUpdate (notification, link) {
const { intl, unread } = this.props;
return (
<HotKeys handlers={this.getHandlers()}>
<div className={classNames('notification notification-update focusable', { unread })} tabIndex='0' aria-label={notificationForScreenReader(intl, intl.formatMessage(messages.update, { name: notification.getIn(['account', 'acct']) }), notification.get('created_at'))}>
<div className='notification__message'>
<div className='notification__favourite-icon-wrapper'>
<Icon id='pencil' fixedWidth />
</div>
<span title={notification.get('created_at')}>
<FormattedMessage id='notification.update' defaultMessage='{name} edited a post' values={{ name: link }} />
</span>
</div>
<StatusContainer
id={notification.get('status')}
account={notification.get('account')}
muted
withDismiss
hidden={this.props.hidden}
getScrollPosition={this.props.getScrollPosition}
updateScrollBottom={this.props.updateScrollBottom}
cachedMediaWidth={this.props.cachedMediaWidth}
cacheMediaWidth={this.props.cacheMediaWidth}
/>
</div>
</HotKeys>
);
}
renderPoll (notification, account) { renderPoll (notification, account) {
const { intl, unread } = this.props; const { intl, unread } = this.props;
const ownPoll = me === account.get('id'); const ownPoll = me === account.get('id');
@ -330,6 +363,8 @@ class Notification extends ImmutablePureComponent {
return this.renderReblog(notification, link); return this.renderReblog(notification, link);
case 'status': case 'status':
return this.renderStatus(notification, link); return this.renderStatus(notification, link);
case 'update':
return this.renderUpdate(notification, link);
case 'poll': case 'poll':
return this.renderPoll(notification, account); return this.renderPoll(notification, account);
} }

View File

@ -36,6 +36,7 @@ const initialState = ImmutableMap({
mention: false, mention: false,
poll: false, poll: false,
status: false, status: false,
update: false,
}), }),
quickFilter: ImmutableMap({ quickFilter: ImmutableMap({
@ -55,6 +56,7 @@ const initialState = ImmutableMap({
mention: true, mention: true,
poll: true, poll: true,
status: true, status: true,
update: true,
}), }),
sounds: ImmutableMap({ sounds: ImmutableMap({
@ -65,6 +67,7 @@ const initialState = ImmutableMap({
mention: true, mention: true,
poll: true, poll: true,
status: true, status: true,
update: true,
}), }),
}), }),

View File

@ -20,6 +20,8 @@ filenames.forEach(filename => {
'notification.mention': full['notification.mention'] || '', 'notification.mention': full['notification.mention'] || '',
'notification.reblog': full['notification.reblog'] || '', 'notification.reblog': full['notification.reblog'] || '',
'notification.poll': full['notification.poll'] || '', 'notification.poll': full['notification.poll'] || '',
'notification.status': full['notification.status'] || '',
'notification.update': full['notification.update'] || '',
'status.show_more': full['status.show_more'] || '', 'status.show_more': full['status.show_more'] || '',
'status.reblog': full['status.reblog'] || '', 'status.reblog': full['status.reblog'] || '',

View File

@ -102,7 +102,7 @@ const handlePush = (event) => {
options.image = undefined; options.image = undefined;
options.actions = [actionExpand(preferred_locale)]; options.actions = [actionExpand(preferred_locale)];
} else if (notification.type === 'mention') { } else if (['mention', 'status'].includes(notification.type)) {
options.actions = [actionReblog(preferred_locale), actionFavourite(preferred_locale)]; options.actions = [actionReblog(preferred_locale), actionFavourite(preferred_locale)];
} }

View File

@ -35,6 +35,7 @@ class Notification < ApplicationRecord
follow_request follow_request
favourite favourite
poll poll
update
).freeze ).freeze
TARGET_STATUS_INCLUDES_BY_TYPE = { TARGET_STATUS_INCLUDES_BY_TYPE = {
@ -43,6 +44,7 @@ class Notification < ApplicationRecord
mention: [mention: :status], mention: [mention: :status],
favourite: [favourite: :status], favourite: [favourite: :status],
poll: [poll: :status], poll: [poll: :status],
update: :status,
}.freeze }.freeze
belongs_to :account, optional: true belongs_to :account, optional: true
@ -76,7 +78,7 @@ class Notification < ApplicationRecord
def target_status def target_status
case type case type
when :status when :status, :update
status status
when :reblog when :reblog
status&.reblog status&.reblog
@ -110,7 +112,7 @@ class Notification < ApplicationRecord
cached_status = cached_statuses_by_id[notification.target_status.id] cached_status = cached_statuses_by_id[notification.target_status.id]
case notification.type case notification.type
when :status when :status, :update
notification.status = cached_status notification.status = cached_status
when :reblog when :reblog
notification.status.reblog = cached_status notification.status.reblog = cached_status

View File

@ -62,6 +62,7 @@ class Status < ApplicationRecord
has_many :favourites, inverse_of: :status, dependent: :destroy has_many :favourites, inverse_of: :status, dependent: :destroy
has_many :bookmarks, inverse_of: :status, dependent: :destroy has_many :bookmarks, inverse_of: :status, dependent: :destroy
has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy
has_many :reblogged_by_accounts, through: :reblogs, class_name: 'Account', source: :account
has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread
has_many :mentions, dependent: :destroy, inverse_of: :status has_many :mentions, dependent: :destroy, inverse_of: :status
has_many :active_mentions, -> { active }, class_name: 'Mention', inverse_of: :status has_many :active_mentions, -> { active }, class_name: 'Mention', inverse_of: :status

View File

@ -11,6 +11,6 @@ class REST::NotificationSerializer < ActiveModel::Serializer
end end
def status_type? def status_type?
[:favourite, :reblog, :status, :mention, :poll].include?(object.type) [:favourite, :reblog, :status, :mention, :poll, :update].include?(object.type)
end end
end end

View File

@ -34,6 +34,7 @@ class FanOutOnWriteService < BaseService
def fan_out_to_local_recipients! def fan_out_to_local_recipients!
deliver_to_self! deliver_to_self!
notify_mentioned_accounts! notify_mentioned_accounts!
notify_about_update! if update?
case @status.visibility.to_sym case @status.visibility.to_sym
when :public, :unlisted, :private when :public, :unlisted, :private
@ -64,6 +65,14 @@ class FanOutOnWriteService < BaseService
end end
end end
def notify_about_update!
@status.reblogged_by_accounts.merge(Account.local).select(:id).reorder(nil).find_in_batches do |accounts|
LocalNotificationWorker.push_bulk(accounts) do |account|
[account.id, @status.id, 'Status', 'update']
end
end
end
def deliver_to_all_followers! def deliver_to_all_followers!
@account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers| @account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers|
FeedInsertWorker.push_bulk(followers) do |follower| FeedInsertWorker.push_bulk(followers) do |follower|

View File

@ -46,6 +46,10 @@ class NotifyService < BaseService
false false
end end
def blocked_update?
false
end
def following_sender? def following_sender?
return @following_sender if defined?(@following_sender) return @following_sender if defined?(@following_sender)
@following_sender = @recipient.following?(@notification.from_account) || @recipient.requested?(@notification.from_account) @following_sender = @recipient.following?(@notification.from_account) || @recipient.requested?(@notification.from_account)

View File

@ -12,7 +12,14 @@ class LocalNotificationWorker
activity = activity_class_name.constantize.find(activity_id) activity = activity_class_name.constantize.find(activity_id)
end end
return if Notification.where(account: receiver, activity: activity).any? # For most notification types, only one notification should exist, and the older one is
# preferred. For updates, such as when a status is edited, the new notification
# should replace the previous ones.
if type == 'update'
Notification.where(account: receiver, activity: activity, type: 'update').in_batches.delete_all
elsif Notification.where(account: receiver, activity: activity, type: type).any?
return
end
NotifyService.new.call(receiver, type || activity_class_name.underscore, activity) NotifyService.new.call(receiver, type || activity_class_name.underscore, activity)
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound

View File

@ -1148,6 +1148,8 @@ en:
title: New boost title: New boost
status: status:
subject: "%{name} just posted" subject: "%{name} just posted"
update:
subject: "%{name} edited a post"
notifications: notifications:
email_events: Events for e-mail notifications email_events: Events for e-mail notifications
email_events_hint: 'Select events that you want to receive notifications for:' email_events_hint: 'Select events that you want to receive notifications for:'