From 974b1b79ce58e6799e5e5bb576e630ca783150de Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 18 Sep 2020 17:26:45 +0200 Subject: Add option to be notified when a followed user posts (#13546) * Add bell button Fix #4890 * Remove duplicate type from post-deployment migration * Fix legacy class type mappings * Improve query performance with better index * Fix validation * Remove redundant index from notifications --- .../controllers/api/v1/accounts_controller_spec.rb | 84 +++++++++++++++------- spec/models/concerns/account_interactions_spec.rb | 2 +- spec/models/follow_request_spec.rb | 2 +- spec/services/import_service_spec.rb | 1 + spec/services/notify_service_spec.rb | 6 +- spec/workers/refollow_worker_spec.rb | 4 +- 6 files changed, 67 insertions(+), 32 deletions(-) (limited to 'spec') diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 024409dab..1e656503f 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -71,50 +71,80 @@ RSpec.describe Api::V1::AccountsController, type: :controller do let(:scopes) { 'write:follows' } 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 + context do + before do + post :follow, params: { id: other_account.id } + end - context 'with unlocked account' do - let(:locked) { false } + context 'with unlocked account' do + let(:locked) { false } - it 'returns http success' do - expect(response).to have_http_status(200) - end + it 'returns http success' do + expect(response).to have_http_status(200) + 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 + end + + it 'creates a following relation between user and target user' do + expect(user.account.following?(other_account)).to be true + end - it 'creates a following relation between user and target user' do - expect(user.account.following?(other_account)).to be true + it_behaves_like 'forbidden for wrong scope', 'read:accounts' end - it_behaves_like 'forbidden for wrong scope', 'read:accounts' + context 'with locked account' do + let(:locked) { true } + + it 'returns http success' do + expect(response).to have_http_status(200) + 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 + + it_behaves_like 'forbidden for wrong scope', 'read:accounts' + end end - context 'with locked account' do - let(:locked) { true } + context 'modifying follow options' do + let(:locked) { false } - it 'returns http success' do - expect(response).to have_http_status(200) + before do + user.account.follow!(other_account, reblogs: false, notify: false) end - it 'returns JSON with following=false and requested=true' do + it 'changes reblogs option' do + post :follow, params: { id: other_account.id, reblogs: true } + json = body_as_json - expect(json[:following]).to be false - expect(json[:requested]).to be true + expect(json[:following]).to be true + expect(json[:showing_reblogs]).to be true + expect(json[:notifying]).to be false end - it 'creates a follow request relation between user and target user' do - expect(user.account.requested?(other_account)).to be true - end + it 'changes notify option' do + post :follow, params: { id: other_account.id, notify: true } + + json = body_as_json - it_behaves_like 'forbidden for wrong scope', 'read:accounts' + expect(json[:following]).to be true + expect(json[:showing_reblogs]).to be false + expect(json[:notifying]).to be true + end end end diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index e8ef61f66..f0380179c 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -14,7 +14,7 @@ describe AccountInteractions do context 'account with Follow' do it 'returns { target_account_id => true }' do Fabricate(:follow, account: account, target_account: target_account) - is_expected.to eq(target_account_id => { reblogs: true }) + is_expected.to eq(target_account_id => { reblogs: true, notify: false }) end end diff --git a/spec/models/follow_request_spec.rb b/spec/models/follow_request_spec.rb index 2cf28b263..cc484a5b9 100644 --- a/spec/models/follow_request_spec.rb +++ b/spec/models/follow_request_spec.rb @@ -7,7 +7,7 @@ RSpec.describe FollowRequest, type: :model do let(:target_account) { Fabricate(:account) } it 'calls Account#follow!, MergeWorker.perform_async, and #destroy!' do - expect(account).to receive(:follow!).with(target_account, reblogs: true, uri: follow_request.uri) + expect(account).to receive(:follow!).with(target_account, reblogs: true, notify: false, uri: follow_request.uri) expect(MergeWorker).to receive(:perform_async).with(target_account.id, account.id) expect(follow_request).to receive(:destroy!) follow_request.authorize! diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb index 7618e9076..b1909d4fd 100644 --- a/spec/services/import_service_spec.rb +++ b/spec/services/import_service_spec.rb @@ -95,6 +95,7 @@ RSpec.describe ImportService, type: :service do let(:import) { Import.create(account: account, type: 'following', data: csv) } it 'follows the listed accounts, including boosts' do subject.call(import) + expect(account.following.count).to eq 1 expect(account.follow_requests.count).to eq 1 expect(Follow.find_by(account: account, target_account: bob).show_reblogs).to be true diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 440018ac9..f2cb22c5e 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -2,13 +2,14 @@ require 'rails_helper' RSpec.describe NotifyService, type: :service do subject do - -> { described_class.new.call(recipient, activity) } + -> { described_class.new.call(recipient, type, activity) } end let(:user) { Fabricate(:user) } let(:recipient) { user.account } let(:sender) { Fabricate(:account, domain: 'example.com') } let(:activity) { Fabricate(:follow, account: sender, target_account: recipient) } + let(:type) { :follow } it { is_expected.to change(Notification, :count).by(1) } @@ -50,6 +51,7 @@ RSpec.describe NotifyService, type: :service do context 'for direct messages' do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct)) } + let(:type) { :mention } before do user.settings.interactions = user.settings.interactions.merge('must_be_following_dm' => enabled) @@ -93,6 +95,7 @@ RSpec.describe NotifyService, type: :service do describe 'reblogs' do let(:status) { Fabricate(:status, account: Fabricate(:account)) } let(:activity) { Fabricate(:status, account: sender, reblog: status) } + let(:type) { :reblog } it 'shows reblogs by default' do recipient.follow!(sender) @@ -114,6 +117,7 @@ RSpec.describe NotifyService, type: :service do let(:asshole) { Fabricate(:account, username: 'asshole') } let(:reply_to) { Fabricate(:status, account: asshole) } let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, thread: reply_to)) } + let(:type) { :mention } it 'does not notify when conversation is muted' do recipient.mute_conversation!(activity.status.conversation) diff --git a/spec/workers/refollow_worker_spec.rb b/spec/workers/refollow_worker_spec.rb index 29771aa59..6b4c04291 100644 --- a/spec/workers/refollow_worker_spec.rb +++ b/spec/workers/refollow_worker_spec.rb @@ -23,8 +23,8 @@ describe RefollowWorker do result = subject.perform(account.id) expect(result).to be_nil - expect(service).to have_received(:call).with(alice, account, reblogs: true) - expect(service).to have_received(:call).with(bob, account, reblogs: false) + expect(service).to have_received(:call).with(alice, account, reblogs: true, notify: false) + expect(service).to have_received(:call).with(bob, account, reblogs: false, notify: false) end end end -- cgit