Refactor how public and tag timelines are queried (#14728)
This commit is contained in:
		
							
								
								
									
										212
									
								
								spec/models/public_feed_spec.rb
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										212
									
								
								spec/models/public_feed_spec.rb
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,212 @@ | ||||
| require 'rails_helper' | ||||
|  | ||||
| RSpec.describe PublicFeed, type: :model do | ||||
|   let(:account) { Fabricate(:account) } | ||||
|  | ||||
|   describe '#get' do | ||||
|     subject { described_class.new(nil).get(20).map(&:id) } | ||||
|  | ||||
|     it 'only includes statuses with public visibility' do | ||||
|       public_status = Fabricate(:status, visibility: :public) | ||||
|       private_status = Fabricate(:status, visibility: :private) | ||||
|  | ||||
|       expect(subject).to include(public_status.id) | ||||
|       expect(subject).not_to include(private_status.id) | ||||
|     end | ||||
|  | ||||
|     it 'does not include replies' do | ||||
|       status = Fabricate(:status) | ||||
|       reply = Fabricate(:status, in_reply_to_id: status.id) | ||||
|  | ||||
|       expect(subject).to include(status.id) | ||||
|       expect(subject).not_to include(reply.id) | ||||
|     end | ||||
|  | ||||
|     it 'does not include boosts' do | ||||
|       status = Fabricate(:status) | ||||
|       boost = Fabricate(:status, reblog_of_id: status.id) | ||||
|  | ||||
|       expect(subject).to include(status.id) | ||||
|       expect(subject).not_to include(boost.id) | ||||
|     end | ||||
|  | ||||
|     it 'filters out silenced accounts' do | ||||
|       account = Fabricate(:account) | ||||
|       silenced_account = Fabricate(:account, silenced: true) | ||||
|       status = Fabricate(:status, account: account) | ||||
|       silenced_status = Fabricate(:status, account: silenced_account) | ||||
|  | ||||
|       expect(subject).to include(status.id) | ||||
|       expect(subject).not_to include(silenced_status.id) | ||||
|     end | ||||
|  | ||||
|     context 'without local_only option' do | ||||
|       let(:viewer) { nil } | ||||
|  | ||||
|       let!(:local_account)  { Fabricate(:account, domain: nil) } | ||||
|       let!(:remote_account) { Fabricate(:account, domain: 'test.com') } | ||||
|       let!(:local_status)   { Fabricate(:status, account: local_account) } | ||||
|       let!(:remote_status)  { Fabricate(:status, account: remote_account) } | ||||
|  | ||||
|       subject { described_class.new(viewer).get(20).map(&:id) } | ||||
|  | ||||
|       context 'without a viewer' do | ||||
|         let(:viewer) { nil } | ||||
|  | ||||
|         it 'includes remote instances statuses' do | ||||
|           expect(subject).to include(remote_status.id) | ||||
|         end | ||||
|  | ||||
|         it 'includes local statuses' do | ||||
|           expect(subject).to include(local_status.id) | ||||
|         end | ||||
|       end | ||||
|  | ||||
|       context 'with a viewer' do | ||||
|         let(:viewer) { Fabricate(:account, username: 'viewer') } | ||||
|  | ||||
|         it 'includes remote instances statuses' do | ||||
|           expect(subject).to include(remote_status.id) | ||||
|         end | ||||
|  | ||||
|         it 'includes local statuses' do | ||||
|           expect(subject).to include(local_status.id) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     context 'with a local_only option set' do | ||||
|       let!(:local_account)  { Fabricate(:account, domain: nil) } | ||||
|       let!(:remote_account) { Fabricate(:account, domain: 'test.com') } | ||||
|       let!(:local_status)   { Fabricate(:status, account: local_account) } | ||||
|       let!(:remote_status)  { Fabricate(:status, account: remote_account) } | ||||
|  | ||||
|       subject { described_class.new(viewer, local: true).get(20).map(&:id) } | ||||
|  | ||||
|       context 'without a viewer' do | ||||
|         let(:viewer) { nil } | ||||
|  | ||||
|         it 'does not include remote instances statuses' do | ||||
|           expect(subject).to include(local_status.id) | ||||
|           expect(subject).not_to include(remote_status.id) | ||||
|         end | ||||
|       end | ||||
|  | ||||
|       context 'with a viewer' do | ||||
|         let(:viewer) { Fabricate(:account, username: 'viewer') } | ||||
|  | ||||
|         it 'does not include remote instances statuses' do | ||||
|           expect(subject).to include(local_status.id) | ||||
|           expect(subject).not_to include(remote_status.id) | ||||
|         end | ||||
|  | ||||
|         it 'is not affected by personal domain blocks' do | ||||
|           viewer.block_domain!('test.com') | ||||
|           expect(subject).to include(local_status.id) | ||||
|           expect(subject).not_to include(remote_status.id) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     context 'with a remote_only option set' do | ||||
|       let!(:local_account)  { Fabricate(:account, domain: nil) } | ||||
|       let!(:remote_account) { Fabricate(:account, domain: 'test.com') } | ||||
|       let!(:local_status)   { Fabricate(:status, account: local_account) } | ||||
|       let!(:remote_status)  { Fabricate(:status, account: remote_account) } | ||||
|  | ||||
|       subject { described_class.new(viewer, remote: true).get(20).map(&:id) } | ||||
|  | ||||
|       context 'without a viewer' do | ||||
|         let(:viewer) { nil } | ||||
|  | ||||
|         it 'does not include local instances statuses' do | ||||
|           expect(subject).not_to include(local_status.id) | ||||
|           expect(subject).to include(remote_status.id) | ||||
|         end | ||||
|       end | ||||
|  | ||||
|       context 'with a viewer' do | ||||
|         let(:viewer) { Fabricate(:account, username: 'viewer') } | ||||
|  | ||||
|         it 'does not include local instances statuses' do | ||||
|           expect(subject).not_to include(local_status.id) | ||||
|           expect(subject).to include(remote_status.id) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     describe 'with an account passed in' do | ||||
|       before do | ||||
|         @account = Fabricate(:account) | ||||
|       end | ||||
|  | ||||
|       subject { described_class.new(@account).get(20).map(&:id) } | ||||
|  | ||||
|       it 'excludes statuses from accounts blocked by the account' do | ||||
|         blocked = Fabricate(:account) | ||||
|         @account.block!(blocked) | ||||
|         blocked_status = Fabricate(:status, account: blocked) | ||||
|  | ||||
|         expect(subject).not_to include(blocked_status.id) | ||||
|       end | ||||
|  | ||||
|       it 'excludes statuses from accounts who have blocked the account' do | ||||
|         blocker = Fabricate(:account) | ||||
|         blocker.block!(@account) | ||||
|         blocked_status = Fabricate(:status, account: blocker) | ||||
|  | ||||
|         expect(subject).not_to include(blocked_status.id) | ||||
|       end | ||||
|  | ||||
|       it 'excludes statuses from accounts muted by the account' do | ||||
|         muted = Fabricate(:account) | ||||
|         @account.mute!(muted) | ||||
|         muted_status = Fabricate(:status, account: muted) | ||||
|  | ||||
|         expect(subject).not_to include(muted_status.id) | ||||
|       end | ||||
|  | ||||
|       it 'excludes statuses from accounts from personally blocked domains' do | ||||
|         blocked = Fabricate(:account, domain: 'example.com') | ||||
|         @account.block_domain!(blocked.domain) | ||||
|         blocked_status = Fabricate(:status, account: blocked) | ||||
|  | ||||
|         expect(subject).not_to include(blocked_status.id) | ||||
|       end | ||||
|  | ||||
|       context 'with language preferences' do | ||||
|         it 'excludes statuses in languages not allowed by the account user' do | ||||
|           user = Fabricate(:user, chosen_languages: [:en, :es]) | ||||
|           @account.update(user: user) | ||||
|           en_status = Fabricate(:status, language: 'en') | ||||
|           es_status = Fabricate(:status, language: 'es') | ||||
|           fr_status = Fabricate(:status, language: 'fr') | ||||
|  | ||||
|           expect(subject).to include(en_status.id) | ||||
|           expect(subject).to include(es_status.id) | ||||
|           expect(subject).not_to include(fr_status.id) | ||||
|         end | ||||
|  | ||||
|         it 'includes all languages when user does not have a setting' do | ||||
|           user = Fabricate(:user, chosen_languages: nil) | ||||
|           @account.update(user: user) | ||||
|  | ||||
|           en_status = Fabricate(:status, language: 'en') | ||||
|           es_status = Fabricate(:status, language: 'es') | ||||
|  | ||||
|           expect(subject).to include(en_status.id) | ||||
|           expect(subject).to include(es_status.id) | ||||
|         end | ||||
|  | ||||
|         it 'includes all languages when account does not have a user' do | ||||
|           expect(@account.user).to be_nil | ||||
|           en_status = Fabricate(:status, language: 'en') | ||||
|           es_status = Fabricate(:status, language: 'es') | ||||
|  | ||||
|           expect(subject).to include(en_status.id) | ||||
|           expect(subject).to include(es_status.id) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
| @ -267,241 +267,6 @@ RSpec.describe Status, type: :model do | ||||
|     end | ||||
|   end | ||||
|  | ||||
|   describe '.as_public_timeline' do | ||||
|     it 'only includes statuses with public visibility' do | ||||
|       public_status = Fabricate(:status, visibility: :public) | ||||
|       private_status = Fabricate(:status, visibility: :private) | ||||
|  | ||||
|       results = Status.as_public_timeline | ||||
|       expect(results).to include(public_status) | ||||
|       expect(results).not_to include(private_status) | ||||
|     end | ||||
|  | ||||
|     it 'does not include replies' do | ||||
|       status = Fabricate(:status) | ||||
|       reply = Fabricate(:status, in_reply_to_id: status.id) | ||||
|  | ||||
|       results = Status.as_public_timeline | ||||
|       expect(results).to include(status) | ||||
|       expect(results).not_to include(reply) | ||||
|     end | ||||
|  | ||||
|     it 'does not include boosts' do | ||||
|       status = Fabricate(:status) | ||||
|       boost = Fabricate(:status, reblog_of_id: status.id) | ||||
|  | ||||
|       results = Status.as_public_timeline | ||||
|       expect(results).to include(status) | ||||
|       expect(results).not_to include(boost) | ||||
|     end | ||||
|  | ||||
|     it 'filters out silenced accounts' do | ||||
|       account = Fabricate(:account) | ||||
|       silenced_account = Fabricate(:account, silenced: true) | ||||
|       status = Fabricate(:status, account: account) | ||||
|       silenced_status = Fabricate(:status, account: silenced_account) | ||||
|  | ||||
|       results = Status.as_public_timeline | ||||
|       expect(results).to include(status) | ||||
|       expect(results).not_to include(silenced_status) | ||||
|     end | ||||
|  | ||||
|     context 'without local_only option' do | ||||
|       let(:viewer) { nil } | ||||
|  | ||||
|       let!(:local_account)  { Fabricate(:account, domain: nil) } | ||||
|       let!(:remote_account) { Fabricate(:account, domain: 'test.com') } | ||||
|       let!(:local_status)   { Fabricate(:status, account: local_account) } | ||||
|       let!(:remote_status)  { Fabricate(:status, account: remote_account) } | ||||
|  | ||||
|       subject { Status.as_public_timeline(viewer, false) } | ||||
|  | ||||
|       context 'without a viewer' do | ||||
|         let(:viewer) { nil } | ||||
|  | ||||
|         it 'includes remote instances statuses' do | ||||
|           expect(subject).to include(remote_status) | ||||
|         end | ||||
|  | ||||
|         it 'includes local statuses' do | ||||
|           expect(subject).to include(local_status) | ||||
|         end | ||||
|       end | ||||
|  | ||||
|       context 'with a viewer' do | ||||
|         let(:viewer) { Fabricate(:account, username: 'viewer') } | ||||
|  | ||||
|         it 'includes remote instances statuses' do | ||||
|           expect(subject).to include(remote_status) | ||||
|         end | ||||
|  | ||||
|         it 'includes local statuses' do | ||||
|           expect(subject).to include(local_status) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     context 'with a local_only option set' do | ||||
|       let!(:local_account)  { Fabricate(:account, domain: nil) } | ||||
|       let!(:remote_account) { Fabricate(:account, domain: 'test.com') } | ||||
|       let!(:local_status)   { Fabricate(:status, account: local_account) } | ||||
|       let!(:remote_status)  { Fabricate(:status, account: remote_account) } | ||||
|  | ||||
|       subject { Status.as_public_timeline(viewer, true) } | ||||
|  | ||||
|       context 'without a viewer' do | ||||
|         let(:viewer) { nil } | ||||
|  | ||||
|         it 'does not include remote instances statuses' do | ||||
|           expect(subject).to include(local_status) | ||||
|           expect(subject).not_to include(remote_status) | ||||
|         end | ||||
|       end | ||||
|  | ||||
|       context 'with a viewer' do | ||||
|         let(:viewer) { Fabricate(:account, username: 'viewer') } | ||||
|  | ||||
|         it 'does not include remote instances statuses' do | ||||
|           expect(subject).to include(local_status) | ||||
|           expect(subject).not_to include(remote_status) | ||||
|         end | ||||
|  | ||||
|         it 'is not affected by personal domain blocks' do | ||||
|           viewer.block_domain!('test.com') | ||||
|           expect(subject).to include(local_status) | ||||
|           expect(subject).not_to include(remote_status) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     context 'with a remote_only option set' do | ||||
|       let!(:local_account)  { Fabricate(:account, domain: nil) } | ||||
|       let!(:remote_account) { Fabricate(:account, domain: 'test.com') } | ||||
|       let!(:local_status)   { Fabricate(:status, account: local_account) } | ||||
|       let!(:remote_status)  { Fabricate(:status, account: remote_account) } | ||||
|  | ||||
|       subject { Status.as_public_timeline(viewer, :remote) } | ||||
|  | ||||
|       context 'without a viewer' do | ||||
|         let(:viewer) { nil } | ||||
|  | ||||
|         it 'does not include local instances statuses' do | ||||
|           expect(subject).not_to include(local_status) | ||||
|           expect(subject).to include(remote_status) | ||||
|         end | ||||
|       end | ||||
|  | ||||
|       context 'with a viewer' do | ||||
|         let(:viewer) { Fabricate(:account, username: 'viewer') } | ||||
|  | ||||
|         it 'does not include local instances statuses' do | ||||
|           expect(subject).not_to include(local_status) | ||||
|           expect(subject).to include(remote_status) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | ||||
|     describe 'with an account passed in' do | ||||
|       before do | ||||
|         @account = Fabricate(:account) | ||||
|       end | ||||
|  | ||||
|       it 'excludes statuses from accounts blocked by the account' do | ||||
|         blocked = Fabricate(:account) | ||||
|         Fabricate(:block, account: @account, target_account: blocked) | ||||
|         blocked_status = Fabricate(:status, account: blocked) | ||||
|  | ||||
|         results = Status.as_public_timeline(@account) | ||||
|         expect(results).not_to include(blocked_status) | ||||
|       end | ||||
|  | ||||
|       it 'excludes statuses from accounts who have blocked the account' do | ||||
|         blocked = Fabricate(:account) | ||||
|         Fabricate(:block, account: blocked, target_account: @account) | ||||
|         blocked_status = Fabricate(:status, account: blocked) | ||||
|  | ||||
|         results = Status.as_public_timeline(@account) | ||||
|         expect(results).not_to include(blocked_status) | ||||
|       end | ||||
|  | ||||
|       it 'excludes statuses from accounts muted by the account' do | ||||
|         muted = Fabricate(:account) | ||||
|         Fabricate(:mute, account: @account, target_account: muted) | ||||
|         muted_status = Fabricate(:status, account: muted) | ||||
|  | ||||
|         results = Status.as_public_timeline(@account) | ||||
|         expect(results).not_to include(muted_status) | ||||
|       end | ||||
|  | ||||
|       it 'excludes statuses from accounts from personally blocked domains' do | ||||
|         blocked = Fabricate(:account, domain: 'example.com') | ||||
|         @account.block_domain!(blocked.domain) | ||||
|         blocked_status = Fabricate(:status, account: blocked) | ||||
|  | ||||
|         results = Status.as_public_timeline(@account) | ||||
|         expect(results).not_to include(blocked_status) | ||||
|       end | ||||
|  | ||||
|       context 'with language preferences' do | ||||
|         it 'excludes statuses in languages not allowed by the account user' do | ||||
|           user = Fabricate(:user, chosen_languages: [:en, :es]) | ||||
|           @account.update(user: user) | ||||
|           en_status = Fabricate(:status, language: 'en') | ||||
|           es_status = Fabricate(:status, language: 'es') | ||||
|           fr_status = Fabricate(:status, language: 'fr') | ||||
|  | ||||
|           results = Status.as_public_timeline(@account) | ||||
|           expect(results).to include(en_status) | ||||
|           expect(results).to include(es_status) | ||||
|           expect(results).not_to include(fr_status) | ||||
|         end | ||||
|  | ||||
|         it 'includes all languages when user does not have a setting' do | ||||
|           user = Fabricate(:user, chosen_languages: nil) | ||||
|           @account.update(user: user) | ||||
|  | ||||
|           en_status = Fabricate(:status, language: 'en') | ||||
|           es_status = Fabricate(:status, language: 'es') | ||||
|  | ||||
|           results = Status.as_public_timeline(@account) | ||||
|           expect(results).to include(en_status) | ||||
|           expect(results).to include(es_status) | ||||
|         end | ||||
|  | ||||
|         it 'includes all languages when account does not have a user' do | ||||
|           expect(@account.user).to be_nil | ||||
|           en_status = Fabricate(:status, language: 'en') | ||||
|           es_status = Fabricate(:status, language: 'es') | ||||
|  | ||||
|           results = Status.as_public_timeline(@account) | ||||
|           expect(results).to include(en_status) | ||||
|           expect(results).to include(es_status) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  | ||||
|   describe '.as_tag_timeline' do | ||||
|     it 'includes statuses with a tag' do | ||||
|       tag = Fabricate(:tag) | ||||
|       status = Fabricate(:status, tags: [tag]) | ||||
|       other = Fabricate(:status) | ||||
|  | ||||
|       results = Status.as_tag_timeline(tag) | ||||
|       expect(results).to include(status) | ||||
|       expect(results).not_to include(other) | ||||
|     end | ||||
|  | ||||
|     it 'allows replies to be included' do | ||||
|       original = Fabricate(:status) | ||||
|       tag = Fabricate(:tag) | ||||
|       status = Fabricate(:status, tags: [tag], in_reply_to_id: original.id) | ||||
|  | ||||
|       results = Status.as_tag_timeline(tag) | ||||
|       expect(results).to include(status) | ||||
|     end | ||||
|   end | ||||
|  | ||||
|   describe '.permitted_for' do | ||||
|     subject { described_class.permitted_for(target_account, account).pluck(:visibility) } | ||||
|  | ||||
|  | ||||
| @ -1,7 +1,7 @@ | ||||
| require 'rails_helper' | ||||
| 
 | ||||
| describe HashtagQueryService, type: :service do | ||||
|   describe '.call' do | ||||
| describe TagFeed, type: :service do | ||||
|   describe '#get' do | ||||
|     let(:account) { Fabricate(:account) } | ||||
|     let(:tag1) { Fabricate(:tag) } | ||||
|     let(:tag2) { Fabricate(:tag) } | ||||
| @ -10,35 +10,35 @@ describe HashtagQueryService, type: :service do | ||||
|     let!(:both) { Fabricate(:status, tags: [tag1, tag2]) } | ||||
| 
 | ||||
|     it 'can add tags in "any" mode' do | ||||
|       results = subject.call(tag1, { any: [tag2.name] }) | ||||
|       results = described_class.new(tag1, nil, any: [tag2.name]).get(20) | ||||
|       expect(results).to include status1 | ||||
|       expect(results).to include status2 | ||||
|       expect(results).to include both | ||||
|     end | ||||
| 
 | ||||
|     it 'can remove tags in "all" mode' do | ||||
|       results = subject.call(tag1, { all: [tag2.name] }) | ||||
|       results = described_class.new(tag1, nil, all: [tag2.name]).get(20) | ||||
|       expect(results).to_not include status1 | ||||
|       expect(results).to_not include status2 | ||||
|       expect(results).to     include both | ||||
|     end | ||||
| 
 | ||||
|     it 'can remove tags in "none" mode' do | ||||
|       results = subject.call(tag1, { none: [tag2.name] }) | ||||
|       results = described_class.new(tag1, nil, none: [tag2.name]).get(20) | ||||
|       expect(results).to     include status1 | ||||
|       expect(results).to_not include status2 | ||||
|       expect(results).to_not include both | ||||
|     end | ||||
| 
 | ||||
|     it 'ignores an invalid mode' do | ||||
|       results = subject.call(tag1, { wark: [tag2.name] }) | ||||
|       results = described_class.new(tag1, nil, wark: [tag2.name]).get(20) | ||||
|       expect(results).to     include status1 | ||||
|       expect(results).to_not include status2 | ||||
|       expect(results).to     include both | ||||
|     end | ||||
| 
 | ||||
|     it 'handles being passed non existant tag names' do | ||||
|       results = subject.call(tag1, { any: ['wark'] }) | ||||
|       results = described_class.new(tag1, nil, any: ['wark']).get(20) | ||||
|       expect(results).to     include status1 | ||||
|       expect(results).to_not include status2 | ||||
|       expect(results).to     include both | ||||
| @ -46,15 +46,23 @@ describe HashtagQueryService, type: :service do | ||||
| 
 | ||||
|     it 'can restrict to an account' do | ||||
|       BlockService.new.call(account, status1.account) | ||||
|       results = subject.call(tag1, { none: [tag2.name] }, account) | ||||
|       results = described_class.new(tag1, account, none: [tag2.name]).get(20) | ||||
|       expect(results).to_not include status1 | ||||
|     end | ||||
| 
 | ||||
|     it 'can restrict to local' do | ||||
|       status1.account.update(domain: 'example.com') | ||||
|       status1.update(local: false, uri: 'example.com/toot') | ||||
|       results = subject.call(tag1, { any: [tag2.name] }, nil, true) | ||||
|       results = described_class.new(tag1, nil, any: [tag2.name], local: true).get(20) | ||||
|       expect(results).to_not include status1 | ||||
|     end | ||||
| 
 | ||||
|     it 'allows replies to be included' do | ||||
|       original = Fabricate(:status) | ||||
|       status = Fabricate(:status, tags: [tag1], in_reply_to_id: original.id) | ||||
| 
 | ||||
|       results = described_class.new(tag1, nil).get(20) | ||||
|       expect(results).to include(status) | ||||
|     end | ||||
|   end | ||||
| end | ||||
| @ -28,10 +28,10 @@ RSpec.describe FanOutOnWriteService, type: :service do | ||||
|   end | ||||
|  | ||||
|   it 'delivers status to hashtag' do | ||||
|     expect(Tag.find_by!(name: 'test').statuses.pluck(:id)).to include status.id | ||||
|     expect(TagFeed.new(Tag.find_by(name: 'test'), alice).get(20).map(&:id)).to include status.id | ||||
|   end | ||||
|  | ||||
|   it 'delivers status to public timeline' do | ||||
|     expect(Status.as_public_timeline(alice).map(&:id)).to include status.id | ||||
|     expect(PublicFeed.new(alice).get(20).map(&:id)).to include status.id | ||||
|   end | ||||
| end | ||||
|  | ||||
		Reference in New Issue
	
	Block a user