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
This commit is contained in:
		@@ -11,7 +11,7 @@ class StatusCacheHydrator
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    # If we're delivering to the author who disabled the display of the application used to create the
 | 
					    # 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
 | 
					    # 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
 | 
					    # 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
 | 
					    # 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
 | 
					      # 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
 | 
					      # 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][: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?
 | 
					      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][:voted] = true
 | 
				
			||||||
          payload[:reblog][:poll][:own_votes] = []
 | 
					          payload[:reblog][:poll][:own_votes] = []
 | 
				
			||||||
        else
 | 
					        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][:voted] = !own_votes.empty?
 | 
				
			||||||
          payload[:reblog][:poll][:own_votes] = own_votes
 | 
					          payload[:reblog][:poll][:own_votes] = own_votes
 | 
				
			||||||
        end
 | 
					        end
 | 
				
			||||||
@@ -50,8 +50,13 @@ class StatusCacheHydrator
 | 
				
			|||||||
      payload[:reblogged]  = Status.where(account_id: account_id, reblog_of_id: @status.id).exists?
 | 
					      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[: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[: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 }
 | 
					      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
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    payload
 | 
					    payload
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -11,8 +11,20 @@ describe StatusCacheHydrator do
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    shared_examples 'shared behavior' do
 | 
					    shared_examples 'shared behavior' do
 | 
				
			||||||
      context 'when handling a new status' 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
 | 
					        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
 | 
				
			||||||
      end
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -26,7 +38,7 @@ describe StatusCacheHydrator do
 | 
				
			|||||||
          end
 | 
					          end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
          it 'renders the same attributes as a full render' do
 | 
					          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
 | 
					        end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -36,7 +48,7 @@ describe StatusCacheHydrator do
 | 
				
			|||||||
          end
 | 
					          end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
          it 'renders the same attributes as a full render' do
 | 
					          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
 | 
					        end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -48,7 +60,7 @@ describe StatusCacheHydrator do
 | 
				
			|||||||
          end
 | 
					          end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
          it 'renders the same attributes as a full render' do
 | 
					          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
 | 
					        end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -62,7 +74,7 @@ describe StatusCacheHydrator do
 | 
				
			|||||||
          end
 | 
					          end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
          it 'renders the same attributes as a full render' do
 | 
					          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
 | 
					        end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -71,7 +83,7 @@ describe StatusCacheHydrator do
 | 
				
			|||||||
          let(:reblog) { Fabricate(:status, poll: poll, account: account) }
 | 
					          let(:reblog) { Fabricate(:status, poll: poll, account: account) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
          it 'renders the same attributes as a full render' do
 | 
					          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
 | 
					        end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -84,7 +96,7 @@ describe StatusCacheHydrator do
 | 
				
			|||||||
          end
 | 
					          end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
          it 'renders the same attributes as a full render' do
 | 
					          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
 | 
					        end
 | 
				
			||||||
      end
 | 
					      end
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user