From 79305428a7c2bda311bc9d367a84acc28f569522 Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Tue, 1 Sep 2020 13:31:28 +0200
Subject: [PATCH] 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
---
 app/controllers/api/v1/lists_controller.rb    |  2 +-
 app/javascript/mastodon/actions/lists.js      |  4 +-
 .../mastodon/features/list_timeline/index.js  | 30 +++++-
 .../styles/mastodon/components.scss           |  4 +
 app/lib/feed_manager.rb                       |  3 +-
 app/models/list.rb                            | 13 ++-
 app/serializers/rest/list_serializer.rb       |  2 +-
 ...0181127165847_add_show_replies_to_lists.rb | 23 +++++
 db/schema.rb                                  |  1 +
 spec/lib/feed_manager_spec.rb                 | 97 ++++++++++++++++++-
 10 files changed, 165 insertions(+), 14 deletions(-)
 create mode 100644 db/migrate/20181127165847_add_show_replies_to_lists.rb

diff --git a/app/controllers/api/v1/lists_controller.rb b/app/controllers/api/v1/lists_controller.rb
index 054172bee3..e5ac45fefb 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 d736bacef4..5ab9224363 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 f3205b2bff..a3be8fbea3 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 d91dde6414..cfcd937fab 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 bebdc4a743..9ab7b53be4 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 c9c94fca1d..8493046e58 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 977da7439a..3e87f71196 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 0000000000..f68c98dafc
--- /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 9e1c2748b4..e37aae9621 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 22eddd2ab6..7219e0e5ba 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