From eaaf2170fe76def60f056df99decc1cf0898cd6f Mon Sep 17 00:00:00 2001 From: Surinna Curtis Date: Thu, 27 Jul 2017 23:31:39 -0500 Subject: API support for muting notifications (and specs) --- .../controllers/api/v1/accounts_controller_spec.rb | 29 ++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'spec/controllers/api/v1/accounts_controller_spec.rb') diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 05df2f844..96e8ecd43 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -114,6 +114,35 @@ RSpec.describe Api::V1::AccountsController, type: :controller do it 'creates a muting relation' do expect(user.account.muting?(other_account)).to be true end + + it 'mutes notifications' do + expect(user.account.muting_notifications?(other_account)).to be true + end + end + + describe 'POST #mute with notifications set to false' do + let(:other_account) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + + before do + user.account.follow!(other_account) + post :mute, params: {id: other_account.id, notifications: false } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'does not remove the following relation between user and target user' do + expect(user.account.following?(other_account)).to be true + end + + it 'creates a muting relation' do + expect(user.account.muting?(other_account)).to be true + end + + it 'does not mute notifications' do + expect(user.account.muting_notifications?(other_account)).to be false + end end describe 'POST #unmute' do -- cgit From ad8e856a5b053096e32debdc12ce9c6d05924657 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 11 Sep 2017 23:50:37 +0200 Subject: Fix error when following locked accounts (#4896) (cherry picked from commit 0ef9d45d0581dddf2f325033c43721f42fcfca9e) --- app/controllers/api/v1/accounts_controller.rb | 15 ++------ .../controllers/api/v1/accounts_controller_spec.rb | 43 +++++++++++++++++----- 2 files changed, 37 insertions(+), 21 deletions(-) (limited to 'spec/controllers/api/v1/accounts_controller_spec.rb') diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index ec3abfbf5..4676f60de 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -15,16 +15,9 @@ class Api::V1::AccountsController < Api::BaseController def follow FollowService.new.call(current_user.account, @account.acct) - unless @account.locked? - relationships = AccountRelationshipsPresenter.new( - [@account.id], - current_user.account_id, - following_map: { @account.id => true }, - requested_map: { @account.id => false } - ) - end + options = @account.locked? ? {} : { following_map: { @account.id => true }, requested_map: { @account.id => false } } - render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships + render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) end def block @@ -58,7 +51,7 @@ class Api::V1::AccountsController < Api::BaseController @account = Account.find(params[:id]) end - def relationships - AccountRelationshipsPresenter.new([@account.id], current_user.account_id) + def relationships(options = {}) + AccountRelationshipsPresenter.new([@account.id], current_user.account_id, options) end end diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 96e8ecd43..053c53e5a 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -18,25 +18,48 @@ RSpec.describe Api::V1::AccountsController, type: :controller do end describe 'POST #follow' do - let(:other_account) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } + let(:other_account) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob', locked: locked)).account } before do post :follow, params: { id: other_account.id } end - it 'returns http success' do - expect(response).to have_http_status(:success) - end + context 'with unlocked account' do + let(:locked) { false } + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'returns JSON with following=true and requested=false' do + json = body_as_json - it 'returns JSON with following=true and requested=false' do - json = body_as_json + expect(json[:following]).to be true + expect(json[:requested]).to be false + end - expect(json[:following]).to be true - expect(json[:requested]).to be false + it 'creates a following relation between user and target user' do + expect(user.account.following?(other_account)).to be true + end end - it 'creates a following relation between user and target user' do - expect(user.account.following?(other_account)).to be true + context 'with locked account' do + let(:locked) { true } + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'returns JSON with following=false and requested=true' do + json = body_as_json + + expect(json[:following]).to be false + expect(json[:requested]).to be true + end + + it 'creates a follow request relation between user and target user' do + expect(user.account.requested?(other_account)).to be true + end end end -- cgit From b95c48748cd4e7a1181cdf3f17e23d6e526a9d95 Mon Sep 17 00:00:00 2001 From: aschmitz Date: Fri, 10 Nov 2017 20:11:10 -0600 Subject: 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). --- app/models/concerns/account_interactions.rb | 6 ++- app/models/follow_request.rb | 2 +- app/services/follow_service.rb | 2 +- app/services/notify_service.rb | 2 +- .../20171028221157_add_reblogs_to_follows.rb | 21 ++++++++ .../v1/accounts/relationships_controller_spec.rb | 4 +- .../controllers/api/v1/accounts_controller_spec.rb | 8 +-- spec/models/concerns/account_interactions_spec.rb | 37 ++++++++++++++ spec/models/follow_request_spec.rb | 24 ++++++++- spec/services/follow_service_spec.rb | 58 ++++++++++++++++++++-- spec/services/notify_service_spec.rb | 20 ++++++++ 11 files changed, 170 insertions(+), 14 deletions(-) create mode 100644 db/migrate/20171028221157_add_reblogs_to_follows.rb (limited to 'spec/controllers/api/v1/accounts_controller_spec.rb') diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 088fef4da..60fd6ded5 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -81,7 +81,7 @@ module AccountInteractions rel.show_reblogs = reblogs rel.save! end - + rel end @@ -156,6 +156,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 diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index 0608ffabc..1a1c52382 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -22,7 +22,7 @@ class FollowRequest < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } def authorize! - account.follow!(target_account, reblogs: reblogs) + account.follow!(target_account, reblogs: show_reblogs) MergeWorker.perform_async(target_account.id, account.id) destroy! diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 70572110d..6db591999 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -39,7 +39,7 @@ class FollowService < BaseService private 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? NotifyService.new.call(target_account, follow_request) diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index fb09df983..3fa3f152c 100644 --- a/app/services/notify_service.rb +++ b/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? diff --git a/db/migrate/20171028221157_add_reblogs_to_follows.rb b/db/migrate/20171028221157_add_reblogs_to_follows.rb new file mode 100644 index 000000000..eb4640a20 --- /dev/null +++ b/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 diff --git a/spec/controllers/api/v1/accounts/relationships_controller_spec.rb b/spec/controllers/api/v1/accounts/relationships_controller_spec.rb index 431fc2194..f25b86ac1 100644 --- a/spec/controllers/api/v1/accounts/relationships_controller_spec.rb +++ b/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 diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 053c53e5a..f3b879421 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/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 diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index ef957fc1d..f47d9d057 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/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 diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index cc6f8ee62..62bd724d7 100644 --- a/spec/models/follow_request_spec.rb +++ b/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 diff --git a/spec/services/follow_service_spec.rb b/spec/services/follow_service_spec.rb index ceb39e5e6..e59a2f1a6 100644 --- a/spec/services/follow_service_spec.rb +++ b/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 diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 7088ae9d1..250a880a2 100644 --- a/spec/services/notify_service_spec.rb +++ b/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) } -- cgit