From 911cc144815babf83ddf99f2daa3682021d401b8 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 1 Dec 2019 17:25:29 +0100 Subject: [PATCH] Add follow_request notification type (#12198) * Add follow_request notification type The notification type already existed in the backend but was never pushed to the front-end. This also means translation strings were also available for the backend, from the notification mailer. Unlike other notification types, these are off by default, to match what I remember of Gargron's view on the topic: that follow requests should not clutter notifications and should instead be reviewed at the user's own leisure in the dedicated column. Since follow requests have their own column, I've deemed it unnecessary to add a specific tab for them in the notification quick filter. * Show follow request link in single-column if there are pending requests, even if account isn't locked * Push follow requests from notifications to the follow_requests list * Offer to accept or reject follow request from the notification * Redesign follow request notification --- .../api/v1/push/subscriptions_controller.rb | 2 +- .../api/web/push_subscriptions_controller.rb | 3 +- .../mastodon/actions/notifications.js | 2 +- .../components/column_settings.js | 11 ++++ .../components/follow_request.js | 59 +++++++++++++++++++ .../notifications/components/notification.js | 27 ++++++++- .../containers/follow_request_container.js | 26 ++++++++ .../ui/components/follow_requests_nav_link.js | 13 ++-- .../mastodon/reducers/notifications.js | 11 +++- .../mastodon/reducers/push_notifications.js | 1 + app/javascript/mastodon/reducers/settings.js | 3 + .../mastodon/reducers/user_lists.js | 11 ++++ .../service_worker/web_push_locales.js | 1 + app/models/notification.rb | 8 +-- app/services/notify_service.rb | 2 +- spec/models/notification_spec.rb | 26 -------- 16 files changed, 158 insertions(+), 48 deletions(-) create mode 100644 app/javascript/mastodon/features/notifications/components/follow_request.js create mode 100644 app/javascript/mastodon/features/notifications/containers/follow_request_container.js diff --git a/app/controllers/api/v1/push/subscriptions_controller.rb b/app/controllers/api/v1/push/subscriptions_controller.rb index 1b658f87083..1cbc92b93d7 100644 --- a/app/controllers/api/v1/push/subscriptions_controller.rb +++ b/app/controllers/api/v1/push/subscriptions_controller.rb @@ -51,6 +51,6 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController def data_params return {} if params[:data].blank? - params.require(:data).permit(alerts: [:follow, :favourite, :reblog, :mention, :poll]) + params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll]) end end diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb index d8153e082fe..f388b17e5de 100644 --- a/app/controllers/api/web/push_subscriptions_controller.rb +++ b/app/controllers/api/web/push_subscriptions_controller.rb @@ -19,6 +19,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController data = { alerts: { follow: alerts_enabled, + follow_request: false, favourite: alerts_enabled, reblog: alerts_enabled, mention: alerts_enabled, @@ -58,6 +59,6 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController end def data_params - @data_params ||= params.require(:data).permit(alerts: [:follow, :favourite, :reblog, :mention, :poll]) + @data_params ||= params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll]) end end diff --git a/app/javascript/mastodon/actions/notifications.js b/app/javascript/mastodon/actions/notifications.js index 3a92e0224e8..798f9b37ea5 100644 --- a/app/javascript/mastodon/actions/notifications.js +++ b/app/javascript/mastodon/actions/notifications.js @@ -110,7 +110,7 @@ export function updateNotifications(notification, intlMessages, intlLocale) { const excludeTypesFromSettings = state => state.getIn(['settings', 'notifications', 'shows']).filter(enabled => !enabled).keySeq().toJS(); const excludeTypesFromFilter = filter => { - const allTypes = ImmutableList(['follow', 'favourite', 'reblog', 'mention', 'poll']); + const allTypes = ImmutableList(['follow', 'follow_request', 'favourite', 'reblog', 'mention', 'poll']); return allTypes.filterNot(item => item === filter).toJS(); }; diff --git a/app/javascript/mastodon/features/notifications/components/column_settings.js b/app/javascript/mastodon/features/notifications/components/column_settings.js index 60a86312a1e..8bd03fbdab2 100644 --- a/app/javascript/mastodon/features/notifications/components/column_settings.js +++ b/app/javascript/mastodon/features/notifications/components/column_settings.js @@ -57,6 +57,17 @@ export default class ColumnSettings extends React.PureComponent { +
+ + +
+ + {showPushSettings && } + + +
+
+
diff --git a/app/javascript/mastodon/features/notifications/components/follow_request.js b/app/javascript/mastodon/features/notifications/components/follow_request.js new file mode 100644 index 00000000000..a80cfb2fa1f --- /dev/null +++ b/app/javascript/mastodon/features/notifications/components/follow_request.js @@ -0,0 +1,59 @@ +import React, { Fragment } from 'react'; +import ImmutablePropTypes from 'react-immutable-proptypes'; +import PropTypes from 'prop-types'; +import Avatar from 'mastodon/components/avatar'; +import DisplayName from 'mastodon/components/display_name'; +import Permalink from 'mastodon/components/permalink'; +import IconButton from 'mastodon/components/icon_button'; +import { defineMessages, injectIntl } from 'react-intl'; +import ImmutablePureComponent from 'react-immutable-pure-component'; + +const messages = defineMessages({ + authorize: { id: 'follow_request.authorize', defaultMessage: 'Authorize' }, + reject: { id: 'follow_request.reject', defaultMessage: 'Reject' }, +}); + +export default @injectIntl +class FollowRequest extends ImmutablePureComponent { + + static propTypes = { + account: ImmutablePropTypes.map.isRequired, + onAuthorize: PropTypes.func.isRequired, + onReject: PropTypes.func.isRequired, + intl: PropTypes.object.isRequired, + }; + + render () { + const { intl, hidden, account, onAuthorize, onReject } = this.props; + + if (!account) { + return
; + } + + if (hidden) { + return ( + + {account.get('display_name')} + {account.get('username')} + + ); + } + + return ( +
+
+ +
+ + + +
+ + +
+
+
+ ); + } + +} diff --git a/app/javascript/mastodon/features/notifications/components/notification.js b/app/javascript/mastodon/features/notifications/components/notification.js index 2dea8afa7eb..74065e5e268 100644 --- a/app/javascript/mastodon/features/notifications/components/notification.js +++ b/app/javascript/mastodon/features/notifications/components/notification.js @@ -7,6 +7,7 @@ import ImmutablePureComponent from 'react-immutable-pure-component'; import { me } from 'mastodon/initial_state'; import StatusContainer from 'mastodon/containers/status_container'; import AccountContainer from 'mastodon/containers/account_container'; +import FollowRequestContainer from '../containers/follow_request_container'; import Icon from 'mastodon/components/icon'; import Permalink from 'mastodon/components/permalink'; @@ -127,7 +128,29 @@ class Notification extends ImmutablePureComponent {
-
+ + ); + } + + renderFollowRequest (notification, account, link) { + const { intl } = this.props; + + return ( + +
+
+
+ +
+ + + + +
+ +
); @@ -261,6 +284,8 @@ class Notification extends ImmutablePureComponent { switch(notification.get('type')) { case 'follow': return this.renderFollow(notification, account, link); + case 'follow_request': + return this.renderFollowRequest(notification, account, link); case 'mention': return this.renderMention(notification); case 'favourite': diff --git a/app/javascript/mastodon/features/notifications/containers/follow_request_container.js b/app/javascript/mastodon/features/notifications/containers/follow_request_container.js new file mode 100644 index 00000000000..f9f6c577e49 --- /dev/null +++ b/app/javascript/mastodon/features/notifications/containers/follow_request_container.js @@ -0,0 +1,26 @@ +import { connect } from 'react-redux'; +import { makeGetAccount } from 'mastodon/selectors'; +import FollowRequest from '../components/follow_request'; +import { authorizeFollowRequest, rejectFollowRequest } from 'mastodon/actions/accounts'; + +const makeMapStateToProps = () => { + const getAccount = makeGetAccount(); + + const mapStateToProps = (state, props) => ({ + account: getAccount(state, props.id), + }); + + return mapStateToProps; +}; + +const mapDispatchToProps = (dispatch, { id }) => ({ + onAuthorize () { + dispatch(authorizeFollowRequest(id)); + }, + + onReject () { + dispatch(rejectFollowRequest(id)); + }, +}); + +export default connect(makeMapStateToProps, mapDispatchToProps)(FollowRequest); diff --git a/app/javascript/mastodon/features/ui/components/follow_requests_nav_link.js b/app/javascript/mastodon/features/ui/components/follow_requests_nav_link.js index 90c953893ad..950ed7b273c 100644 --- a/app/javascript/mastodon/features/ui/components/follow_requests_nav_link.js +++ b/app/javascript/mastodon/features/ui/components/follow_requests_nav_link.js @@ -4,12 +4,10 @@ import { fetchFollowRequests } from 'mastodon/actions/accounts'; import { connect } from 'react-redux'; import { NavLink, withRouter } from 'react-router-dom'; import IconWithBadge from 'mastodon/components/icon_with_badge'; -import { me } from 'mastodon/initial_state'; import { List as ImmutableList } from 'immutable'; import { FormattedMessage } from 'react-intl'; const mapStateToProps = state => ({ - locked: state.getIn(['accounts', me, 'locked']), count: state.getIn(['user_lists', 'follow_requests', 'items'], ImmutableList()).size, }); @@ -19,22 +17,19 @@ class FollowRequestsNavLink extends React.Component { static propTypes = { dispatch: PropTypes.func.isRequired, - locked: PropTypes.bool, count: PropTypes.number.isRequired, }; componentDidMount () { - const { dispatch, locked } = this.props; + const { dispatch } = this.props; - if (locked) { - dispatch(fetchFollowRequests()); - } + dispatch(fetchFollowRequests()); } render () { - const { locked, count } = this.props; + const { count } = this.props; - if (!locked || count === 0) { + if (count === 0) { return null; } diff --git a/app/javascript/mastodon/reducers/notifications.js b/app/javascript/mastodon/reducers/notifications.js index 6ba80bd6a57..60e901e39e6 100644 --- a/app/javascript/mastodon/reducers/notifications.js +++ b/app/javascript/mastodon/reducers/notifications.js @@ -13,6 +13,8 @@ import { import { ACCOUNT_BLOCK_SUCCESS, ACCOUNT_MUTE_SUCCESS, + FOLLOW_REQUEST_AUTHORIZE_SUCCESS, + FOLLOW_REQUEST_REJECT_SUCCESS, } from '../actions/accounts'; import { DOMAIN_BLOCK_SUCCESS } from 'mastodon/actions/domain_blocks'; import { TIMELINE_DELETE, TIMELINE_DISCONNECT } from '../actions/timelines'; @@ -89,8 +91,8 @@ const expandNormalizedNotifications = (state, notifications, next, isLoadingRece }); }; -const filterNotifications = (state, accountIds) => { - const helper = list => list.filterNot(item => item !== null && accountIds.includes(item.get('account'))); +const filterNotifications = (state, accountIds, type) => { + const helper = list => list.filterNot(item => item !== null && accountIds.includes(item.get('account')) && (type === undefined || type === item.get('type'))); return state.update('items', helper).update('pendingItems', helper); }; @@ -129,6 +131,11 @@ export default function notifications(state = initialState, action) { return action.relationship.muting_notifications ? filterNotifications(state, [action.relationship.id]) : state; case DOMAIN_BLOCK_SUCCESS: return filterNotifications(state, action.accounts); + case FOLLOW_REQUEST_AUTHORIZE_SUCCESS: + case FOLLOW_REQUEST_REJECT_SUCCESS: + return filterNotifications(state, [action.id], 'follow_request'); + case ACCOUNT_MUTE_SUCCESS: + return action.relationship.muting_notifications ? filterNotifications(state, [action.relationship.id]) : state; case NOTIFICATIONS_CLEAR: return state.set('items', ImmutableList()).set('pendingItems', ImmutableList()).set('hasMore', false); case TIMELINE_DELETE: diff --git a/app/javascript/mastodon/reducers/push_notifications.js b/app/javascript/mastodon/reducers/push_notifications.js index 317352b7906..c48cfb705ac 100644 --- a/app/javascript/mastodon/reducers/push_notifications.js +++ b/app/javascript/mastodon/reducers/push_notifications.js @@ -6,6 +6,7 @@ const initialState = Immutable.Map({ subscription: null, alerts: new Immutable.Map({ follow: false, + follow_request: false, favourite: false, reblog: false, mention: false, diff --git a/app/javascript/mastodon/reducers/settings.js b/app/javascript/mastodon/reducers/settings.js index 793a99f8f56..efef2ad9a53 100644 --- a/app/javascript/mastodon/reducers/settings.js +++ b/app/javascript/mastodon/reducers/settings.js @@ -30,6 +30,7 @@ const initialState = ImmutableMap({ notifications: ImmutableMap({ alerts: ImmutableMap({ follow: true, + follow_request: false, favourite: true, reblog: true, mention: true, @@ -44,6 +45,7 @@ const initialState = ImmutableMap({ shows: ImmutableMap({ follow: true, + follow_request: false, favourite: true, reblog: true, mention: true, @@ -52,6 +54,7 @@ const initialState = ImmutableMap({ sounds: ImmutableMap({ follow: true, + follow_request: false, favourite: true, reblog: true, mention: true, diff --git a/app/javascript/mastodon/reducers/user_lists.js b/app/javascript/mastodon/reducers/user_lists.js index 08e94022f1c..a7853452f0d 100644 --- a/app/javascript/mastodon/reducers/user_lists.js +++ b/app/javascript/mastodon/reducers/user_lists.js @@ -1,3 +1,6 @@ +import { + NOTIFICATIONS_UPDATE, +} from '../actions/notifications'; import { FOLLOWERS_FETCH_SUCCESS, FOLLOWERS_EXPAND_SUCCESS, @@ -53,6 +56,12 @@ const appendToList = (state, type, id, accounts, next) => { }); }; +const normalizeFollowRequest = (state, notification) => { + return state.updateIn(['follow_requests', 'items'], list => { + return list.filterNot(item => item === notification.account.id).unshift(notification.account.id); + }); +}; + export default function userLists(state = initialState, action) { switch(action.type) { case FOLLOWERS_FETCH_SUCCESS: @@ -67,6 +76,8 @@ export default function userLists(state = initialState, action) { return state.setIn(['reblogged_by', action.id], ImmutableList(action.accounts.map(item => item.id))); case FAVOURITES_FETCH_SUCCESS: return state.setIn(['favourited_by', action.id], ImmutableList(action.accounts.map(item => item.id))); + case NOTIFICATIONS_UPDATE: + return action.notification.type === 'follow_request' ? normalizeFollowRequest(state, action.notification) : state; case FOLLOW_REQUESTS_FETCH_SUCCESS: return state.setIn(['follow_requests', 'items'], ImmutableList(action.accounts.map(item => item.id))).setIn(['follow_requests', 'next'], action.next); case FOLLOW_REQUESTS_EXPAND_SUCCESS: diff --git a/app/javascript/mastodon/service_worker/web_push_locales.js b/app/javascript/mastodon/service_worker/web_push_locales.js index 5ce8c7b50a1..1265f3cfafb 100644 --- a/app/javascript/mastodon/service_worker/web_push_locales.js +++ b/app/javascript/mastodon/service_worker/web_push_locales.js @@ -16,6 +16,7 @@ filenames.forEach(filename => { filtered[locale] = { 'notification.favourite': full['notification.favourite'] || '', 'notification.follow': full['notification.follow'] || '', + 'notification.follow_request': full['notification.follow_request'] || '', 'notification.mention': full['notification.mention'] || '', 'notification.reblog': full['notification.reblog'] || '', 'notification.poll': full['notification.poll'] || '', diff --git a/app/models/notification.rb b/app/models/notification.rb index 498673ff13d..ad7528f505c 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -42,7 +42,7 @@ class Notification < ApplicationRecord validates :activity_type, inclusion: { in: TYPE_CLASS_MAP.values } scope :browserable, ->(exclude_types = [], account_id = nil) { - types = TYPE_CLASS_MAP.values - activity_types_from_types(exclude_types + [:follow_request]) + types = TYPE_CLASS_MAP.values - activity_types_from_types(exclude_types) if account_id.nil? where(activity_type: types) else @@ -50,7 +50,7 @@ class Notification < ApplicationRecord end } - cache_associated :from_account, status: STATUS_INCLUDES, mention: [status: STATUS_INCLUDES], favourite: [:account, status: STATUS_INCLUDES], follow: :account, poll: [status: STATUS_INCLUDES] + 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 @@ -69,10 +69,6 @@ class Notification < ApplicationRecord end end - def browserable? - type != :follow_request - end - class << self def cache_ids select(:id, :updated_at, :activity_type, :activity_id) diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index b5c72158950..9364a6ae879 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -9,7 +9,7 @@ class NotifyService < BaseService return if recipient.user.nil? || blocked? create_notification! - push_notification! if @notification.browserable? + push_notification! push_to_conversation! if direct_message? send_email! if email_enabled? rescue ActiveRecord::RecordInvalid diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 59c582cde0d..d2e676ec26e 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -34,32 +34,6 @@ RSpec.describe Notification, type: :model do end end - describe '#browserable?' do - let(:notification) { Fabricate(:notification) } - - subject { notification.browserable? } - - context 'type is :follow_request' do - before do - allow(notification).to receive(:type).and_return(:follow_request) - end - - it 'returns false' do - is_expected.to be false - end - end - - context 'type is not :follow_request' do - before do - allow(notification).to receive(:type).and_return(:else) - end - - it 'returns true' do - is_expected.to be true - end - end - end - describe '#type' do it 'returns :reblog for a Status' do notification = Notification.new(activity: Status.new)