Add forward_to_domains parameter to POST /api/v1/reports (#25866)

This commit is contained in:
Claire 2023-07-10 18:26:56 +02:00 committed by GitHub
parent f3fca78756
commit c27b82a437
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 153 additions and 80 deletions

View file

@ -23,6 +23,6 @@ class Api::V1::ReportsController < Api::BaseController
end end
def report_params def report_params
params.permit(:account_id, :comment, :category, :forward, status_ids: [], rule_ids: []) params.permit(:account_id, :comment, :category, :forward, forward_to_domains: [], status_ids: [], rule_ids: [])
end end
end end

View file

@ -1,53 +1,75 @@
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { PureComponent } from 'react'; import { useCallback, useEffect, useRef } from 'react';
import { injectIntl, defineMessages, FormattedMessage } from 'react-intl'; import { useIntl, defineMessages, FormattedMessage } from 'react-intl';
import { OrderedSet, List as ImmutableList } from 'immutable';
import ImmutablePropTypes from 'react-immutable-proptypes';
import { shallowEqual } from 'react-redux';
import { createSelector } from 'reselect';
import Toggle from 'react-toggle'; import Toggle from 'react-toggle';
import { fetchAccount } from 'mastodon/actions/accounts';
import Button from 'mastodon/components/button'; import Button from 'mastodon/components/button';
import { useAppDispatch, useAppSelector } from 'mastodon/store';
const messages = defineMessages({ const messages = defineMessages({
placeholder: { id: 'report.placeholder', defaultMessage: 'Type or paste additional comments' }, placeholder: { id: 'report.placeholder', defaultMessage: 'Type or paste additional comments' },
}); });
class Comment extends PureComponent { const selectRepliedToAccountIds = createSelector(
[
static propTypes = { (state) => state.get('statuses'),
onSubmit: PropTypes.func.isRequired, (_, statusIds) => statusIds,
comment: PropTypes.string.isRequired, ],
onChangeComment: PropTypes.func.isRequired, (statusesMap, statusIds) => statusIds.map((statusId) => statusesMap.getIn([statusId, 'in_reply_to_account_id'])),
intl: PropTypes.object.isRequired, {
isSubmitting: PropTypes.bool, resultEqualityCheck: shallowEqual,
forward: PropTypes.bool,
isRemote: PropTypes.bool,
domain: PropTypes.string,
onChangeForward: PropTypes.func.isRequired,
};
handleClick = () => {
const { onSubmit } = this.props;
onSubmit();
};
handleChange = e => {
const { onChangeComment } = this.props;
onChangeComment(e.target.value);
};
handleKeyDown = e => {
if (e.keyCode === 13 && (e.ctrlKey || e.metaKey)) {
this.handleClick();
} }
}; );
handleForwardChange = e => { const Comment = ({ comment, domain, statusIds, isRemote, isSubmitting, selectedDomains, onSubmit, onChangeComment, onToggleDomain }) => {
const { onChangeForward } = this.props; const intl = useIntl();
onChangeForward(e.target.checked);
};
render () { const dispatch = useAppDispatch();
const { comment, isRemote, forward, domain, isSubmitting, intl } = this.props; const loadedRef = useRef(false);
const handleClick = useCallback(() => onSubmit(), [onSubmit]);
const handleChange = useCallback((e) => onChangeComment(e.target.value), [onChangeComment]);
const handleToggleDomain = useCallback(e => onToggleDomain(e.target.value, e.target.checked), [onToggleDomain]);
const handleKeyDown = useCallback((e) => {
if (e.keyCode === 13 && (e.ctrlKey || e.metaKey)) {
handleClick();
}
}, [handleClick]);
// Memoize accountIds since we don't want it to trigger `useEffect` on each render
const accountIds = useAppSelector((state) => domain ? selectRepliedToAccountIds(state, statusIds) : ImmutableList());
// While we could memoize `availableDomains`, it is pretty inexpensive to recompute
const accountsMap = useAppSelector((state) => state.get('accounts'));
const availableDomains = domain ? OrderedSet([domain]).union(accountIds.map((accountId) => accountsMap.getIn([accountId, 'acct'], '').split('@')[1]).filter(domain => !!domain)) : OrderedSet();
useEffect(() => {
if (loadedRef.current) {
return;
}
loadedRef.current = true;
// First, pre-select known domains
availableDomains.forEach((domain) => {
onToggleDomain(domain, true);
});
// Then, fetch missing replied-to accounts
const unknownAccounts = OrderedSet(accountIds.filter(accountId => accountId && !accountsMap.has(accountId)));
unknownAccounts.forEach((accountId) => {
dispatch(fetchAccount(accountId));
});
});
return ( return (
<> <>
@ -57,8 +79,8 @@ class Comment extends PureComponent {
className='report-dialog-modal__textarea' className='report-dialog-modal__textarea'
placeholder={intl.formatMessage(messages.placeholder)} placeholder={intl.formatMessage(messages.placeholder)}
value={comment} value={comment}
onChange={this.handleChange} onChange={handleChange}
onKeyDown={this.handleKeyDown} onKeyDown={handleKeyDown}
disabled={isSubmitting} disabled={isSubmitting}
/> />
@ -66,22 +88,34 @@ class Comment extends PureComponent {
<> <>
<p className='report-dialog-modal__lead'><FormattedMessage id='report.forward_hint' defaultMessage='The account is from another server. Send an anonymized copy of the report there as well?' /></p> <p className='report-dialog-modal__lead'><FormattedMessage id='report.forward_hint' defaultMessage='The account is from another server. Send an anonymized copy of the report there as well?' /></p>
<label className='report-dialog-modal__toggle'> { availableDomains.map((domain) => (
<Toggle checked={forward} disabled={isSubmitting} onChange={this.handleForwardChange} /> <label className='report-dialog-modal__toggle' key={`toggle-${domain}`}>
<Toggle checked={selectedDomains.includes(domain)} disabled={isSubmitting} onChange={handleToggleDomain} value={domain} />
<FormattedMessage id='report.forward' defaultMessage='Forward to {target}' values={{ target: domain }} /> <FormattedMessage id='report.forward' defaultMessage='Forward to {target}' values={{ target: domain }} />
</label> </label>
))}
</> </>
)} )}
<div className='flex-spacer' /> <div className='flex-spacer' />
<div className='report-dialog-modal__actions'> <div className='report-dialog-modal__actions'>
<Button onClick={this.handleClick} disabled={isSubmitting}><FormattedMessage id='report.submit' defaultMessage='Submit report' /></Button> <Button onClick={handleClick} disabled={isSubmitting}><FormattedMessage id='report.submit' defaultMessage='Submit report' /></Button>
</div> </div>
</> </>
); );
} }
} Comment.propTypes = {
comment: PropTypes.string.isRequired,
domain: PropTypes.string,
statusIds: ImmutablePropTypes.list.isRequired,
isRemote: PropTypes.bool,
isSubmitting: PropTypes.bool,
selectedDomains: ImmutablePropTypes.set.isRequired,
onSubmit: PropTypes.func.isRequired,
onChangeComment: PropTypes.func.isRequired,
onToggleDomain: PropTypes.func.isRequired,
};
export default injectIntl(Comment); export default Comment;

View file

@ -45,25 +45,26 @@ class ReportModal extends ImmutablePureComponent {
state = { state = {
step: 'category', step: 'category',
selectedStatusIds: OrderedSet(this.props.statusId ? [this.props.statusId] : []), selectedStatusIds: OrderedSet(this.props.statusId ? [this.props.statusId] : []),
selectedDomains: OrderedSet(),
comment: '', comment: '',
category: null, category: null,
selectedRuleIds: OrderedSet(), selectedRuleIds: OrderedSet(),
forward: true,
isSubmitting: false, isSubmitting: false,
isSubmitted: false, isSubmitted: false,
}; };
handleSubmit = () => { handleSubmit = () => {
const { dispatch, accountId } = this.props; const { dispatch, accountId } = this.props;
const { selectedStatusIds, comment, category, selectedRuleIds, forward } = this.state; const { selectedStatusIds, selectedDomains, comment, category, selectedRuleIds } = this.state;
this.setState({ isSubmitting: true }); this.setState({ isSubmitting: true });
dispatch(submitReport({ dispatch(submitReport({
account_id: accountId, account_id: accountId,
status_ids: selectedStatusIds.toArray(), status_ids: selectedStatusIds.toArray(),
selected_domains: selectedDomains.toArray(),
comment, comment,
forward, forward: selectedDomains.size > 0,
category, category,
rule_ids: selectedRuleIds.toArray(), rule_ids: selectedRuleIds.toArray(),
}, this.handleSuccess, this.handleFail)); }, this.handleSuccess, this.handleFail));
@ -87,13 +88,19 @@ class ReportModal extends ImmutablePureComponent {
} }
}; };
handleRuleToggle = (ruleId, checked) => { handleDomainToggle = (domain, checked) => {
const { selectedRuleIds } = this.state;
if (checked) { if (checked) {
this.setState({ selectedRuleIds: selectedRuleIds.add(ruleId) }); this.setState((state) => ({ selectedDomains: state.selectedDomains.add(domain) }));
} else { } else {
this.setState({ selectedRuleIds: selectedRuleIds.remove(ruleId) }); this.setState((state) => ({ selectedDomains: state.selectedDomains.remove(domain) }));
}
};
handleRuleToggle = (ruleId, checked) => {
if (checked) {
this.setState((state) => ({ selectedRuleIds: state.selectedRuleIds.add(ruleId) }));
} else {
this.setState((state) => ({ selectedRuleIds: state.selectedRuleIds.remove(ruleId) }));
} }
}; };
@ -105,10 +112,6 @@ class ReportModal extends ImmutablePureComponent {
this.setState({ comment }); this.setState({ comment });
}; };
handleChangeForward = forward => {
this.setState({ forward });
};
handleNextStep = step => { handleNextStep = step => {
this.setState({ step }); this.setState({ step });
}; };
@ -136,8 +139,8 @@ class ReportModal extends ImmutablePureComponent {
step, step,
selectedStatusIds, selectedStatusIds,
selectedRuleIds, selectedRuleIds,
selectedDomains,
comment, comment,
forward,
category, category,
isSubmitting, isSubmitting,
isSubmitted, isSubmitted,
@ -185,10 +188,11 @@ class ReportModal extends ImmutablePureComponent {
isSubmitting={isSubmitting} isSubmitting={isSubmitting}
isRemote={isRemote} isRemote={isRemote}
comment={comment} comment={comment}
forward={forward}
domain={domain} domain={domain}
onChangeComment={this.handleChangeComment} onChangeComment={this.handleChangeComment}
onChangeForward={this.handleChangeForward} statusIds={selectedStatusIds}
selectedDomains={selectedDomains}
onToggleDomain={this.handleDomainToggle}
/> />
); );
break; break;

View file

@ -5784,6 +5784,7 @@ a.status-card.compact:hover {
&__toggle { &__toggle {
display: flex; display: flex;
align-items: center; align-items: center;
margin-bottom: 10px;
& > span { & > span {
font-size: 17px; font-size: 17px;

View file

@ -16,7 +16,11 @@ class ReportService < BaseService
create_report! create_report!
notify_staff! notify_staff!
forward_to_origin! if forward?
if forward?
forward_to_origin!
forward_to_replied_to!
end
@report @report
end end
@ -29,7 +33,7 @@ class ReportService < BaseService
status_ids: reported_status_ids, status_ids: reported_status_ids,
comment: @comment, comment: @comment,
uri: @options[:uri], uri: @options[:uri],
forwarded: forward?, forwarded: forward_to_origin?,
category: @category, category: @category,
rule_ids: @rule_ids rule_ids: @rule_ids
) )
@ -45,11 +49,15 @@ class ReportService < BaseService
end end
def forward_to_origin! def forward_to_origin!
return unless forward_to_origin?
# Send report to the server where the account originates from # Send report to the server where the account originates from
ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, @target_account.inbox_url) ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, @target_account.inbox_url)
end
def forward_to_replied_to!
# Send report to servers to which the account was replying to, so they also have a chance to act # Send report to servers to which the account was replying to, so they also have a chance to act
inbox_urls = Account.remote.where(id: Status.where(id: reported_status_ids).where.not(in_reply_to_account_id: nil).select(:in_reply_to_account_id)).inboxes - [@target_account.inbox_url] inbox_urls = Account.remote.where(domain: forward_to_domains).where(id: Status.where(id: reported_status_ids).where.not(in_reply_to_account_id: nil).select(:in_reply_to_account_id)).inboxes - [@target_account.inbox_url]
inbox_urls.each do |inbox_url| inbox_urls.each do |inbox_url|
ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, inbox_url) ActivityPub::DeliveryWorker.perform_async(payload, some_local_account.id, inbox_url)
@ -60,6 +68,14 @@ class ReportService < BaseService
!@target_account.local? && ActiveModel::Type::Boolean.new.cast(@options[:forward]) !@target_account.local? && ActiveModel::Type::Boolean.new.cast(@options[:forward])
end end
def forward_to_origin?
forward? && forward_to_domains.include?(@target_account.domain)
end
def forward_to_domains
@forward_to_domains ||= (@options[:forward_to_domains] || [@target_account.domain]).filter_map { |domain| TagManager.instance.normalize_domain(domain&.strip) }.uniq
end
def reported_status_ids def reported_status_ids
return AccountStatusesFilter.new(@target_account, @source_account).results.with_discarded.find(Array(@status_ids)).pluck(:id) if @source_account.local? return AccountStatusesFilter.new(@target_account, @source_account).results.with_discarded.find(Array(@status_ids)).pluck(:id) if @source_account.local?

View file

@ -44,9 +44,27 @@ RSpec.describe ReportService, type: :service do
stub_request(:post, 'http://foo.com/inbox').to_return(status: 200) stub_request(:post, 'http://foo.com/inbox').to_return(status: 200)
end end
it 'sends ActivityPub payload to the author of the replied-to post' do context 'when forward_to_domains includes both the replied-to domain and the origin domain' do
subject.call(source_account, remote_account, status_ids: [reported_status.id], forward: forward) it 'sends ActivityPub payload to both the author of the replied-to post and the reported user' do
subject.call(source_account, remote_account, status_ids: [reported_status.id], forward: forward, forward_to_domains: [remote_account.domain, remote_thread_account.domain])
expect(a_request(:post, 'http://foo.com/inbox')).to have_been_made expect(a_request(:post, 'http://foo.com/inbox')).to have_been_made
expect(a_request(:post, 'http://example.com/inbox')).to have_been_made
end
end
context 'when forward_to_domains includes only the replied-to domain' do
it 'sends ActivityPub payload only to the author of the replied-to post' do
subject.call(source_account, remote_account, status_ids: [reported_status.id], forward: forward, forward_to_domains: [remote_thread_account.domain])
expect(a_request(:post, 'http://foo.com/inbox')).to have_been_made
expect(a_request(:post, 'http://example.com/inbox')).to_not have_been_made
end
end
context 'when forward_to_domains does not include the replied-to domain' do
it 'does not send ActivityPub payload to the author of the replied-to post' do
subject.call(source_account, remote_account, status_ids: [reported_status.id], forward: forward)
expect(a_request(:post, 'http://foo.com/inbox')).to_not have_been_made
end
end end
end end
end end