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
This commit is contained in:
ThibG 2019-12-01 17:25:29 +01:00 committed by Eugen Rochko
parent acb1927158
commit 60d26cabf0
16 changed files with 158 additions and 48 deletions

View file

@ -51,6 +51,6 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController
def data_params def data_params
return {} if params[:data].blank? 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
end end

View file

@ -19,6 +19,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController
data = { data = {
alerts: { alerts: {
follow: alerts_enabled, follow: alerts_enabled,
follow_request: false,
favourite: alerts_enabled, favourite: alerts_enabled,
reblog: alerts_enabled, reblog: alerts_enabled,
mention: alerts_enabled, mention: alerts_enabled,
@ -58,6 +59,6 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController
end end
def data_params 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
end end

View file

@ -110,7 +110,7 @@ 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', 'favourite', 'reblog', 'mention', 'poll']); const allTypes = ImmutableList(['follow', 'follow_request', 'favourite', 'reblog', 'mention', 'poll']);
return allTypes.filterNot(item => item === filter).toJS(); return allTypes.filterNot(item => item === filter).toJS();
}; };

View file

@ -57,6 +57,17 @@ export default class ColumnSettings extends React.PureComponent {
</div> </div>
</div> </div>
<div role='group' aria-labelledby='notifications-follow-request'>
<span id='notifications-follow-request' className='column-settings__section'><FormattedMessage id='notifications.column_settings.follow_request' defaultMessage='New follow requests:' /></span>
<div className='column-settings__row'>
<SettingToggle prefix='notifications_desktop' settings={settings} settingPath={['alerts', 'follow_request']} onChange={onChange} label={alertStr} />
{showPushSettings && <SettingToggle prefix='notifications_push' settings={pushSettings} settingPath={['alerts', 'follow_request']} onChange={this.onPushChange} label={pushStr} />}
<SettingToggle prefix='notifications' settings={settings} settingPath={['shows', 'follow_request']} onChange={onChange} label={showStr} />
<SettingToggle prefix='notifications' settings={settings} settingPath={['sounds', 'follow_request']} onChange={onChange} label={soundStr} />
</div>
</div>
<div role='group' aria-labelledby='notifications-favourite'> <div role='group' aria-labelledby='notifications-favourite'>
<span id='notifications-favourite' className='column-settings__section'><FormattedMessage id='notifications.column_settings.favourite' defaultMessage='Favourites:' /></span> <span id='notifications-favourite' className='column-settings__section'><FormattedMessage id='notifications.column_settings.favourite' defaultMessage='Favourites:' /></span>

View file

@ -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 <div />;
}
if (hidden) {
return (
<Fragment>
{account.get('display_name')}
{account.get('username')}
</Fragment>
);
}
return (
<div className='account'>
<div className='account__wrapper'>
<Permalink key={account.get('id')} className='account__display-name' title={account.get('acct')} href={account.get('url')} to={`/accounts/${account.get('id')}`}>
<div className='account__avatar-wrapper'><Avatar account={account} size={36} /></div>
<DisplayName account={account} />
</Permalink>
<div className='account__relationship'>
<IconButton title={intl.formatMessage(messages.authorize)} icon='check' onClick={onAuthorize} />
<IconButton title={intl.formatMessage(messages.reject)} icon='times' onClick={onReject} />
</div>
</div>
</div>
);
}
}

View file

@ -7,6 +7,7 @@ import ImmutablePureComponent from 'react-immutable-pure-component';
import { me } from 'mastodon/initial_state'; import { me } from 'mastodon/initial_state';
import StatusContainer from 'mastodon/containers/status_container'; import StatusContainer from 'mastodon/containers/status_container';
import AccountContainer from 'mastodon/containers/account_container'; import AccountContainer from 'mastodon/containers/account_container';
import FollowRequestContainer from '../containers/follow_request_container';
import Icon from 'mastodon/components/icon'; import Icon from 'mastodon/components/icon';
import Permalink from 'mastodon/components/permalink'; import Permalink from 'mastodon/components/permalink';
@ -127,7 +128,29 @@ class Notification extends ImmutablePureComponent {
</span> </span>
</div> </div>
<AccountContainer id={account.get('id')} withNote={false} hidden={this.props.hidden} /> <AccountContainer id={account.get('id')} hidden={this.props.hidden} />
</div>
</HotKeys>
);
}
renderFollowRequest (notification, account, link) {
const { intl } = this.props;
return (
<HotKeys handlers={this.getHandlers()}>
<div className='notification notification-follow-request focusable' tabIndex='0' aria-label={notificationForScreenReader(intl, intl.formatMessage({ id: 'notification.follow_request', defaultMessage: '{name} has requested to follow you' }, { name: account.get('acct') }), notification.get('created_at'))}>
<div className='notification__message'>
<div className='notification__favourite-icon-wrapper'>
<Icon id='user' fixedWidth />
</div>
<span title={notification.get('created_at')}>
<FormattedMessage id='notification.follow_request' defaultMessage='{name} has requested to follow you' values={{ name: link }} />
</span>
</div>
<FollowRequestContainer id={account.get('id')} withNote={false} hidden={this.props.hidden} />
</div> </div>
</HotKeys> </HotKeys>
); );
@ -261,6 +284,8 @@ class Notification extends ImmutablePureComponent {
switch(notification.get('type')) { switch(notification.get('type')) {
case 'follow': case 'follow':
return this.renderFollow(notification, account, link); return this.renderFollow(notification, account, link);
case 'follow_request':
return this.renderFollowRequest(notification, account, link);
case 'mention': case 'mention':
return this.renderMention(notification); return this.renderMention(notification);
case 'favourite': case 'favourite':

View file

@ -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);

View file

@ -4,12 +4,10 @@ import { fetchFollowRequests } from 'mastodon/actions/accounts';
import { connect } from 'react-redux'; import { connect } from 'react-redux';
import { NavLink, withRouter } from 'react-router-dom'; import { NavLink, withRouter } from 'react-router-dom';
import IconWithBadge from 'mastodon/components/icon_with_badge'; import IconWithBadge from 'mastodon/components/icon_with_badge';
import { me } from 'mastodon/initial_state';
import { List as ImmutableList } from 'immutable'; import { List as ImmutableList } from 'immutable';
import { FormattedMessage } from 'react-intl'; import { FormattedMessage } from 'react-intl';
const mapStateToProps = state => ({ const mapStateToProps = state => ({
locked: state.getIn(['accounts', me, 'locked']),
count: state.getIn(['user_lists', 'follow_requests', 'items'], ImmutableList()).size, count: state.getIn(['user_lists', 'follow_requests', 'items'], ImmutableList()).size,
}); });
@ -19,22 +17,19 @@ class FollowRequestsNavLink extends React.Component {
static propTypes = { static propTypes = {
dispatch: PropTypes.func.isRequired, dispatch: PropTypes.func.isRequired,
locked: PropTypes.bool,
count: PropTypes.number.isRequired, count: PropTypes.number.isRequired,
}; };
componentDidMount () { componentDidMount () {
const { dispatch, locked } = this.props; const { dispatch } = this.props;
if (locked) { dispatch(fetchFollowRequests());
dispatch(fetchFollowRequests());
}
} }
render () { render () {
const { locked, count } = this.props; const { count } = this.props;
if (!locked || count === 0) { if (count === 0) {
return null; return null;
} }

View file

@ -13,6 +13,8 @@ import {
import { import {
ACCOUNT_BLOCK_SUCCESS, ACCOUNT_BLOCK_SUCCESS,
ACCOUNT_MUTE_SUCCESS, ACCOUNT_MUTE_SUCCESS,
FOLLOW_REQUEST_AUTHORIZE_SUCCESS,
FOLLOW_REQUEST_REJECT_SUCCESS,
} from '../actions/accounts'; } from '../actions/accounts';
import { DOMAIN_BLOCK_SUCCESS } from 'mastodon/actions/domain_blocks'; import { DOMAIN_BLOCK_SUCCESS } from 'mastodon/actions/domain_blocks';
import { TIMELINE_DELETE, TIMELINE_DISCONNECT } from '../actions/timelines'; import { TIMELINE_DELETE, TIMELINE_DISCONNECT } from '../actions/timelines';
@ -89,8 +91,8 @@ const expandNormalizedNotifications = (state, notifications, next, isLoadingRece
}); });
}; };
const filterNotifications = (state, accountIds) => { const filterNotifications = (state, accountIds, type) => {
const helper = list => list.filterNot(item => item !== null && accountIds.includes(item.get('account'))); 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); 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; return action.relationship.muting_notifications ? filterNotifications(state, [action.relationship.id]) : state;
case DOMAIN_BLOCK_SUCCESS: case DOMAIN_BLOCK_SUCCESS:
return filterNotifications(state, action.accounts); 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: case NOTIFICATIONS_CLEAR:
return state.set('items', ImmutableList()).set('pendingItems', ImmutableList()).set('hasMore', false); return state.set('items', ImmutableList()).set('pendingItems', ImmutableList()).set('hasMore', false);
case TIMELINE_DELETE: case TIMELINE_DELETE:

View file

@ -6,6 +6,7 @@ const initialState = Immutable.Map({
subscription: null, subscription: null,
alerts: new Immutable.Map({ alerts: new Immutable.Map({
follow: false, follow: false,
follow_request: false,
favourite: false, favourite: false,
reblog: false, reblog: false,
mention: false, mention: false,

View file

@ -30,6 +30,7 @@ const initialState = ImmutableMap({
notifications: ImmutableMap({ notifications: ImmutableMap({
alerts: ImmutableMap({ alerts: ImmutableMap({
follow: true, follow: true,
follow_request: false,
favourite: true, favourite: true,
reblog: true, reblog: true,
mention: true, mention: true,
@ -44,6 +45,7 @@ const initialState = ImmutableMap({
shows: ImmutableMap({ shows: ImmutableMap({
follow: true, follow: true,
follow_request: false,
favourite: true, favourite: true,
reblog: true, reblog: true,
mention: true, mention: true,
@ -52,6 +54,7 @@ const initialState = ImmutableMap({
sounds: ImmutableMap({ sounds: ImmutableMap({
follow: true, follow: true,
follow_request: false,
favourite: true, favourite: true,
reblog: true, reblog: true,
mention: true, mention: true,

View file

@ -1,3 +1,6 @@
import {
NOTIFICATIONS_UPDATE,
} from '../actions/notifications';
import { import {
FOLLOWERS_FETCH_SUCCESS, FOLLOWERS_FETCH_SUCCESS,
FOLLOWERS_EXPAND_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) { export default function userLists(state = initialState, action) {
switch(action.type) { switch(action.type) {
case FOLLOWERS_FETCH_SUCCESS: 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))); return state.setIn(['reblogged_by', action.id], ImmutableList(action.accounts.map(item => item.id)));
case FAVOURITES_FETCH_SUCCESS: case FAVOURITES_FETCH_SUCCESS:
return state.setIn(['favourited_by', action.id], ImmutableList(action.accounts.map(item => item.id))); 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: case FOLLOW_REQUESTS_FETCH_SUCCESS:
return state.setIn(['follow_requests', 'items'], ImmutableList(action.accounts.map(item => item.id))).setIn(['follow_requests', 'next'], action.next); return state.setIn(['follow_requests', 'items'], ImmutableList(action.accounts.map(item => item.id))).setIn(['follow_requests', 'next'], action.next);
case FOLLOW_REQUESTS_EXPAND_SUCCESS: case FOLLOW_REQUESTS_EXPAND_SUCCESS:

View file

@ -16,6 +16,7 @@ filenames.forEach(filename => {
filtered[locale] = { filtered[locale] = {
'notification.favourite': full['notification.favourite'] || '', 'notification.favourite': full['notification.favourite'] || '',
'notification.follow': full['notification.follow'] || '', 'notification.follow': full['notification.follow'] || '',
'notification.follow_request': full['notification.follow_request'] || '',
'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'] || '',

View file

@ -42,7 +42,7 @@ class Notification < ApplicationRecord
validates :activity_type, inclusion: { in: TYPE_CLASS_MAP.values } validates :activity_type, inclusion: { in: TYPE_CLASS_MAP.values }
scope :browserable, ->(exclude_types = [], account_id = nil) { 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? if account_id.nil?
where(activity_type: types) where(activity_type: types)
else else
@ -50,7 +50,7 @@ class Notification < ApplicationRecord
end 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 def type
@type ||= TYPE_CLASS_MAP.invert[activity_type].to_sym @type ||= TYPE_CLASS_MAP.invert[activity_type].to_sym
@ -69,10 +69,6 @@ class Notification < ApplicationRecord
end end
end end
def browserable?
type != :follow_request
end
class << self class << self
def cache_ids def cache_ids
select(:id, :updated_at, :activity_type, :activity_id) select(:id, :updated_at, :activity_type, :activity_id)

View file

@ -9,7 +9,7 @@ class NotifyService < BaseService
return if recipient.user.nil? || blocked? return if recipient.user.nil? || blocked?
create_notification! create_notification!
push_notification! if @notification.browserable? push_notification!
push_to_conversation! if direct_message? push_to_conversation! if direct_message?
send_email! if email_enabled? send_email! if email_enabled?
rescue ActiveRecord::RecordInvalid rescue ActiveRecord::RecordInvalid

View file

@ -34,32 +34,6 @@ RSpec.describe Notification, type: :model do
end end
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 describe '#type' do
it 'returns :reblog for a Status' do it 'returns :reblog for a Status' do
notification = Notification.new(activity: Status.new) notification = Notification.new(activity: Status.new)