Merge pull request #212 from aschmitz/feat/mute-reblogs

Allow hiding reblogs on a per-follow basis

app/controllers/api/v1/accounts_controller.rb

@@ -13,9 +13,11 @@ class Api::V1::AccountsController < Api::BaseController
   end
 
   def follow
-    FollowService.new.call(current_user.account, @account.acct)
+    reblogs_arg = { reblogs: params[:reblogs] }
     
-    options = @account.locked? ? {} : { following_map: { @account.id => true }, requested_map: { @account.id => false } }
+    FollowService.new.call(current_user.account, @account.acct, reblogs_arg)
+
+    options = @account.locked? ? {} : { following_map: { @account.id => reblogs_arg }, requested_map: { @account.id => false } }
 
     render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options)
   end

app/javascript/glitch/components/account/header.js

@@ -152,7 +152,7 @@ appropriate icon.
             <IconButton
               size={26}
               icon={following ? 'user-times' : 'user-plus'}
-              active={following}
+              active={following ? true : false}
               title={intl.formatMessage(following ? messages.unfollow : messages.follow)}
               onClick={this.props.onFollow}
             />

app/javascript/mastodon/actions/accounts.js

@@ -105,12 +105,13 @@ export function fetchAccountFail(id, error) {
   };
 };
 
-export function followAccount(id) {
+export function followAccount(id, reblogs = true) {
   return (dispatch, getState) => {
+    const alreadyFollowing = getState().getIn(['relationships', id, 'following']);
     dispatch(followAccountRequest(id));
 
-    api(getState).post(`/api/v1/accounts/${id}/follow`).then(response => {
-      dispatch(followAccountSuccess(response.data));
+    api(getState).post(`/api/v1/accounts/${id}/follow`, { reblogs }).then(response => {
+      dispatch(followAccountSuccess(response.data, alreadyFollowing));
     }).catch(error => {
       dispatch(followAccountFail(error));
     });
@@ -136,10 +137,11 @@ export function followAccountRequest(id) {
   };
 };
 
-export function followAccountSuccess(relationship) {
+export function followAccountSuccess(relationship, alreadyFollowing) {
   return {
     type: ACCOUNT_FOLLOW_SUCCESS,
     relationship,
+    alreadyFollowing,
   };
 };
 

app/javascript/mastodon/components/account.js

@@ -93,7 +93,7 @@ export default class Account extends ImmutablePureComponent {
           </div>
         );
       } else {
-        buttons = <IconButton icon={following ? 'user-times' : 'user-plus'} title={intl.formatMessage(following ? messages.unfollow : messages.follow)} onClick={this.handleFollow} active={following} />;
+        buttons = <IconButton icon={following ? 'user-times' : 'user-plus'} title={intl.formatMessage(following ? messages.unfollow : messages.follow)} onClick={this.handleFollow} active={following ? true : false} />;
       }
     }
 

app/javascript/mastodon/features/account/components/action_bar.js

@@ -19,6 +19,8 @@ const messages = defineMessages({
   media: { id: 'account.media', defaultMessage: 'Media' },
   blockDomain: { id: 'account.block_domain', defaultMessage: 'Hide everything from {domain}' },
   unblockDomain: { id: 'account.unblock_domain', defaultMessage: 'Unhide {domain}' },
+  hideReblogs: { id: 'account.hide_reblogs', defaultMessage: 'Hide boosts from @{name}' },
+  showReblogs: { id: 'account.show_reblogs', defaultMessage: 'Show boosts from @{name}' },
 });
 
 @injectIntl
@@ -30,6 +32,7 @@ export default class ActionBar extends React.PureComponent {
     onFollow: PropTypes.func,
     onBlock: PropTypes.func.isRequired,
     onMention: PropTypes.func.isRequired,
+    onReblogToggle: PropTypes.func.isRequired,
     onReport: PropTypes.func.isRequired,
     onMute: PropTypes.func.isRequired,
     onBlockDomain: PropTypes.func.isRequired,
@@ -60,6 +63,15 @@ export default class ActionBar extends React.PureComponent {
     if (account.get('id') === me) {
       menu.push({ text: intl.formatMessage(messages.edit_profile), href: '/settings/profile' });
     } else {
+      const following = account.getIn(['relationship', 'following']);
+      if (following) {
+        if (following.get('reblogs')) {
+          menu.push({ text: intl.formatMessage(messages.hideReblogs, { name: account.get('username') }), action: this.props.onReblogToggle });
+        } else {
+          menu.push({ text: intl.formatMessage(messages.showReblogs, { name: account.get('username') }), action: this.props.onReblogToggle });
+        }
+      }
+
       if (account.getIn(['relationship', 'muting'])) {
         menu.push({ text: intl.formatMessage(messages.unmute, { name: account.get('username') }), action: this.props.onMute });
       } else {

app/javascript/mastodon/features/account_timeline/components/header.js

@@ -14,6 +14,7 @@ export default class Header extends ImmutablePureComponent {
     onFollow: PropTypes.func.isRequired,
     onBlock: PropTypes.func.isRequired,
     onMention: PropTypes.func.isRequired,
+    onReblogToggle: PropTypes.func.isRequired,
     onReport: PropTypes.func.isRequired,
     onMute: PropTypes.func.isRequired,
     onBlockDomain: PropTypes.func.isRequired,
@@ -40,6 +41,10 @@ export default class Header extends ImmutablePureComponent {
     this.props.onReport(this.props.account);
   }
 
+  handleReblogToggle = () => {
+    this.props.onReblogToggle(this.props.account);
+  }
+
   handleMute = () => {
     this.props.onMute(this.props.account);
   }
@@ -80,6 +85,7 @@ export default class Header extends ImmutablePureComponent {
           me={me}
           onBlock={this.handleBlock}
           onMention={this.handleMention}
+          onReblogToggle={this.handleReblogToggle}
           onReport={this.handleReport}
           onMute={this.handleMute}
           onBlockDomain={this.handleBlockDomain}

app/javascript/mastodon/features/account_timeline/containers/header_container.js

@@ -68,6 +68,14 @@ const mapDispatchToProps = (dispatch, { intl }) => ({
     dispatch(mentionCompose(account, router));
   },
 
+  onReblogToggle (account) {
+    if (account.getIn(['relationship', 'following', 'reblogs'])) {
+      dispatch(followAccount(account.get('id'), false));
+    } else {
+      dispatch(followAccount(account.get('id'), true));
+    }
+  },
+
   onReport (account) {
     dispatch(initReport(account));
   },

app/javascript/mastodon/reducers/accounts_counters.js

@@ -126,6 +126,7 @@ export default function accountsCounters(state = initialState, action) {
   case STATUS_FETCH_SUCCESS:
     return normalizeAccountFromStatus(state, action.status);
   case ACCOUNT_FOLLOW_SUCCESS:
+    if (action.alreadyFollowing) { return state; }
     return state.updateIn([action.relationship.id, 'followers_count'], num => num + 1);
   case ACCOUNT_UNFOLLOW_SUCCESS:
     return state.updateIn([action.relationship.id, 'followers_count'], num => Math.max(0, num - 1));

app/lib/feed_manager.rb

@@ -160,7 +160,9 @@ class FeedManager
       should_filter &&= status.account_id != status.in_reply_to_account_id                                               # and it's not a self-reply
       return should_filter
     elsif status.reblog?                                                                                                 # Filter out a reblog
-      should_filter   = Block.where(account_id: status.reblog.account_id, target_account_id: receiver_id).exists?        # or if the author of the reblogged status is blocking me
+      src_id = status.account_id
+      should_filter   = Follow.where(account_id: receiver_id, target_account_id: src_id, show_reblogs: false).exists?    # if the reblogger's reblogs are suppressed
+      should_filter ||= Block.where(account_id: status.reblog.account_id, target_account_id: receiver_id).exists?        # or if the author of the reblogged status is blocking me
       should_filter ||= AccountDomainBlock.where(account_id: receiver_id, domain: status.reblog.account.domain).exists?  # or the author's domain is blocked
       return should_filter
     end

app/models/concerns/account_interactions.rb

@@ -5,7 +5,11 @@ module AccountInteractions
 
   class_methods do
     def following_map(target_account_ids, account_id)
-      follow_mapping(Follow.where(target_account_id: target_account_ids, account_id: account_id), :target_account_id)
+      Follow.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |follow, mapping|
+        mapping[follow.target_account_id] = {
+          reblogs: follow.show_reblogs?
+        }
+      end
     end
 
     def followed_by_map(target_account_ids, account_id)
@@ -25,7 +29,11 @@ module AccountInteractions
     end
 
     def requested_map(target_account_ids, account_id)
-      follow_mapping(FollowRequest.where(target_account_id: target_account_ids, account_id: account_id), :target_account_id)
+      FollowRequest.where(target_account_id: target_account_ids, account_id: account_id).each_with_object({}) do |follow_request, mapping|
+        mapping[follow_request.target_account_id] = {
+          reblogs: follow_request.show_reblogs?
+        }
+      end
     end
 
     def domain_blocking_map(target_account_ids, account_id)
@@ -66,8 +74,12 @@ module AccountInteractions
     has_many :domain_blocks, class_name: 'AccountDomainBlock', dependent: :destroy
   end
 
-  def follow!(other_account)
-    active_relationships.find_or_create_by!(target_account: other_account)
+  def follow!(other_account, reblogs: nil)
+    reblogs = true if reblogs.nil?
+    rel = active_relationships.create_with(show_reblogs: reblogs).find_or_create_by!(target_account: other_account)
+    rel.update!(show_reblogs: reblogs)
+
+    rel
   end
 
   def block!(other_account)
@@ -141,6 +153,10 @@ module AccountInteractions
     mute_relationships.where(target_account: other_account, hide_notifications: true).exists?
   end
 
+  def muting_reblogs?(other_account)
+    active_relationships.where(target_account: other_account, show_reblogs: false).exists?
+  end
+
   def requested?(other_account)
     follow_requests.where(target_account: other_account).exists?
   end

app/models/follow.rb

@@ -8,6 +8,7 @@
 #  account_id        :integer          not null
 #  id                :integer          not null, primary key
 #  target_account_id :integer          not null
+#  show_reblogs      :boolean          default(TRUE), not null
 #
 
 class Follow < ApplicationRecord

app/models/follow_request.rb

@@ -8,6 +8,7 @@
 #  account_id        :integer          not null
 #  id                :integer          not null, primary key
 #  target_account_id :integer          not null
+#  show_reblogs      :boolean          default(TRUE), not null
 #
 
 class FollowRequest < ApplicationRecord
@@ -21,7 +22,7 @@ class FollowRequest < ApplicationRecord
   validates :account_id, uniqueness: { scope: :target_account_id }
 
   def authorize!
-    account.follow!(target_account)
+    account.follow!(target_account, reblogs: show_reblogs)
     MergeWorker.perform_async(target_account.id, account.id)
 
     destroy!

app/services/follow_service.rb

@@ -6,25 +6,38 @@ class FollowService < BaseService
   # Follow a remote user, notify remote user about the follow
   # @param [Account] source_account From which to follow
   # @param [String, Account] uri User URI to follow in the form of username@domain (or account record)
-  def call(source_account, uri)
+  # @param [true, false, nil] reblogs Whether or not to show reblogs, defaults to true
+  def call(source_account, uri, reblogs: nil)
+    reblogs = true if reblogs.nil?
     target_account = uri.is_a?(Account) ? uri : ResolveRemoteAccountService.new.call(uri)
 
     raise ActiveRecord::RecordNotFound if target_account.nil? || target_account.id == source_account.id || target_account.suspended?
     raise Mastodon::NotPermittedError  if target_account.blocking?(source_account) || source_account.blocking?(target_account)
 
-    return if source_account.following?(target_account) || source_account.requested?(target_account)
+    if source_account.following?(target_account)
+      # We're already following this account, but we'll call follow! again to
+      # make sure the reblogs status is set correctly.
+      source_account.follow!(target_account, reblogs: reblogs)
+      return
+    elsif source_account.requested?(target_account)
+      # This isn't managed by a method in AccountInteractions, so we modify it
+      # ourselves if necessary.
+      req = follow_requests.find_by(target_account: other_account)
+      req.update!(show_reblogs: reblogs)
+      return
+    end
 
     if target_account.locked? || target_account.activitypub?
-      request_follow(source_account, target_account)
+      request_follow(source_account, target_account, reblogs: reblogs)
     else
-      direct_follow(source_account, target_account)
+      direct_follow(source_account, target_account, reblogs: reblogs)
     end
   end
 
   private
 
-  def request_follow(source_account, target_account)
-    follow_request = FollowRequest.create!(account: source_account, target_account: target_account)
+  def request_follow(source_account, target_account, reblogs: true)
+    follow_request = FollowRequest.create!(account: source_account, target_account: target_account, show_reblogs: reblogs)
 
     if target_account.local?
       NotifyService.new.call(target_account, follow_request)
@@ -38,8 +51,8 @@ class FollowService < BaseService
     follow_request
   end
 
-  def direct_follow(source_account, target_account)
-    follow = source_account.follow!(target_account)
+  def direct_follow(source_account, target_account, reblogs: true)
+    follow = source_account.follow!(target_account, reblogs: reblogs)
 
     if target_account.local?
       NotifyService.new.call(target_account, follow)

app/services/notify_service.rb

@@ -29,7 +29,7 @@ class NotifyService < BaseService
   end
 
   def blocked_reblog?
-    false
+    @recipient.muting_reblogs?(@notification.from_account)
   end
 
   def blocked_follow_request?

db/migrate/20171028221157_add_reblogs_to_follows.rb

@@ -0,0 +1,21 @@
+require Rails.root.join('lib', 'mastodon', 'migration_helpers')
+
+class AddReblogsToFollows < ActiveRecord::Migration[5.1]
+  include Mastodon::MigrationHelpers
+
+  safety_assured do
+    disable_ddl_transaction!
+  end
+
+  def up
+    safety_assured do
+      add_column_with_default :follows, :show_reblogs, :boolean, default: true, allow_null: false
+      add_column_with_default :follow_requests, :show_reblogs, :boolean, default: true, allow_null: false
+    end
+  end
+  
+  def down
+    remove_column :follows, :show_reblogs
+    remove_column :follow_requests, :show_reblogs
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171021191900) do
+ActiveRecord::Schema.define(version: 20171028221157) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -145,6 +145,7 @@ ActiveRecord::Schema.define(version: 20171021191900) do
     t.datetime "updated_at", null: false
     t.bigint "account_id", null: false
     t.bigint "target_account_id", null: false
+    t.boolean "show_reblogs", default: true, null: false
     t.index ["account_id", "target_account_id"], name: "index_follow_requests_on_account_id_and_target_account_id", unique: true
   end
 
@@ -153,6 +154,7 @@ ActiveRecord::Schema.define(version: 20171021191900) do
     t.datetime "updated_at", null: false
     t.bigint "account_id", null: false
     t.bigint "target_account_id", null: false
+    t.boolean "show_reblogs", default: true, null: false
     t.index ["account_id", "target_account_id"], name: "index_follows_on_account_id_and_target_account_id", unique: true
   end
 

spec/controllers/api/v1/accounts/relationships_controller_spec.rb

@@ -32,7 +32,7 @@ describe Api::V1::Accounts::RelationshipsController do
         json = body_as_json
 
         expect(json).to be_a Enumerable
-        expect(json.first[:following]).to be true
+        expect(json.first[:following]).to be_truthy
         expect(json.first[:followed_by]).to be false
       end
     end
@@ -51,7 +51,7 @@ describe Api::V1::Accounts::RelationshipsController do
 
         expect(json).to be_a Enumerable
         expect(json.first[:id]).to eq simon.id.to_s
-        expect(json.first[:following]).to be true
+        expect(json.first[:following]).to be_truthy
         expect(json.first[:followed_by]).to be false
         expect(json.first[:muting]).to be false
         expect(json.first[:requested]).to be false

spec/controllers/api/v1/accounts_controller_spec.rb

@@ -31,10 +31,10 @@ RSpec.describe Api::V1::AccountsController, type: :controller do
         expect(response).to have_http_status(:success)
       end
 
-      it 'returns JSON with following=true and requested=false' do
+      it 'returns JSON with following=truthy and requested=false' do
         json = body_as_json
 
-        expect(json[:following]).to be true
+        expect(json[:following]).to be_truthy
         expect(json[:requested]).to be false
       end
 
@@ -50,11 +50,11 @@ RSpec.describe Api::V1::AccountsController, type: :controller do
         expect(response).to have_http_status(:success)
       end
 
-      it 'returns JSON with following=false and requested=true' do
+      it 'returns JSON with following=false and requested=truthy' do
         json = body_as_json
 
         expect(json[:following]).to be false
-        expect(json[:requested]).to be true
+        expect(json[:requested]).to be_truthy
       end
 
       it 'creates a follow request relation between user and target user' do

spec/lib/feed_manager_spec.rb

@@ -56,6 +56,13 @@ RSpec.describe FeedManager do
         expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true
       end
 
+      it 'returns true for reblog from account with reblogs disabled' do
+        status = Fabricate(:status, text: 'Hello world', account: jeff)
+        reblog = Fabricate(:status, reblog: status, account: alice)
+        bob.follow!(alice, reblogs: false)
+        expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true
+      end
+
       it 'returns false for reply by followee to another followee' do
         status = Fabricate(:status, text: 'Hello world', account: jeff)
         reply  = Fabricate(:status, text: 'Nay', thread: status, account: alice)

spec/models/concerns/account_interactions_spec.rb

@@ -37,4 +37,41 @@ describe AccountInteractions do
       end
     end
   end
+
+  describe 'ignoring reblogs from an account' do
+    before do
+      @me = Fabricate(:account, username: 'Me')
+      @you = Fabricate(:account, username: 'You')
+    end
+
+    context 'with the reblogs option unspecified' do
+      before do
+        @me.follow!(@you)
+      end
+
+      it 'defaults to showing reblogs' do
+        expect(@me.muting_reblogs?(@you)).to be(false)
+      end
+    end
+
+    context 'with the reblogs option set to false' do
+      before do
+        @me.follow!(@you, reblogs: false)
+      end
+
+      it 'does mute reblogs' do
+        expect(@me.muting_reblogs?(@you)).to be(true)
+      end
+    end
+
+    context 'with the reblogs option set to true' do
+      before do
+        @me.follow!(@you, reblogs: true)
+      end
+
+      it 'does not mute reblogs' do
+        expect(@me.muting_reblogs?(@you)).to be(false)
+      end
+    end
+  end
 end

spec/models/follow_request_spec.rb

@@ -1,7 +1,29 @@
 require 'rails_helper'
 
 RSpec.describe FollowRequest, type: :model do
-  describe '#authorize!'
+  describe '#authorize!' do
+    it 'generates a Follow' do
+      follow_request = Fabricate.create(:follow_request)
+      follow_request.authorize!
+      target = follow_request.target_account
+      expect(follow_request.account.following?(target)).to be true
+    end
+
+    it 'correctly passes show_reblogs when true' do
+      follow_request = Fabricate.create(:follow_request, show_reblogs: true)
+      follow_request.authorize!
+      target = follow_request.target_account
+      expect(follow_request.account.muting_reblogs?(target)).to be false
+    end
+
+    it 'correctly passes show_reblogs when false' do
+      follow_request = Fabricate.create(:follow_request, show_reblogs: false)
+      follow_request.authorize!
+      target = follow_request.target_account
+      expect(follow_request.account.muting_reblogs?(target)).to be true
+    end
+  end
+
   describe '#reject!'
 
   describe 'validations' do

spec/services/follow_service_spec.rb

@@ -13,8 +13,20 @@ RSpec.describe FollowService do
         subject.call(sender, bob.acct)
       end
 
-      it 'creates a follow request' do
-        expect(FollowRequest.find_by(account: sender, target_account: bob)).to_not be_nil
+      it 'creates a follow request with reblogs' do
+        expect(FollowRequest.find_by(account: sender, target_account: bob, show_reblogs: true)).to_not be_nil
+      end
+    end
+
+    describe 'locked account, no reblogs' do
+      let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, locked: true, username: 'bob')).account }
+
+      before do
+        subject.call(sender, bob.acct, reblogs: false)
+      end
+
+      it 'creates a follow request without reblogs' do
+        expect(FollowRequest.find_by(account: sender, target_account: bob, show_reblogs: false)).to_not be_nil
       end
     end
 
@@ -25,8 +37,22 @@ RSpec.describe FollowService do
         subject.call(sender, bob.acct)
       end
 
-      it 'creates a following relation' do
+      it 'creates a following relation with reblogs' do
         expect(sender.following?(bob)).to be true
+        expect(sender.muting_reblogs?(bob)).to be false
+      end
+    end
+
+    describe 'unlocked account, no reblogs' do
+      let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
+
+      before do
+        subject.call(sender, bob.acct, reblogs: false)
+      end
+
+      it 'creates a following relation without reblogs' do
+        expect(sender.following?(bob)).to be true
+        expect(sender.muting_reblogs?(bob)).to be true
       end
     end
 
@@ -42,6 +68,32 @@ RSpec.describe FollowService do
         expect(sender.following?(bob)).to be true
       end
     end
+
+    describe 'already followed account, turning reblogs off' do
+      let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
+
+      before do
+        sender.follow!(bob, reblogs: true)
+        subject.call(sender, bob.acct, reblogs: false)
+      end
+
+      it 'disables reblogs' do
+        expect(sender.muting_reblogs?(bob)).to be true
+      end
+    end
+
+    describe 'already followed account, turning reblogs on' do
+      let(:bob) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account }
+
+      before do
+        sender.follow!(bob, reblogs: false)
+        subject.call(sender, bob.acct, reblogs: true)
+      end
+
+      it 'disables reblogs' do
+        expect(sender.muting_reblogs?(bob)).to be false
+      end
+    end
   end
 
   context 'remote OStatus account' do

spec/services/notify_service_spec.rb

@@ -48,6 +48,26 @@ RSpec.describe NotifyService do
     is_expected.to_not change(Notification, :count)
   end
 
+  describe 'reblogs' do
+    let(:status)   { Fabricate(:status, account: Fabricate(:account)) }
+    let(:activity) { Fabricate(:status, account: sender, reblog: status) }
+
+    it 'shows reblogs by default' do
+      recipient.follow!(sender)
+      is_expected.to change(Notification, :count)
+    end
+
+    it 'shows reblogs when explicitly enabled' do
+      recipient.follow!(sender, reblogs: true)
+      is_expected.to change(Notification, :count)
+    end
+
+    it 'hides reblogs when disabled' do
+      recipient.follow!(sender, reblogs: false)
+      is_expected.to_not change(Notification, :count)
+    end
+  end
+
   context do
     let(:asshole)  { Fabricate(:account, username: 'asshole') }
     let(:reply_to) { Fabricate(:status, account: asshole) }