diff options
author | Eugen Rochko <eugen@zeonfederated.com> | 2020-09-18 17:26:45 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-09-18 17:26:45 +0200 |
commit | 974b1b79ce58e6799e5e5bb576e630ca783150de (patch) | |
tree | 93dfcb52fc58d714b3a9bd454f7589fe98c1d1ae | |
parent | 75e4bd9413143ee208d00814c728fc2bf0c58cf2 (diff) |
Add option to be notified when a followed user posts (#13546)
* Add bell button Fix #4890 * Remove duplicate type from post-deployment migration * Fix legacy class type mappings * Improve query performance with better index * Fix validation * Remove redundant index from notifications
42 files changed, 324 insertions, 106 deletions
diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 61dcb87c2..aef51a647 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -30,9 +30,8 @@ class Api::V1::AccountsController < Api::BaseController end def follow - FollowService.new.call(current_user.account, @account, reblogs: truthy_param?(:reblogs), with_rate_limit: true) - - options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: truthy_param?(:reblogs) } }, requested_map: { @account.id => false } } + follow = FollowService.new.call(current_user.account, @account, reblogs: params.key?(:reblogs) ? truthy_param?(:reblogs) : nil, notify: params.key?(:notify) ? truthy_param?(:notify) : nil, with_rate_limit: true) + options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: follow.show_reblogs?, notify: follow.notify? } }, requested_map: { @account.id => false } } render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) end diff --git a/app/controllers/api/v1/follow_requests_controller.rb b/app/controllers/api/v1/follow_requests_controller.rb index 0420b7bef..b34c76f29 100644 --- a/app/controllers/api/v1/follow_requests_controller.rb +++ b/app/controllers/api/v1/follow_requests_controller.rb @@ -13,7 +13,7 @@ class Api::V1::FollowRequestsController < Api::BaseController def authorize AuthorizeFollowService.new.call(account, current_account) - NotifyService.new.call(current_account, Follow.find_by(account: account, target_account: current_account)) + NotifyService.new.call(current_account, :follow, Follow.find_by(account: account, target_account: current_account)) render json: account, serializer: REST::RelationshipSerializer, relationships: relationships end diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js index d28f7dad8..723c04e55 100644 --- a/app/javascript/mastodon/actions/accounts.js +++ b/app/javascript/mastodon/actions/accounts.js @@ -109,14 +109,14 @@ export function fetchAccountFail(id, error) { }; }; -export function followAccount(id, reblogs = true) { +export function followAccount(id, options = { reblogs: true }) { return (dispatch, getState) => { const alreadyFollowing = getState().getIn(['relationships', id, 'following']); const locked = getState().getIn(['accounts', id, 'locked'], false); dispatch(followAccountRequest(id, locked)); - api(getState).post(`/api/v1/accounts/${id}/follow`, { reblogs }).then(response => { + api(getState).post(`/api/v1/accounts/${id}/follow`, options).then(response => { dispatch(followAccountSuccess(response.data, alreadyFollowing)); }).catch(error => { dispatch(followAccountFail(error, locked)); diff --git a/app/javascript/mastodon/actions/notifications.js b/app/javascript/mastodon/actions/notifications.js index a26844f84..099e42f6c 100644 --- a/app/javascript/mastodon/actions/notifications.js +++ b/app/javascript/mastodon/actions/notifications.js @@ -59,7 +59,7 @@ export function updateNotifications(notification, intlMessages, intlLocale) { let filtered = false; - if (notification.type === 'mention') { + if (['mention', 'status'].includes(notification.type)) { const dropRegex = filters[0]; const regex = filters[1]; const searchIndex = searchTextFromRawStatus(notification.status); diff --git a/app/javascript/mastodon/features/account/components/header.js b/app/javascript/mastodon/features/account/components/header.js index 02217b62c..2b97af4e6 100644 --- a/app/javascript/mastodon/features/account/components/header.js +++ b/app/javascript/mastodon/features/account/components/header.js @@ -7,6 +7,7 @@ import ImmutablePureComponent from 'react-immutable-pure-component'; import { autoPlayGif, me, isStaff } from 'mastodon/initial_state'; import classNames from 'classnames'; import Icon from 'mastodon/components/icon'; +import IconButton from 'mastodon/components/icon_button'; import Avatar from 'mastodon/components/avatar'; import { counterRenderer } from 'mastodon/components/common_counter'; import ShortNumber from 'mastodon/components/short_number'; @@ -35,6 +36,8 @@ const messages = defineMessages({ unblockDomain: { id: 'account.unblock_domain', defaultMessage: 'Unblock domain {domain}' }, hideReblogs: { id: 'account.hide_reblogs', defaultMessage: 'Hide boosts from @{name}' }, showReblogs: { id: 'account.show_reblogs', defaultMessage: 'Show boosts from @{name}' }, + enableNotifications: { id: 'account.enable_notifications', defaultMessage: 'Notify me when @{name} posts' }, + disableNotifications: { id: 'account.disable_notifications', defaultMessage: 'Stop notifying me when @{name} posts' }, pins: { id: 'navigation_bar.pins', defaultMessage: 'Pinned toots' }, preferences: { id: 'navigation_bar.preferences', defaultMessage: 'Preferences' }, follow_requests: { id: 'navigation_bar.follow_requests', defaultMessage: 'Follow requests' }, @@ -68,8 +71,9 @@ class Header extends ImmutablePureComponent { onBlock: PropTypes.func.isRequired, onMention: PropTypes.func.isRequired, onDirect: PropTypes.func.isRequired, - onReport: PropTypes.func.isRequired, onReblogToggle: PropTypes.func.isRequired, + onNotifyToggle: PropTypes.func.isRequired, + onReport: PropTypes.func.isRequired, onMute: PropTypes.func.isRequired, onBlockDomain: PropTypes.func.isRequired, onUnblockDomain: PropTypes.func.isRequired, @@ -144,6 +148,7 @@ class Header extends ImmutablePureComponent { let info = []; let actionBtn = ''; + let bellBtn = ''; let lockedIcon = ''; let menu = []; @@ -173,6 +178,10 @@ class Header extends ImmutablePureComponent { actionBtn = <Button className='logo-button' text={intl.formatMessage(messages.edit_profile)} onClick={this.openEditProfile} />; } + if (account.getIn(['relationship', 'requested']) || account.getIn(['relationship', 'following'])) { + bellBtn = <IconButton icon='bell-o' size={24} active={account.getIn(['relationship', 'notifying'])} title={intl.formatMessage(account.getIn(['relationship', 'notifying']) ? messages.disableNotifications : messages.enableNotifications, { name: account.get('username') })} onClick={this.props.onNotifyToggle} />; + } + if (account.get('moved') && !account.getIn(['relationship', 'following'])) { actionBtn = ''; } @@ -287,6 +296,7 @@ class Header extends ImmutablePureComponent { {!suspended && ( <div className='account__header__tabs__buttons'> {actionBtn} + {bellBtn} <DropdownMenuContainer items={menu} icon='ellipsis-v' size={24} direction='right' /> </div> diff --git a/app/javascript/mastodon/features/account_timeline/components/header.js b/app/javascript/mastodon/features/account_timeline/components/header.js index abb15edcc..6b52defe4 100644 --- a/app/javascript/mastodon/features/account_timeline/components/header.js +++ b/app/javascript/mastodon/features/account_timeline/components/header.js @@ -55,6 +55,10 @@ export default class Header extends ImmutablePureComponent { this.props.onReblogToggle(this.props.account); } + handleNotifyToggle = () => { + this.props.onNotifyToggle(this.props.account); + } + handleMute = () => { this.props.onMute(this.props.account); } @@ -106,6 +110,7 @@ export default class Header extends ImmutablePureComponent { onMention={this.handleMention} onDirect={this.handleDirect} onReblogToggle={this.handleReblogToggle} + onNotifyToggle={this.handleNotifyToggle} onReport={this.handleReport} onMute={this.handleMute} onBlockDomain={this.handleBlockDomain} diff --git a/app/javascript/mastodon/features/account_timeline/containers/header_container.js b/app/javascript/mastodon/features/account_timeline/containers/header_container.js index 8728b4806..e12019547 100644 --- a/app/javascript/mastodon/features/account_timeline/containers/header_container.js +++ b/app/javascript/mastodon/features/account_timeline/containers/header_container.js @@ -76,9 +76,9 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ onReblogToggle (account) { if (account.getIn(['relationship', 'showing_reblogs'])) { - dispatch(followAccount(account.get('id'), false)); + dispatch(followAccount(account.get('id'), { reblogs: false })); } else { - dispatch(followAccount(account.get('id'), true)); + dispatch(followAccount(account.get('id'), { reblogs: true })); } }, @@ -90,6 +90,14 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ } }, + onNotifyToggle (account) { + if (account.getIn(['relationship', 'notifying'])) { + dispatch(followAccount(account.get('id'), { notify: false })); + } else { + dispatch(followAccount(account.get('id'), { notify: true })); + } + }, + onReport (account) { dispatch(initReport(account)); }, diff --git a/app/javascript/mastodon/features/notifications/components/filter_bar.js b/app/javascript/mastodon/features/notifications/components/filter_bar.js index 2fd28d832..368eb0b7e 100644 --- a/app/javascript/mastodon/features/notifications/components/filter_bar.js +++ b/app/javascript/mastodon/features/notifications/components/filter_bar.js @@ -9,6 +9,7 @@ const tooltips = defineMessages({ boosts: { id: 'notifications.filter.boosts', defaultMessage: 'Boosts' }, polls: { id: 'notifications.filter.polls', defaultMessage: 'Poll results' }, follows: { id: 'notifications.filter.follows', defaultMessage: 'Follows' }, + statuses: { id: 'notifications.filter.statuses', defaultMessage: 'Updates from people you follow' }, }); export default @injectIntl @@ -88,6 +89,13 @@ class FilterBar extends React.PureComponent { <Icon id='tasks' fixedWidth /> </button> <button + className={selectedFilter === 'status' ? 'active' : ''} + onClick={this.onClick('status')} + title={intl.formatMessage(tooltips.statuses)} + > + <Icon id='home' fixedWidth /> + </button> + <button className={selectedFilter === 'follow' ? 'active' : ''} onClick={this.onClick('follow')} title={intl.formatMessage(tooltips.follows)} diff --git a/app/javascript/mastodon/features/notifications/components/notification.js b/app/javascript/mastodon/features/notifications/components/notification.js index 74065e5e2..62a97f187 100644 --- a/app/javascript/mastodon/features/notifications/components/notification.js +++ b/app/javascript/mastodon/features/notifications/components/notification.js @@ -17,6 +17,7 @@ const messages = defineMessages({ ownPoll: { id: 'notification.own_poll', defaultMessage: 'Your poll has ended' }, poll: { id: 'notification.poll', defaultMessage: 'A poll you have voted in has ended' }, reblog: { id: 'notification.reblog', defaultMessage: '{name} boosted your status' }, + status: { id: 'notification.status', defaultMessage: '{name} just posted' }, }); const notificationForScreenReader = (intl, message, timestamp) => { @@ -237,6 +238,38 @@ class Notification extends ImmutablePureComponent { ); } + renderStatus (notification, link) { + const { intl } = this.props; + + return ( + <HotKeys handlers={this.getHandlers()}> + <div className='notification notification-status focusable' tabIndex='0' aria-label={notificationForScreenReader(intl, intl.formatMessage(messages.status, { name: notification.getIn(['account', 'acct']) }), notification.get('created_at'))}> + <div className='notification__message'> + <div className='notification__favourite-icon-wrapper'> + <Icon id='home' fixedWidth /> + </div> + + <span title={notification.get('created_at')}> + <FormattedMessage id='notification.status' defaultMessage='{name} just posted' 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) { const { intl } = this.props; const ownPoll = me === account.get('id'); @@ -292,6 +325,8 @@ class Notification extends ImmutablePureComponent { return this.renderFavourite(notification, link); case 'reblog': return this.renderReblog(notification, link); + case 'status': + return this.renderStatus(notification, link); case 'poll': return this.renderPoll(notification, account); } diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index cfcd937fa..7defa0d16 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -6502,6 +6502,10 @@ noscript { padding: 2px; } + & > .icon-button { + margin-right: 8px; + } + .button { margin: 0 8px; } diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 94aee7939..224451f41 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -118,13 +118,13 @@ class ActivityPub::Activity end def notify_about_reblog(status) - NotifyService.new.call(status.reblog.account, status) + NotifyService.new.call(status.reblog.account, :reblog, status) end def notify_about_mentions(status) status.active_mentions.includes(:account).each do |mention| next unless mention.account.local? && audience_includes?(mention.account) - NotifyService.new.call(mention.account, mention) + NotifyService.new.call(mention.account, :mention, mention) end end diff --git a/app/lib/activitypub/activity/follow.rb b/app/lib/activitypub/activity/follow.rb index ec92f4255..0beec68ab 100644 --- a/app/lib/activitypub/activity/follow.rb +++ b/app/lib/activitypub/activity/follow.rb @@ -22,10 +22,10 @@ class ActivityPub::Activity::Follow < ActivityPub::Activity follow_request = FollowRequest.create!(account: @account, target_account: target_account, uri: @json['id']) if target_account.locked? || @account.silenced? - NotifyService.new.call(target_account, follow_request) + NotifyService.new.call(target_account, :follow_request, follow_request) else AuthorizeFollowService.new.call(@account, target_account) - NotifyService.new.call(target_account, ::Follow.find_by(account: @account, target_account: target_account)) + NotifyService.new.call(target_account, :follow, ::Follow.find_by(account: @account, target_account: target_account)) end end diff --git a/app/lib/activitypub/activity/like.rb b/app/lib/activitypub/activity/like.rb index 674d5fe47..c065f01f8 100644 --- a/app/lib/activitypub/activity/like.rb +++ b/app/lib/activitypub/activity/like.rb @@ -7,6 +7,6 @@ class ActivityPub::Activity::Like < ActivityPub::Activity return if original_status.nil? || !original_status.account.local? || delete_arrived_first?(@json['id']) || @account.favourited?(original_status) favourite = original_status.favourites.create!(account: @account) - NotifyService.new.call(original_status.account, favourite) + NotifyService.new.call(original_status.account, :favourite, favourite) end end diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index be7211f2c..427ebdae2 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -8,6 +8,7 @@ module AccountInteractions Follow.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |follow, mapping| mapping[follow.target_account_id] = { reblogs: follow.show_reblogs?, + notify: follow.notify?, } end end @@ -36,6 +37,7 @@ module AccountInteractions FollowRequest.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |follow_request, mapping| mapping[follow_request.target_account_id] = { reblogs: follow_request.show_reblogs?, + notify: follow_request.notify?, } end end @@ -95,25 +97,29 @@ module AccountInteractions has_many :announcement_mutes, dependent: :destroy end - def follow!(other_account, reblogs: nil, uri: nil, rate_limit: false) - reblogs = true if reblogs.nil? - - rel = active_relationships.create_with(show_reblogs: reblogs, uri: uri, rate_limit: rate_limit) + def follow!(other_account, reblogs: nil, notify: nil, uri: nil, rate_limit: false) + rel = active_relationships.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, uri: uri, rate_limit: rate_limit) .find_or_create_by!(target_account: other_account) - rel.update!(show_reblogs: reblogs) + rel.show_reblogs = reblogs unless reblogs.nil? + rel.notify = notify unless notify.nil? + + rel.save! if rel.changed? + remove_potential_friendship(other_account) rel end - def request_follow!(other_account, reblogs: nil, uri: nil, rate_limit: false) - reblogs = true if reblogs.nil? - - rel = follow_requests.create_with(show_reblogs: reblogs, uri: uri, rate_limit: rate_limit) + def request_follow!(other_account, reblogs: nil, notify: nil, uri: nil, rate_limit: false) + rel = follow_requests.create_with(show_reblogs: reblogs.nil? ? true : reblogs, notify: notify.nil? ? false : notify, uri: uri, rate_limit: rate_limit) .find_or_create_by!(target_account: other_account) - rel.update!(show_reblogs: reblogs) + rel.show_reblogs = reblogs unless reblogs.nil? + rel.notify = notify unless notify.nil? + + rel.save! if rel.changed? + remove_potential_friendship(other_account) rel diff --git a/app/models/follow.rb b/app/models/follow.rb index f3e48a2ed..0b4ddbf3f 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -10,6 +10,7 @@ # target_account_id :bigint(8) not null # show_reblogs :boolean default(TRUE), not null # uri :string +# notify :boolean default(FALSE), not null # class Follow < ApplicationRecord @@ -34,7 +35,7 @@ class Follow < ApplicationRecord end def revoke_request! - FollowRequest.create!(account: account, target_account: target_account, show_reblogs: show_reblogs, uri: uri) + FollowRequest.create!(account: account, target_account: target_account, show_reblogs: show_reblogs, notify: notify, uri: uri) destroy! end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 3325e264c..c1f19149b 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -10,6 +10,7 @@ # target_account_id :bigint(8) not null # show_reblogs :boolean default(TRUE), not null # uri :string +# notify :boolean default(FALSE), not null # class FollowRequest < ApplicationRecord @@ -28,7 +29,7 @@ class FollowRequest < ApplicationRecord validates_with FollowLimitValidator, on: :create def authorize! - account.follow!(target_account, reblogs: show_reblogs, uri: uri) + account.follow!(target_account, reblogs: show_reblogs, notify: notify, uri: uri) MergeWorker.perform_async(target_account.id, account.id) if account.local? destroy! end diff --git a/app/models/notification.rb b/app/models/notification.rb index 4d7a392b1..e83123c97 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -10,21 +10,34 @@ # updated_at :datetime not null # account_id :bigint(8) not null # from_account_id :bigint(8) not null +# type :string # class Notification < ApplicationRecord + self.inheritance_column = nil + include Paginable include Cacheable - TYPE_CLASS_MAP = { - mention: 'Mention', - reblog: 'Status', - follow: 'Follow', - follow_request: 'FollowRequest', - favourite: 'Favourite', - poll: 'Poll', + LEGACY_TYPE_CLASS_MAP = { + 'Mention' => :mention, + 'Status' => :reblog, + 'Follow' => :follow, + 'FollowRequest' => :follow_request, + 'Favourite' => :favourite, + 'Poll' => :poll, }.freeze + TYPES = %i( + mention + status + reblog + follow + follow_request + favourite + poll + ).freeze + STATUS_INCLUDES = [:account, :application, :preloadable_poll, :media_attachments, :tags, active_mentions: :account, reblog: [:account, :application, :preloadable_poll, :media_attachments, :tags, active_mentions: :account]].freeze belongs_to :account, optional: true @@ -38,29 +51,30 @@ class Notification < ApplicationRecord belongs_to :favourite, foreign_type: 'Favourite', foreign_key: 'activity_id', optional: true belongs_to :poll, foreign_type: 'Poll', foreign_key: 'activity_id', optional: true - validates :account_id, uniqueness: { scope: [:activity_type, :activity_id] } - validates :activity_type, inclusion: { in: TYPE_CLASS_MAP.values } + validates :type, inclusion: { in: TYPES } scope :without_suspended, -> { joins(:from_account).merge(Account.without_suspended) } scope :browserable, ->(exclude_types = [], account_id = nil) { - types = TYPE_CLASS_MAP.values - activity_types_from_types(exclude_types) + types = TYPES - exclude_types.map(&:to_sym) if account_id.nil? - where(activity_type: types) + where(type: types) else - where(activity_type: types, from_account_id: account_id) + where(type: types, from_account_id: account_id) end } cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account, follow_request: :account, poll: [status: STATUS_INCLUDES] def type - @type ||= TYPE_CLASS_MAP.invert[activity_type].to_sym + @type ||= (super || LEGACY_TYPE_CLASS_MAP[activity_type]).to_sym end def target_status case type + when :status + status when :reblog status&.reblog when :favourite @@ -89,10 +103,6 @@ class Notification < ApplicationRecord item.target_status.account = accounts[item.target_status.account_id] if item.target_status end end - - def activity_types_from_types(types) - types.map { |type| TYPE_CLASS_MAP[type.to_sym] }.compact - end end after_initialize :set_from_account diff --git a/app/serializers/rest/notification_serializer.rb b/app/serializers/rest/notification_serializer.rb index 80812ad0d..27b031fcc 100644 --- a/app/serializers/rest/notification_serializer.rb +++ b/app/serializers/rest/notification_serializer.rb @@ -11,6 +11,6 @@ class REST::NotificationSerializer < ActiveModel::Serializer end def status_type? - [:favourite, :reblog, :mention, :poll].include?(object.type) + [:favourite, :reblog, :status, :mention, :poll].include?(object.type) end end diff --git a/app/serializers/rest/relationship_serializer.rb b/app/serializers/rest/relationship_serializer.rb index c2f3c9a11..afd4cddf9 100644 --- a/app/serializers/rest/relationship_serializer.rb +++ b/app/serializers/rest/relationship_serializer.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true class REST::RelationshipSerializer < ActiveModel::Serializer - attributes :id, :following, :showing_reblogs, :followed_by, :blocking, :blocked_by, - :muting, :muting_notifications, :requested, :domain_blocking, - :endorsed, :note + attributes :id, :following, :showing_reblogs, :notifying, :followed_by, + :blocking, :blocked_by, :muting, :muting_notifications, :requested, + :domain_blocking, :endorsed, :note def id object.id.to_s @@ -19,6 +19,12 @@ class REST::RelationshipSerializer < ActiveModel::Serializer false end + def notifying + (instance_options[:relationships].following[object.id] || {})[:notify] || + (instance_options[:relationships].requested[object.id] || {})[:notify] || + false + end + def followed_by instance_options[:relationships].followed_by[object.id] || false end diff --git a/app/services/favourite_service.rb b/app/services/favourite_service.rb index 02b26458a..a0ab3b4b7 100644 --- a/app/services/favourite_service.rb +++ b/app/services/favourite_service.rb @@ -29,7 +29,7 @@ class FavouriteService < BaseService status = favourite.status if status.account.local? - NotifyService.new.call(status.account, favourite) + NotifyService.new.call(status.account, :favourite, favourite) elsif status.account.activitypub? ActivityPub::DeliveryWorker.perform_async(build_json(favourite), favourite.account_id, status.account.inbox_url) end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 311ae7fa6..962572851 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -9,12 +9,13 @@ class FollowService < BaseService # @param [String, Account] uri User URI to follow in the form of username@domain (or account record) # @param [Hash] options # @option [Boolean] :reblogs Whether or not to show reblogs, defaults to true + # @option [Boolean] :notify Whether to create notifications about new posts, defaults to false # @option [Boolean] :bypass_locked # @option [Boolean] :with_rate_limit def call(source_account, target_account, options = {}) @source_account = source_account @target_account = ResolveAccountService.new.call(target_account, skip_webfinger: true) - @options = { reblogs: true, bypass_locked: false, with_rate_limit: false }.merge(options) + @options = { bypass_locked: false, with_rate_limit: false }.merge(options) raise ActiveRecord::RecordNotFound if following_not_possible? raise Mastodon::NotPermittedError if following_not_allowed? @@ -45,18 +46,18 @@ class FollowService < BaseService end def change_follow_options! - @source_account.follow!(@target_account, reblogs: @options[:reblogs]) + @source_account.follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify]) end def change_follow_request_options! - @source_account.request_follow!(@target_account, reblogs: @options[:reblogs]) + @source_account.request_follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify]) end def request_follow! - follow_request = @source_account.request_follow!(@target_account, reblogs: @options[:reblogs], rate_limit: @options[:with_rate_limit]) + follow_request = @source_account.request_follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit]) if @target_account.local? - LocalNotificationWorker.perform_async(@target_account.id, follow_request.id, follow_request.class.name) + LocalNotificationWorker.perform_async(@target_account.id, follow_request.id, follow_request.class.name, :follow_request) elsif @target_account.activitypub? ActivityPub::DeliveryWorker.perform_async(build_json(follow_request), @source_account.id, @target_account.inbox_url) end @@ -65,9 +66,9 @@ class FollowService < BaseService end def direct_follow! - follow = @source_account.follow!(@target_account, reblogs: @options[:reblogs], rate_limit: @options[:with_rate_limit]) + follow = @source_account.follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit]) - LocalNotificationWorker.perform_async(@target_account.id, follow.id, follow.class.name) + LocalNotificationWorker.perform_async(@target_account.id, follow.id, follow.class.name, :follow) MergeWorker.perform_async(@target_account.id, @source_account.id) follow diff --git a/app/services/import_service.rb b/app/services/import_service.rb index 4cad93767..7e55452de 100644 --- a/app/services/import_service.rb +++ b/app/services/import_service.rb @@ -25,7 +25,7 @@ class ImportService < BaseService def import_follows! parse_import_data!(['Account address']) - import_relationships!('follow', 'unfollow', @account.following, follow_limit, reblogs: 'Show boosts') + import_relationships!('follow', 'unfollow', @account.following, follow_limit, reblogs: { header: 'Show boosts', default: true }) end def import_blocks! @@ -35,7 +35,7 @@ class ImportService < BaseService def import_mutes! parse_import_data!(['Account address']) - import_relationships!('mute', 'unmute', @account.muting, ROWS_PROCESSING_LIMIT, notifications: 'Hide notifications') + import_relationships!('mute', 'unmute', @account.muting, ROWS_PROCESSING_LIMIT, notifications: { header: 'Hide notifications', default: true }) end def import_domain_blocks! @@ -65,7 +65,7 @@ class ImportService < BaseService def import_relationships!(action, undo_action, overwrite_scope, limit, extra_fields = {}) local_domain_suffix = "@#{Rails.configuration.x.local_domain}" - items = @data.take(limit).map { |row| [row['Account address']&.strip&.delete_suffix(local_domain_suffix), Hash[extra_fields.map { |key, header| [key, row[header]&.strip] }]] }.reject { |(id, _)| id.blank? } + items = @data.take(limit).map { |row| [row['Account address']&.strip&.delete_suffix(local_domain_suffix), Hash[extra_fields.map { |key, field_settings| [key, row[field_settings[:header]]&.strip || field_settings[:default]] }]] }.reject { |(id, _)| id.blank? } if @import.overwrite? presence_hash = items.each_with_object({}) { |(id, extra), mapping| mapping[id] = [true, extra] } diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index e4ca10eb1..fc187db40 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true class NotifyService < BaseService - def call(recipient, activity) + def call(recipient, type, activity) @recipient = recipient @activity = activity - @notification = Notification.new(account: @recipient, activity: @activity) + @notification = Notification.new(account: @recipient, type: type, activity: @activity) return if recipient.user.nil? || blocked? @@ -22,6 +22,10 @@ class NotifyService < BaseService FeedManager.instance.filter?(:mentions, @notification.mention.status, @recipient) end + def blocked_status? + false + end + def blocked_favourite? false end diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 79af3fc54..12f0f1b08 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -58,7 +58,7 @@ class ProcessMentionsService < BaseService mentioned_account = mention.account if mentioned_account.local? - LocalNotificationWorker.perform_async(mentioned_account.id, mention.id, mention.class.name) + LocalNotificationWorker.perform_async(mentioned_account.id, mention.id, mention.class.name, :mention) elsif mentioned_account.activitypub? ActivityPub::DeliveryWorker.perform_async(activitypub_json, mention.status.account_id, mentioned_account.inbox_url) end diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 6866d2fac..5032397b3 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -45,7 +45,7 @@ class ReblogService < BaseService reblogged_status = reblog.reblog if reblogged_status.account.local? - LocalNotificationWorker.perform_async(reblogged_status.account_id, reblog.id, reblog.class.name) + LocalNotificationWorker.perform_async(reblogged_status.account_id, reblog.id, reblog.class.name, :reblog) elsif reblogged_status.account.activitypub? && !reblogged_status.account.following?(reblog.account) ActivityPub::DeliveryWorker.perform_async(build_json(reblog), reblog.account_id, reblogged_status.account.inbox_url) end diff --git a/app/workers/feed_insert_worker.rb b/app/workers/feed_insert_worker.rb index 633ec91bd..b70c7e389 100644 --- a/app/workers/feed_insert_worker.rb +++ b/app/workers/feed_insert_worker.rb @@ -23,7 +23,10 @@ class FeedInsertWorker private def check_and_insert - perform_push unless feed_filtered? + return if feed_filtered? + + perform_push + perform_notify if notify? end def feed_filtered? @@ -35,6 +38,12 @@ class FeedInsertWorker end end + def notify? + return false if @type != :home || @status.reblog? || (@status.reply? && @status.in_reply_to_account_id != @status.account_id) + + Follow.find_by(account: @follower, target_account: @status.account)&.notify? + end + def perform_push case @type when :home @@ -43,4 +52,8 @@ class FeedInsertWorker FeedManager.instance.push_to_list(@list, @status) end end + + def perform_notify + NotifyService.new.call(@follower, :status, @status) + end end diff --git a/app/workers/local_notification_worker.rb b/app/workers/local_notification_worker.rb index 48635e498..6b08ca6fc 100644 --- a/app/workers/local_notification_worker.rb +++ b/app/workers/local_notification_worker.rb @@ -3,7 +3,7 @@ class LocalNotificationWorker include Sidekiq::Worker - def perform(receiver_account_id, activity_id = nil, activity_class_name = nil) + def perform(receiver_account_id, activity_id = nil, activity_class_name = nil, type = nil) if activity_id.nil? && activity_class_name.nil? activity = Mention.find(receiver_account_id) receiver = activity.account @@ -12,7 +12,7 @@ class LocalNotificationWorker activity = activity_class_name.constantize.find(activity_id) end - NotifyService.new.call(receiver, activity) + NotifyService.new.call(receiver, type || activity_class_name.underscore, activity) rescue ActiveRecord::RecordNotFound true end diff --git a/app/workers/poll_expiration_notify_worker.rb b/app/workers/poll_expiration_notify_worker.rb index 64b4cbd7e..8a12fc075 100644 --- a/app/workers/poll_expiration_notify_worker.rb +++ b/app/workers/poll_expiration_notify_worker.rb @@ -11,12 +11,12 @@ class PollExpirationNotifyWorker # Notify poll owner and remote voters if poll.local? ActivityPub::DistributePollUpdateWorker.perform_async(poll.status.id) - NotifyService.new.call(poll.account, poll) + NotifyService.new.call(poll.account, :poll, poll) end # Notify local voters poll.votes.includes(:account).map(&:account).select(&:local?).each do |account| - NotifyService.new.call(account, poll) + NotifyService.new.call(account, :poll, poll) end rescue ActiveRecord::RecordNotFound true diff --git a/app/workers/refollow_worker.rb b/app/workers/refollow_worker.rb index 9b07ce1b5..98940680d 100644 --- a/app/workers/refollow_worker.rb +++ b/app/workers/refollow_worker.rb @@ -11,6 +11,7 @@ class RefollowWorker target_account.passive_relationships.where(account: Account.where(domain: nil)).includes(:account).reorder(nil).find_each do |follow| reblogs = follow.show_reblogs? + notify = follow.notify? # Locally unfollow remote account follower = follow.account @@ -18,7 +19,7 @@ class RefollowWorker # Schedule re-follow begin - FollowService.new.call(follower, target_account, reblogs: reblogs) + FollowService.new.call(follower, target_account, reblogs: reblogs, notify: notify) rescue Mastodon::NotPermittedError, ActiveRecord::RecordNotFound, Mastodon::UnexpectedResponseError, HTTP::Error, OpenSSL::SSL::SSLError next end diff --git a/app/workers/unfollow_follow_worker.rb b/app/workers/unfollow_follow_worker.rb index b6e665a41..71b5a0e3f 100644 --- a/app/workers/unfollow_follow_worker.rb +++ b/app/workers/unfollow_follow_worker.rb @@ -10,10 +10,11 @@ class UnfollowFollowWorker old_target_account = Account.find(old_target_account_id) new_target_account = Account.find(new_target_account_id) - follow = follower_account.active_relationships.find_by(target_account: old_target_account) + follow = follower_account.active_relationships.find_by(target_account: old_target_account) reblogs = follow&.show_reblogs? + notify = follow&.notify? - FollowService.new.call(follower_account, new_target_account, reblogs: reblogs, bypass_locked: bypass_locked) + FollowService.new.call(follower_account, new_target_account, reblogs: reblogs, notify: notify, bypass_locked: bypass_locked) UnfollowService.new.call(follower_account, old_target_account, skip_unmerge: true) rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError true diff --git a/db/migrate/20200917192924_add_notify_to_follows.rb b/db/migrate/20200917192924_add_notify_to_follows.rb new file mode 100644 index 000000000..d27471c44 --- /dev/null +++ b/db/migrate/20200917192924_add_notify_to_follows.rb @@ -0,0 +1,19 @@ +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddNotifyToFollows < ActiveRecord::Migration[5.1] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def up + safety_assured do + add_column_with_default :follows, :notify, :boolean, default: false, allow_null: false + add_column_with_default :follow_requests, :notify, :boolean, default: false, allow_null: false + end + end + + def down + remove_column :follows, :notify + remove_column :follow_requests, :notify + end +end diff --git a/db/migrate/20200917193034_add_type_to_notifications.rb b/db/migrate/20200917193034_add_type_to_notifications.rb new file mode 100644 index 000000000..002be3aa0 --- /dev/null +++ b/db/migrate/20200917193034_add_type_to_notifications.rb @@ -0,0 +1,5 @@ +class AddTypeToNotifications < ActiveRecord::Migration[5.2] + def change + add_column :notifications, :type, :string + end +end diff --git a/db/migrate/20200917222316_add_index_notifications_on_type.rb b/db/migrate/20200917222316_add_index_notifications_on_type.rb new file mode 100644 index 000000000..9bd23c1d3 --- /dev/null +++ b/db/migrate/20200917222316_add_index_notifications_on_type.rb @@ -0,0 +1,7 @@ +class AddIndexNotificationsOnType < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def change + add_index :notifications, [:account_id, :id, :type], order: { id: :desc }, algorithm: :concurrently + end +end diff --git a/db/post_migrate/20200917193528_migrate_notifications_type.rb b/db/post_migrate/20200917193528_migrate_notifications_type.rb new file mode 100644 index 000000000..88e423084 --- /dev/null +++ b/db/post_migrate/20200917193528_migrate_notifications_type.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class MigrateNotificationsType < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + TYPES_TO_MIGRATE = { + 'Mention' => :mention, + 'Status' => :reblog, + 'Follow' => :follow, + 'FollowRequest' => :follow_request, + 'Favourite' => :favourite, + 'Poll' => :poll, + }.freeze + + def up + TYPES_TO_MIGRATE.each_pair do |activity_type, type| + Notification.where(activity_type: activity_type, type: nil).in_batches.update_all(type: type) + end + end + + def down; end +end diff --git a/db/post_migrate/20200917222734_remove_index_notifications_on_account_activity.rb b/db/post_migrate/20200917222734_remove_index_notifications_on_account_activity.rb new file mode 100644 index 000000000..cb7f78e53 --- /dev/null +++ b/db/post_migrate/20200917222734_remove_index_notifications_on_account_activity.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RemoveIndexNotificationsOnAccountActivity < ActiveRecord::Migration[5.2] + disable_ddl_transaction! + + def up + remove_index :notifications, name: :account_activity + remove_index :notifications, name: :index_notifications_on_account_id_and_id + end + + def down + add_index :notifications, [:account_id, :activity_id, :activity_type], unique: true, name: 'account_activity', algorithm: :concurrently + add_index :notifications, [:account_id, :id], order: { id: :desc }, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 038e39130..0029d620a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_09_08_193330) do +ActiveRecord::Schema.define(version: 2020_09_17_222734) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -411,6 +411,7 @@ ActiveRecord::Schema.define(version: 2020_09_08_193330) do t.bigint "target_account_id", null: false t.boolean "show_reblogs", default: true, null: false t.string "uri" + t.boolean "notify", default: false, null: false t.index ["account_id", "target_account_id"], name: "index_follow_requests_on_account_id_and_target_account_id", unique: true end @@ -421,6 +422,7 @@ ActiveRecord::Schema.define(version: 2020_09_08_193330) do t.bigint "target_account_id", null: false t.boolean "show_reblogs", default: true, null: false t.string "uri" + t.boolean "notify", default: false, null: false t.index ["account_id", "target_account_id"], name: "index_follows_on_account_id_and_target_account_id", unique: true t.index ["target_account_id"], name: "index_follows_on_target_account_id" end @@ -545,8 +547,8 @@ ActiveRecord::Schema.define(version: 2020_09_08_193330) do t.datetime "updated_at", null: false t.bigint "account_id", null: false t.bigint "from_account_id", null: false - t.index ["account_id", "activity_id", "activity_type"], name: "account_activity", unique: true - t.index ["account_id", "id"], name: "index_notifications_on_account_id_and_id", order: { id: :desc } + t.string "type" + t.index ["account_id", "id", "type"], name: "index_notifications_on_account_id_and_id_and_type", order: { id: :desc } t.index ["activity_id", "activity_type"], name: "index_notifications_on_activity_id_and_activity_type" t.index ["from_account_id"], name: "index_notifications_on_from_account_id" end diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 024409dab..1e656503f 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -71,50 +71,80 @@ RSpec.describe Api::V1::AccountsController, type: :controller do let(:scopes) { 'write:follows' } let(:other_account) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', locked: locked)).account } - before do - post :follow, params: { id: other_account.id } - end + context do + before do + post :follow, params: { id: other_account.id } + end - context 'with unlocked account' do - let(:locked) { false } + context 'with unlocked account' do + let(:locked) { false } - it 'returns http success' do - expect(response).to have_http_status(200) - end + it 'returns http success' do + expect(response).to have_http_status(200) + end - it 'returns JSON with following=true and requested=false' do - json = body_as_json + it 'returns JSON with following=true and requested=false' do + json = body_as_json - expect(json[:following]).to be true - expect(json[:requested]).to be false - end + expect(json[:following]).to be true + expect(json[:requested]).to be false + end + + it 'creates a following relation between user and target user' do + expect(user.account.following?(other_account)).to be true + end - it 'creates a following relation between user and target user' do - expect(user.account.following?(other_account)).to be true + it_behaves_like 'forbidden for wrong scope', 'read:accounts' end - it_behaves_like 'forbidden for wrong scope', 'read:accounts' + context 'with locked account' do + let(:locked) { true } + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + it 'returns JSON with following=false and requested=true' do + json = body_as_json + + expect(json[:following]).to be false + expect(json[:requested]).to be true + end + + it 'creates a follow request relation between user and target user' do + expect(user.account.requested?(other_account)).to be true + end + + it_behaves_like 'forbidden for wrong scope', 'read:accounts' + end end - context 'with locked account' do - let(:locked) { true } + context 'modifying follow options' do + let(:locked) { false } - it 'returns http success' do - expect(response).to have_http_status(200) + before do + user.account.follow!(other_account, reblogs: false, notify: false) end - it 'returns JSON with following=false and requested=true' do + it 'changes reblogs option' do + post :follow, params: { id: other_account.id, reblogs: true } + json = body_as_json - expect(json[:following]).to be false - expect(json[:requested]).to be true + expect(json[:following]).to be true + expect(json[:showing_reblogs]).to be true + expect(json[:notifying]).to be false end - it 'creates a follow request relation between user and target user' do - expect(user.account.requested?(other_account)).to be true - end + it 'changes notify option' do + post :follow, params: { id: other_account.id, notify: true } + + json = body_as_json - it_behaves_like 'forbidden for wrong scope', 'read:accounts' + expect(json[:following]).to be true + expect(json[:showing_reblogs]).to be false + expect(json[:notifying]).to be true + end end end diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index e8ef61f66..f0380179c 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -14,7 +14,7 @@ describe AccountInteractions do context 'account with Follow' do it 'returns { target_account_id => true }' do Fabricate(:follow, account: account, target_account: target_account) - is_expected.to eq(target_account_id => { reblogs: true }) + is_expected.to eq(target_account_id => { reblogs: true, notify: false }) end end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index 2cf28b263..cc484a5b9 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -7,7 +7,7 @@ RSpec.describe FollowRequest, type: :model do let(:target_account) { Fabricate(:account) } it 'calls Account#follow!, MergeWorker.perform_async, and #destroy!' do - expect(account).to receive(:follow!).with(target_account, reblogs: true, uri: follow_request.uri) + expect(account).to receive(:follow!).with(target_account, reblogs: true, notify: false, uri: follow_request.uri) expect(MergeWorker).to receive(:perform_async).with(target_account.id, account.id) expect(follow_request).to receive(:destroy!) follow_request.authorize! diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb index 7618e9076..b1909d4fd 100644 --- a/spec/services/import_service_spec.rb +++ b/spec/services/import_service_spec.rb @@ -95,6 +95,7 @@ RSpec.describe ImportService, type: :service do let(:import) { Import.create(account: account, type: 'following', data: csv) } it 'follows the listed accounts, including boosts' do subject.call(import) + expect(account.following.count).to eq 1 expect(account.follow_requests.count).to eq 1 expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 440018ac9..f2cb22c5e 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -2,13 +2,14 @@ require 'rails_helper' RSpec.describe NotifyService, type: :service do subject do - -> { described_class.new.call(recipient, activity) } + -> { described_class.new.call(recipient, type, activity) } end let(:user) { Fabricate(:user) } let(:recipient) { user.account } let(:sender) { Fabricate(:account, domain: 'example.com') } let(:activity) { Fabricate(:follow, account: sender, target_account: recipient) } + let(:type) { :follow } it { is_expected.to change(Notification, :count).by(1) } @@ -50,6 +51,7 @@ RSpec.describe NotifyService, type: :service do context 'for direct messages' do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct)) } + let(:type) { :mention } before do user.settings.interactions = user.settings.interactions.merge('must_be_following_dm' => enabled) @@ -93,6 +95,7 @@ RSpec.describe NotifyService, type: :service do describe 'reblogs' do let(:status) { Fabricate(:status, account: Fabricate(:account)) } let(:activity) { Fabricate(:status, account: sender, reblog: status) } + let(:type) { :reblog } it 'shows reblogs by default' do recipient.follow!(sender) @@ -114,6 +117,7 @@ RSpec.describe NotifyService, type: :service do let(:asshole) { Fabricate(:account, username: 'asshole') } let(:reply_to) { Fabricate(:status, account: asshole) } let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, thread: reply_to)) } + let(:type) { :mention } it 'does not notify when conversation is muted' do recipient.mute_conversation!(activity.status.conversation) diff --git a/spec/workers/refollow_worker_spec.rb b/spec/workers/refollow_worker_spec.rb index 29771aa59..6b4c04291 100644 --- a/spec/workers/refollow_worker_spec.rb +++ b/spec/workers/refollow_worker_spec.rb @@ -23,8 +23,8 @@ describe RefollowWorker do result = subject.perform(account.id) expect(result).to be_nil - expect(service).to have_received(:call).with(alice, account, reblogs: true) - expect(service).to have_received(:call).with(bob, account, reblogs: false) + expect(service).to have_received(:call).with(alice, account, reblogs: true, notify: false) + expect(service).to have_received(:call).with(bob, account, reblogs: false, notify: false) end end end |