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
							
								
									a143764c4c
								
							
						
					
					
						commit
						50d0c1e95f
					
				
					 10 changed files with 165 additions and 14 deletions
				
			
		|  | @ -38,6 +38,6 @@ class Api::V1::ListsController < Api::BaseController | |||
|   end | ||||
| 
 | ||||
|   def list_params | ||||
|     params.permit(:title) | ||||
|     params.permit(:title, :replies_policy) | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -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) { | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -5934,6 +5934,10 @@ a.status-card.compact:hover { | |||
|   } | ||||
| } | ||||
| 
 | ||||
| .column-settings__row .radio-button { | ||||
|   display: block; | ||||
| } | ||||
| 
 | ||||
| .radio-button { | ||||
|   font-size: 14px; | ||||
|   position: relative; | ||||
|  |  | |||
|  | @ -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 | ||||
| 
 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
							
								
								
									
										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.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 | ||||
| 
 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue