diff options
author | ThibG <thib@sitedethib.com> | 2020-09-01 13:31:28 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-09-01 13:31:28 +0200 |
commit | 79305428a7c2bda311bc9d367a84acc28f569522 (patch) | |
tree | 2094941006afa438c0244c6721836018b6fbd3b4 | |
parent | 1c308af84cddf8491b11aa6431c225faa80a9a5b (diff) |
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
-rw-r--r-- | app/controllers/api/v1/lists_controller.rb | 2 | ||||
-rw-r--r-- | app/javascript/mastodon/actions/lists.js | 4 | ||||
-rw-r--r-- | app/javascript/mastodon/features/list_timeline/index.js | 30 | ||||
-rw-r--r-- | app/javascript/styles/mastodon/components.scss | 4 | ||||
-rw-r--r-- | app/lib/feed_manager.rb | 3 | ||||
-rw-r--r-- | app/models/list.rb | 13 | ||||
-rw-r--r-- | app/serializers/rest/list_serializer.rb | 2 | ||||
-rw-r--r-- | db/migrate/20181127165847_add_show_replies_to_lists.rb | 23 | ||||
-rw-r--r-- | db/schema.rb | 1 | ||||
-rw-r--r-- | spec/lib/feed_manager_spec.rb | 97 |
10 files changed, 165 insertions, 14 deletions
diff --git a/app/controllers/api/v1/lists_controller.rb b/app/controllers/api/v1/lists_controller.rb index 054172bee..e5ac45fef 100644 --- a/app/controllers/api/v1/lists_controller.rb +++ b/app/controllers/api/v1/lists_controller.rb @@ -38,6 +38,6 @@ class Api::V1::ListsController < Api::BaseController end def list_params - params.permit(:title) + params.permit(:title, :replies_policy) end end diff --git a/app/javascript/mastodon/actions/lists.js b/app/javascript/mastodon/actions/lists.js index d736bacef..5ab922436 100644 --- a/app/javascript/mastodon/actions/lists.js +++ b/app/javascript/mastodon/actions/lists.js @@ -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) { diff --git a/app/javascript/mastodon/features/list_timeline/index.js b/app/javascript/mastodon/features/list_timeline/index.js index f3205b2bf..a3be8fbea 100644 --- a/app/javascript/mastodon/features/list_timeline/index.js +++ b/app/javascript/mastodon/features/list_timeline/index.js @@ -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) => ({ @@ -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 ( @@ -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> @@ -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 diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index d91dde641..cfcd937fa 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -5934,6 +5934,10 @@ a.status-card.compact:hover { } } +.column-settings__row .radio-button { + display: block; +} + .radio-button { font-size: 14px; position: relative; diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index bebdc4a74..9ab7b53be 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -49,7 +49,8 @@ class FeedManager 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 diff --git a/app/models/list.rb b/app/models/list.rb index c9c94fca1..8493046e5 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -3,11 +3,12 @@ # # 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 @@ -15,6 +16,8 @@ class List < ApplicationRecord 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 diff --git a/app/serializers/rest/list_serializer.rb b/app/serializers/rest/list_serializer.rb index 977da7439..3e87f7119 100644 --- a/app/serializers/rest/list_serializer.rb +++ b/app/serializers/rest/list_serializer.rb @@ -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 diff --git a/db/migrate/20181127165847_add_show_replies_to_lists.rb b/db/migrate/20181127165847_add_show_replies_to_lists.rb new file mode 100644 index 000000000..f68c98daf --- /dev/null +++ b/db/migrate/20181127165847_add_show_replies_to_lists.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index 9e1c2748b..e37aae962 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -468,6 +468,7 @@ ActiveRecord::Schema.define(version: 2020_06_30_190544) do 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 diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 22eddd2ab..7219e0e5b 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -309,14 +309,109 @@ RSpec.describe FeedManager do 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 |