Skip to content

Commit

Permalink
Add configuration option to filter replies in lists (mastodon#9205)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ClearlyClaire authored and thenameisnigel-old committed Sep 7, 2020
1 parent 08d22e7 commit 93f3b18
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 14 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api/v1/lists_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ def set_list
end

def list_params
params.permit(:title)
params.permit(:title, :replies_policy)
end
end
4 changes: 2 additions & 2 deletions app/javascript/mastodon/actions/lists.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ export const createListFail = error => ({
error,
});

export const updateList = (id, title, shouldReset) => (dispatch, getState) => {
export const updateList = (id, title, shouldReset, replies_policy) => (dispatch, getState) => {
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));

if (shouldReset) {
Expand Down
30 changes: 27 additions & 3 deletions app/javascript/mastodon/features/list_timeline/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,19 @@ import { addColumn, removeColumn, moveColumn } from '../../actions/columns';
import { FormattedMessage, defineMessages, injectIntl } from 'react-intl';
import { connectListStream } from '../../actions/streaming';
import { expandListTimeline } from '../../actions/timelines';
import { fetchList, deleteList } from '../../actions/lists';
import { fetchList, deleteList, updateList } from '../../actions/lists';
import { openModal } from '../../actions/modal';
import MissingIndicator from '../../components/missing_indicator';
import LoadingIndicator from '../../components/loading_indicator';
import Icon from 'mastodon/components/icon';
import RadioButton from 'mastodon/components/radio_button';

const messages = defineMessages({
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' },
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) => ({
Expand Down Expand Up @@ -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 () {
const { shouldUpdateScroll, hasUnread, columnId, multiColumn, list } = this.props;
const { shouldUpdateScroll, hasUnread, columnId, multiColumn, list, intl } = this.props;
const { id } = this.props.params;
const pinned = !!columnId;
const title = list ? list.get('title') : id;
const replies_policy = list ? list.get('replies_policy') : undefined;

if (typeof list === 'undefined') {
return (
Expand Down Expand Up @@ -166,7 +177,7 @@ class ListTimeline extends React.PureComponent {
pinned={pinned}
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}>
<Icon id='pencil' /> <FormattedMessage id='lists.edit' defaultMessage='Edit list' />
</button>
Expand All @@ -175,6 +186,19 @@ class ListTimeline extends React.PureComponent {
<Icon id='trash' /> <FormattedMessage id='lists.delete' defaultMessage='Delete list' />
</button>
</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>

<StatusListContainer
Expand Down
4 changes: 4 additions & 0 deletions app/javascript/styles/mastodon/components.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5934,6 +5934,10 @@ a.status-card.compact:hover {
}
}

.column-settings__row .radio-button {
display: block;
}

.radio-button {
font-size: 14px;
position: relative;
Expand Down
3 changes: 2 additions & 1 deletion app/lib/feed_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def unpush_from_home(account, status)
def push_to_list(list, status)
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 &&= !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
end

Expand Down
13 changes: 8 additions & 5 deletions app/models/list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,21 @@
#
# Table name: lists
#
# id :bigint(8) not null, primary key
# account_id :bigint(8) not null
# title :string default(""), not null
# created_at :datetime not null
# updated_at :datetime not null
# id :bigint(8) not null, primary key
# account_id :bigint(8) not null
# title :string default(""), not null
# created_at :datetime not null
# updated_at :datetime not null
# replies_policy :integer default("list_replies"), not null
#

class List < ApplicationRecord
include Paginable

PER_ACCOUNT_LIMIT = 50

enum replies_policy: [:list_replies, :all_replies, :no_replies], _prefix: :show

belongs_to :account, optional: true

has_many :list_accounts, inverse_of: :list, dependent: :destroy
Expand Down
2 changes: 1 addition & 1 deletion app/serializers/rest/list_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

class REST::ListSerializer < ActiveModel::Serializer
attributes :id, :title
attributes :id, :title, :replies_policy

def id
object.id.to_s
Expand Down
23 changes: 23 additions & 0 deletions db/migrate/20181127165847_add_show_replies_to_lists.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@
t.string "title", default: "", null: false
t.datetime "created_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"
end

Expand Down
97 changes: 96 additions & 1 deletion spec/lib/feed_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,109 @@
end

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
list = Fabricate(:list)
reblog = Fabricate(:status)
status = Fabricate(:status, reblog: reblog)
FeedManager.instance.push_to_list(list, status)

expect(FeedManager.instance.push_to_list(list, reblog)).to eq false
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

describe '#merge_into_timeline' do
Expand Down

0 comments on commit 93f3b18

Please sign in to comment.