From bb89f83cc06b9665205c62db21163f2ce43d97ed Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 4 Nov 2022 20:01:33 +0100 Subject: [PATCH] Fix additional issues with status cache hydration (#19747) * Spare one SQL query when hydrating polls * Improve tests * Fix more discrepancies * Fix possible crash when the status has no application set --- app/lib/status_cache_hydrator.rb | 13 +++++++++---- spec/lib/status_cache_hydrator_spec.rb | 26 +++++++++++++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/app/lib/status_cache_hydrator.rb b/app/lib/status_cache_hydrator.rb index 552419fff..ffe813ee9 100644 --- a/app/lib/status_cache_hydrator.rb +++ b/app/lib/status_cache_hydrator.rb @@ -11,7 +11,7 @@ class StatusCacheHydrator # If we're delivering to the author who disabled the display of the application used to create the # status, we need to hydrate the application, since it was not rendered for the basic payload - payload[:application] = ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:application].nil? && @status.account_id == account_id + payload[:application] = ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:application].nil? && @status.account_id == account_id && @status.application.present? # We take advantage of the fact that some relationships can only occur with an original status, not # the reblog that wraps it, so we can assume that some values are always false @@ -23,7 +23,7 @@ class StatusCacheHydrator # If the reblogged status is being delivered to the author who disabled the display of the application # used to create the status, we need to hydrate it here too - payload[:reblog][:application] = ActiveModelSerializers::SerializableResource.new(@status.reblog.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id && @status.reblog.application_id.present? + payload[:reblog][:application] = ActiveModelSerializers::SerializableResource.new(@status.reblog.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id && @status.reblog.application.present? payload[:reblog][:favourited] = Favourite.where(account_id: account_id, status_id: @status.reblog_of_id).exists? payload[:reblog][:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.reblog_of_id).exists? @@ -37,7 +37,7 @@ class StatusCacheHydrator payload[:reblog][:poll][:voted] = true payload[:reblog][:poll][:own_votes] = [] else - own_votes = @status.reblog.poll.votes.where(account_id: account_id).pluck(:choice) + own_votes = PollVote.where(poll_id: @status.reblog.poll_id, account_id: account_id).pluck(:choice) payload[:reblog][:poll][:voted] = !own_votes.empty? payload[:reblog][:poll][:own_votes] = own_votes end @@ -50,8 +50,13 @@ class StatusCacheHydrator payload[:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.id).exists? payload[:muted] = ConversationMute.where(account_id: account_id, conversation_id: @status.conversation_id).exists? payload[:bookmarked] = Bookmark.where(account_id: account_id, status_id: @status.id).exists? - payload[:pinned] = StatusPin.where(account_id: account_id, status_id: @status.id).exists? + payload[:pinned] = StatusPin.where(account_id: account_id, status_id: @status.id).exists? if @status.account_id == account_id payload[:filtered] = CustomFilter.apply_cached_filters(CustomFilter.cached_filters_for(@status.id), @status).map { |filter| ActiveModelSerializers::SerializableResource.new(filter, serializer: REST::FilterResultSerializer).as_json } + + if payload[:poll] + payload[:poll][:voted] = @status.account_id == account_id + payload[:poll][:own_votes] = [] + end end payload diff --git a/spec/lib/status_cache_hydrator_spec.rb b/spec/lib/status_cache_hydrator_spec.rb index 873d58464..c9d8d0fe1 100644 --- a/spec/lib/status_cache_hydrator_spec.rb +++ b/spec/lib/status_cache_hydrator_spec.rb @@ -11,8 +11,20 @@ describe StatusCacheHydrator do shared_examples 'shared behavior' do context 'when handling a new status' do + let(:poll) { Fabricate(:poll) } + let(:status) { Fabricate(:status, poll: poll) } + it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) + end + end + + context 'when handling a new status with own poll' do + let(:poll) { Fabricate(:poll, account: account) } + let(:status) { Fabricate(:status, poll: poll, account: account) } + + it 'renders the same attributes as a full render' do + expect(subject).to eql(compare_to_hash) end end @@ -26,7 +38,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -36,7 +48,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -48,7 +60,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -62,7 +74,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -71,7 +83,7 @@ describe StatusCacheHydrator do let(:reblog) { Fabricate(:status, poll: poll, account: account) } it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -84,7 +96,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end end