Per-user reblog hiding implementation/fixes/tests

Note that this will only hide/show *future* reblogs by a user, and does
nothing to remove/add reblogs that are already in the timeline. I don't
think that's a particularly confusing behavior, and it's a lot easier
to implement (similar to mutes, I believe).
This commit is contained in:
aschmitz 2017-11-10 20:11:10 -06:00
parent 2aedd8eaad
commit 2f1fdd37d3
11 changed files with 170 additions and 14 deletions

View file

@ -81,7 +81,7 @@ module AccountInteractions
rel.show_reblogs = reblogs rel.show_reblogs = reblogs
rel.save! rel.save!
end end
rel rel
end end
@ -156,6 +156,10 @@ module AccountInteractions
mute_relationships.where(target_account: other_account, hide_notifications: true).exists? mute_relationships.where(target_account: other_account, hide_notifications: true).exists?
end end
def muting_reblogs?(other_account)
active_relationships.where(target_account: other_account, show_reblogs: false).exists?
end
def requested?(other_account) def requested?(other_account)
follow_requests.where(target_account: other_account).exists? follow_requests.where(target_account: other_account).exists?
end end

View file

@ -22,7 +22,7 @@ class FollowRequest < ApplicationRecord
validates :account_id, uniqueness: { scope: :target_account_id } validates :account_id, uniqueness: { scope: :target_account_id }
def authorize! def authorize!
account.follow!(target_account, reblogs: reblogs) account.follow!(target_account, reblogs: show_reblogs)
MergeWorker.perform_async(target_account.id, account.id) MergeWorker.perform_async(target_account.id, account.id)
destroy! destroy!

View file

@ -39,7 +39,7 @@ class FollowService < BaseService
private private
def request_follow(source_account, target_account, reblogs: true) def request_follow(source_account, target_account, reblogs: true)
follow_request = FollowRequest.create!(account: source_account, target_account: target_account, reblogs: reblogs) follow_request = FollowRequest.create!(account: source_account, target_account: target_account, show_reblogs: reblogs)
if target_account.local? if target_account.local?
NotifyService.new.call(target_account, follow_request) NotifyService.new.call(target_account, follow_request)

View file

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

View file

@ -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

View file

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

View file

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

View file

@ -37,4 +37,41 @@ describe AccountInteractions do
end end
end 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 end

View file

@ -1,7 +1,29 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe FollowRequest, type: :model do 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 '#reject!'
describe 'validations' do describe 'validations' do

View file

@ -13,8 +13,20 @@ RSpec.describe FollowService do
subject.call(sender, bob.acct) subject.call(sender, bob.acct)
end end
it 'creates a follow request' do it 'creates a follow request with reblogs' do
expect(FollowRequest.find_by(account: sender, target_account: bob)).to_not be_nil 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
end end
@ -25,8 +37,22 @@ RSpec.describe FollowService do
subject.call(sender, bob.acct) subject.call(sender, bob.acct)
end end
it 'creates a following relation' do it 'creates a following relation with reblogs' do
expect(sender.following?(bob)).to be true 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
end end
@ -42,6 +68,32 @@ RSpec.describe FollowService do
expect(sender.following?(bob)).to be true expect(sender.following?(bob)).to be true
end end
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 end
context 'remote OStatus account' do context 'remote OStatus account' do

View file

@ -48,6 +48,26 @@ RSpec.describe NotifyService do
is_expected.to_not change(Notification, :count) is_expected.to_not change(Notification, :count)
end 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 context do
let(:asshole) { Fabricate(:account, username: 'asshole') } let(:asshole) { Fabricate(:account, username: 'asshole') }
let(:reply_to) { Fabricate(:status, account: asshole) } let(:reply_to) { Fabricate(:status, account: asshole) }