Add configuration option to filter replies in lists (#9205)
* Add database support for list show-reply preferences * Add backend support to read and update list-specific show_replies settings * Add basic UI to set list replies setting * Add specs for list replies policy * Switch "cycling" reply policy link to a set of radio inputs * Capitalize replies_policy strings * Change radio button design to be consistent with that of the directory explorer
This commit is contained in:
parent
1c308af84c
commit
79305428a7
10 changed files with 165 additions and 14 deletions
|
@ -38,6 +38,6 @@ class Api::V1::ListsController < Api::BaseController
|
||||||
end
|
end
|
||||||
|
|
||||||
def list_params
|
def list_params
|
||||||
params.permit(:title)
|
params.permit(:title, :replies_policy)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -150,10 +150,10 @@ export const createListFail = error => ({
|
||||||
error,
|
error,
|
||||||
});
|
});
|
||||||
|
|
||||||
export const updateList = (id, title, shouldReset) => (dispatch, getState) => {
|
export const updateList = (id, title, shouldReset, replies_policy) => (dispatch, getState) => {
|
||||||
dispatch(updateListRequest(id));
|
dispatch(updateListRequest(id));
|
||||||
|
|
||||||
api(getState).put(`/api/v1/lists/${id}`, { title }).then(({ data }) => {
|
api(getState).put(`/api/v1/lists/${id}`, { title, replies_policy }).then(({ data }) => {
|
||||||
dispatch(updateListSuccess(data));
|
dispatch(updateListSuccess(data));
|
||||||
|
|
||||||
if (shouldReset) {
|
if (shouldReset) {
|
||||||
|
|
|
@ -10,15 +10,19 @@ import { addColumn, removeColumn, moveColumn } from '../../actions/columns';
|
||||||
import { FormattedMessage, defineMessages, injectIntl } from 'react-intl';
|
import { FormattedMessage, defineMessages, injectIntl } from 'react-intl';
|
||||||
import { connectListStream } from '../../actions/streaming';
|
import { connectListStream } from '../../actions/streaming';
|
||||||
import { expandListTimeline } from '../../actions/timelines';
|
import { expandListTimeline } from '../../actions/timelines';
|
||||||
import { fetchList, deleteList } from '../../actions/lists';
|
import { fetchList, deleteList, updateList } from '../../actions/lists';
|
||||||
import { openModal } from '../../actions/modal';
|
import { openModal } from '../../actions/modal';
|
||||||
import MissingIndicator from '../../components/missing_indicator';
|
import MissingIndicator from '../../components/missing_indicator';
|
||||||
import LoadingIndicator from '../../components/loading_indicator';
|
import LoadingIndicator from '../../components/loading_indicator';
|
||||||
import Icon from 'mastodon/components/icon';
|
import Icon from 'mastodon/components/icon';
|
||||||
|
import RadioButton from 'mastodon/components/radio_button';
|
||||||
|
|
||||||
const messages = defineMessages({
|
const messages = defineMessages({
|
||||||
deleteMessage: { id: 'confirmations.delete_list.message', defaultMessage: 'Are you sure you want to permanently delete this list?' },
|
deleteMessage: { id: 'confirmations.delete_list.message', defaultMessage: 'Are you sure you want to permanently delete this list?' },
|
||||||
deleteConfirm: { id: 'confirmations.delete_list.confirm', defaultMessage: 'Delete' },
|
deleteConfirm: { id: 'confirmations.delete_list.confirm', defaultMessage: 'Delete' },
|
||||||
|
all_replies: { id: 'lists.replies_policy.all_replies', defaultMessage: 'Any followed user' },
|
||||||
|
no_replies: { id: 'lists.replies_policy.no_replies', defaultMessage: 'No one' },
|
||||||
|
list_replies: { id: 'lists.replies_policy.list_replies', defaultMessage: 'Members of the list' },
|
||||||
});
|
});
|
||||||
|
|
||||||
const mapStateToProps = (state, props) => ({
|
const mapStateToProps = (state, props) => ({
|
||||||
|
@ -131,11 +135,18 @@ class ListTimeline extends React.PureComponent {
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
handleRepliesPolicyChange = ({ target }) => {
|
||||||
|
const { dispatch } = this.props;
|
||||||
|
const { id } = this.props.params;
|
||||||
|
dispatch(updateList(id, undefined, false, target.value));
|
||||||
|
}
|
||||||
|
|
||||||
render () {
|
render () {
|
||||||
const { shouldUpdateScroll, hasUnread, columnId, multiColumn, list } = this.props;
|
const { shouldUpdateScroll, hasUnread, columnId, multiColumn, list, intl } = this.props;
|
||||||
const { id } = this.props.params;
|
const { id } = this.props.params;
|
||||||
const pinned = !!columnId;
|
const pinned = !!columnId;
|
||||||
const title = list ? list.get('title') : id;
|
const title = list ? list.get('title') : id;
|
||||||
|
const replies_policy = list ? list.get('replies_policy') : undefined;
|
||||||
|
|
||||||
if (typeof list === 'undefined') {
|
if (typeof list === 'undefined') {
|
||||||
return (
|
return (
|
||||||
|
@ -166,7 +177,7 @@ class ListTimeline extends React.PureComponent {
|
||||||
pinned={pinned}
|
pinned={pinned}
|
||||||
multiColumn={multiColumn}
|
multiColumn={multiColumn}
|
||||||
>
|
>
|
||||||
<div className='column-header__links'>
|
<div className='column-settings__row column-header__links'>
|
||||||
<button className='text-btn column-header__setting-btn' tabIndex='0' onClick={this.handleEditClick}>
|
<button className='text-btn column-header__setting-btn' tabIndex='0' onClick={this.handleEditClick}>
|
||||||
<Icon id='pencil' /> <FormattedMessage id='lists.edit' defaultMessage='Edit list' />
|
<Icon id='pencil' /> <FormattedMessage id='lists.edit' defaultMessage='Edit list' />
|
||||||
</button>
|
</button>
|
||||||
|
@ -175,6 +186,19 @@ class ListTimeline extends React.PureComponent {
|
||||||
<Icon id='trash' /> <FormattedMessage id='lists.delete' defaultMessage='Delete list' />
|
<Icon id='trash' /> <FormattedMessage id='lists.delete' defaultMessage='Delete list' />
|
||||||
</button>
|
</button>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{ replies_policy !== undefined && (
|
||||||
|
<div role='group' aria-labelledby={`list-${id}-replies-policy`}>
|
||||||
|
<span id={`list-${id}-replies-policy`} className='column-settings__section'>
|
||||||
|
<FormattedMessage id='lists.replies_policy.title' defaultMessage='Show replies to:' />
|
||||||
|
</span>
|
||||||
|
<div className='column-settings__row'>
|
||||||
|
{ ['no_replies', 'list_replies', 'all_replies'].map(policy => (
|
||||||
|
<RadioButton name='order' value={policy} label={intl.formatMessage(messages[policy])} checked={replies_policy === policy} onChange={this.handleRepliesPolicyChange} />
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
</ColumnHeader>
|
</ColumnHeader>
|
||||||
|
|
||||||
<StatusListContainer
|
<StatusListContainer
|
||||||
|
|
|
@ -5934,6 +5934,10 @@ a.status-card.compact:hover {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.column-settings__row .radio-button {
|
||||||
|
display: block;
|
||||||
|
}
|
||||||
|
|
||||||
.radio-button {
|
.radio-button {
|
||||||
font-size: 14px;
|
font-size: 14px;
|
||||||
position: relative;
|
position: relative;
|
||||||
|
|
|
@ -49,7 +49,8 @@ class FeedManager
|
||||||
def push_to_list(list, status)
|
def push_to_list(list, status)
|
||||||
if status.reply? && status.in_reply_to_account_id != status.account_id
|
if status.reply? && status.in_reply_to_account_id != status.account_id
|
||||||
should_filter = status.in_reply_to_account_id != list.account_id
|
should_filter = status.in_reply_to_account_id != list.account_id
|
||||||
should_filter &&= !ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists?
|
should_filter &&= !list.show_all_replies?
|
||||||
|
should_filter &&= !(list.show_list_replies? && ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists?)
|
||||||
return false if should_filter
|
return false if should_filter
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -3,11 +3,12 @@
|
||||||
#
|
#
|
||||||
# Table name: lists
|
# Table name: lists
|
||||||
#
|
#
|
||||||
# id :bigint(8) not null, primary key
|
# id :bigint(8) not null, primary key
|
||||||
# account_id :bigint(8) not null
|
# account_id :bigint(8) not null
|
||||||
# title :string default(""), not null
|
# title :string default(""), not null
|
||||||
# created_at :datetime not null
|
# created_at :datetime not null
|
||||||
# updated_at :datetime not null
|
# updated_at :datetime not null
|
||||||
|
# replies_policy :integer default("list_replies"), not null
|
||||||
#
|
#
|
||||||
|
|
||||||
class List < ApplicationRecord
|
class List < ApplicationRecord
|
||||||
|
@ -15,6 +16,8 @@ class List < ApplicationRecord
|
||||||
|
|
||||||
PER_ACCOUNT_LIMIT = 50
|
PER_ACCOUNT_LIMIT = 50
|
||||||
|
|
||||||
|
enum replies_policy: [:list_replies, :all_replies, :no_replies], _prefix: :show
|
||||||
|
|
||||||
belongs_to :account, optional: true
|
belongs_to :account, optional: true
|
||||||
|
|
||||||
has_many :list_accounts, inverse_of: :list, dependent: :destroy
|
has_many :list_accounts, inverse_of: :list, dependent: :destroy
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
# frozen_string_literal: true
|
# frozen_string_literal: true
|
||||||
|
|
||||||
class REST::ListSerializer < ActiveModel::Serializer
|
class REST::ListSerializer < ActiveModel::Serializer
|
||||||
attributes :id, :title
|
attributes :id, :title, :replies_policy
|
||||||
|
|
||||||
def id
|
def id
|
||||||
object.id.to_s
|
object.id.to_s
|
||||||
|
|
23
db/migrate/20181127165847_add_show_replies_to_lists.rb
Normal file
23
db/migrate/20181127165847_add_show_replies_to_lists.rb
Normal file
|
@ -0,0 +1,23 @@
|
||||||
|
require Rails.root.join('lib', 'mastodon', 'migration_helpers')
|
||||||
|
|
||||||
|
class AddShowRepliesToLists < ActiveRecord::Migration[5.2]
|
||||||
|
include Mastodon::MigrationHelpers
|
||||||
|
|
||||||
|
disable_ddl_transaction!
|
||||||
|
|
||||||
|
def up
|
||||||
|
safety_assured do
|
||||||
|
add_column_with_default(
|
||||||
|
:lists,
|
||||||
|
:replies_policy,
|
||||||
|
:integer,
|
||||||
|
allow_null: false,
|
||||||
|
default: 0
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def down
|
||||||
|
remove_column :lists, :replies_policy
|
||||||
|
end
|
||||||
|
end
|
|
@ -468,6 +468,7 @@ ActiveRecord::Schema.define(version: 2020_06_30_190544) do
|
||||||
t.string "title", default: "", null: false
|
t.string "title", default: "", null: false
|
||||||
t.datetime "created_at", null: false
|
t.datetime "created_at", null: false
|
||||||
t.datetime "updated_at", null: false
|
t.datetime "updated_at", null: false
|
||||||
|
t.integer "replies_policy", default: 0, null: false
|
||||||
t.index ["account_id"], name: "index_lists_on_account_id"
|
t.index ["account_id"], name: "index_lists_on_account_id"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -309,14 +309,109 @@ RSpec.describe FeedManager do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#push_to_list' do
|
describe '#push_to_list' do
|
||||||
|
let(:owner) { Fabricate(:account, username: 'owner') }
|
||||||
|
let(:alice) { Fabricate(:account, username: 'alice') }
|
||||||
|
let(:bob) { Fabricate(:account, username: 'bob') }
|
||||||
|
let(:eve) { Fabricate(:account, username: 'eve') }
|
||||||
|
let(:list) { Fabricate(:list, account: owner) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
owner.follow!(alice)
|
||||||
|
owner.follow!(bob)
|
||||||
|
owner.follow!(eve)
|
||||||
|
|
||||||
|
list.accounts << alice
|
||||||
|
list.accounts << bob
|
||||||
|
end
|
||||||
|
|
||||||
it "does not push when the given status's reblog is already inserted" do
|
it "does not push when the given status's reblog is already inserted" do
|
||||||
list = Fabricate(:list)
|
|
||||||
reblog = Fabricate(:status)
|
reblog = Fabricate(:status)
|
||||||
status = Fabricate(:status, reblog: reblog)
|
status = Fabricate(:status, reblog: reblog)
|
||||||
FeedManager.instance.push_to_list(list, status)
|
FeedManager.instance.push_to_list(list, status)
|
||||||
|
|
||||||
expect(FeedManager.instance.push_to_list(list, reblog)).to eq false
|
expect(FeedManager.instance.push_to_list(list, reblog)).to eq false
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when replies policy is set to no replies' do
|
||||||
|
before do
|
||||||
|
list.replies_policy = :no_replies
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes statuses that are not replies' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, status)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes statuses that are replies to list owner' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: owner)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not push replies to another member of the list' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: alice)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq false
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when replies policy is set to list-only replies' do
|
||||||
|
before do
|
||||||
|
list.replies_policy = :list_replies
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes statuses that are not replies' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, status)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes statuses that are replies to list owner' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: owner)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes replies to another member of the list' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: alice)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not push replies to someone not a member of the list' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: eve)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq false
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when replies policy is set to any reply' do
|
||||||
|
before do
|
||||||
|
list.replies_policy = :all_replies
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes statuses that are not replies' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, status)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes statuses that are replies to list owner' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: owner)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes replies to another member of the list' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: alice)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq true
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'pushes replies to someone not a member of the list' do
|
||||||
|
status = Fabricate(:status, text: 'Hello world', account: eve)
|
||||||
|
reply = Fabricate(:status, text: 'Nay', thread: status, account: bob)
|
||||||
|
expect(FeedManager.instance.push_to_list(list, reply)).to eq true
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#merge_into_timeline' do
|
describe '#merge_into_timeline' do
|
||||||
|
|
Loading…
Reference in a new issue