Skip to content

Commit

Permalink
Add follow_request notification type (mastodon#12198)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ClearlyClaire authored and Gargron committed Dec 1, 2019
1 parent f60cd97 commit 911cc14
Show file tree
Hide file tree
Showing 16 changed files with 158 additions and 48 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v1/push/subscriptions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ def subscription_params

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
3 changes: 2 additions & 1 deletion app/controllers/api/web/push_subscriptions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def create
data = {
alerts: {
follow: alerts_enabled,
follow_request: false,
favourite: alerts_enabled,
reblog: alerts_enabled,
mention: alerts_enabled,
Expand Down Expand Up @@ -58,6 +59,6 @@ def subscription_params
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
2 changes: 1 addition & 1 deletion app/javascript/mastodon/actions/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ export default class ColumnSettings extends React.PureComponent {
</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'>
<span id='notifications-favourite' className='column-settings__section'><FormattedMessage id='notifications.column_settings.favourite' defaultMessage='Favourites:' /></span>

Expand Down
Original file line number Diff line number Diff line change
@@ -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>
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -127,7 +128,29 @@ class Notification extends ImmutablePureComponent {
</span>
</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>
</HotKeys>
);
Expand Down Expand Up @@ -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':
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});

Expand All @@ -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;
}

Expand Down
11 changes: 9 additions & 2 deletions app/javascript/mastodon/reducers/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
};

Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions app/javascript/mastodon/reducers/push_notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions app/javascript/mastodon/reducers/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const initialState = ImmutableMap({
notifications: ImmutableMap({
alerts: ImmutableMap({
follow: true,
follow_request: false,
favourite: true,
reblog: true,
mention: true,
Expand All @@ -44,6 +45,7 @@ const initialState = ImmutableMap({

shows: ImmutableMap({
follow: true,
follow_request: false,
favourite: true,
reblog: true,
mention: true,
Expand All @@ -52,6 +54,7 @@ const initialState = ImmutableMap({

sounds: ImmutableMap({
follow: true,
follow_request: false,
favourite: true,
reblog: true,
mention: true,
Expand Down
11 changes: 11 additions & 0 deletions app/javascript/mastodon/reducers/user_lists.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import {
NOTIFICATIONS_UPDATE,
} from '../actions/notifications';
import {
FOLLOWERS_FETCH_SUCCESS,
FOLLOWERS_EXPAND_SUCCESS,
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down
1 change: 1 addition & 0 deletions app/javascript/mastodon/service_worker/web_push_locales.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'] || '',
Expand Down
8 changes: 2 additions & 6 deletions app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ 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
where(activity_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, 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
Expand All @@ -69,10 +69,6 @@ def target_status
end
end

def browserable?
type != :follow_request
end

class << self
def cache_ids
select(:id, :updated_at, :activity_type, :activity_id)
Expand Down
2 changes: 1 addition & 1 deletion app/services/notify_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def call(recipient, activity)
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
Expand Down
26 changes: 0 additions & 26 deletions spec/models/notification_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,32 +34,6 @@
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)
Expand Down

0 comments on commit 911cc14

Please sign in to comment.