Fix poll updates being saved as status edits (#17373)

Fix #17344

app/services/activitypub/process_status_update_service.rb

@@ -10,6 +10,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
     @status                    = status
     @account                   = status.account
     @media_attachments_changed = false
+    @poll_changed              = false
 
     # Only native types can be updated at the moment
     return if !expected_type? || already_updated_more_recently?
@@ -27,6 +28,9 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
         end
 
         queue_poll_notifications!
+
+        next unless significant_changes?
+
         reset_preview_card!
         broadcast_updates!
       else
@@ -92,7 +96,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
       # If for some reasons the options were changed, it invalidates all previous
       # votes, so we need to remove them
       if poll_parser.significantly_changes?(poll)
-        @media_attachments_changed = true
+        @poll_changed = true
         poll.votes.delete_all unless poll.new_record?
       end
 
@@ -107,7 +111,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
       @status.poll_id = poll.id
     elsif previous_poll.present?
       previous_poll.destroy!
-      @media_attachments_changed = true
+      @poll_changed = true
       @status.poll_id = nil
     end
   end
@@ -117,7 +121,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
     @status.spoiler_text = @status_parser.spoiler_text || ''
     @status.sensitive    = @account.sensitized? || @status_parser.sensitive || false
     @status.language     = @status_parser.language || detected_language
-    @status.edited_at    = @status_parser.edited_at || Time.now.utc
+    @status.edited_at    = @status_parser.edited_at || Time.now.utc if significant_changes?
 
     @status.save!
   end
@@ -227,12 +231,12 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
   end
 
   def create_edit!
-    return unless @status.text_previously_changed? || @status.spoiler_text_previously_changed? || @media_attachments_changed
+    return unless significant_changes?
 
     @status_edit = @status.edits.create(
       text: @status.text,
       spoiler_text: @status.spoiler_text,
-      media_attachments_changed: @media_attachments_changed,
+      media_attachments_changed: @media_attachments_changed || @poll_changed,
       account_id: @account.id,
       created_at: @status.edited_at
     )
@@ -248,13 +252,17 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
     mime_type.present? && !MediaAttachment.supported_mime_types.include?(mime_type)
   end
 
+  def significant_changes?
+    @status.text_changed? || @status.text_previously_changed? || @status.spoiler_text_changed? || @status.spoiler_text_previously_changed? || @media_attachments_changed || @poll_changed
+  end
+
   def already_updated_more_recently?
     @status.edited_at.present? && @status_parser.edited_at.present? && @status.edited_at > @status_parser.edited_at
   end
 
   def reset_preview_card!
-    @status.preview_cards.clear if @status.text_previously_changed? || @status.spoiler_text.present?
+    @status.preview_cards.clear
-    LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id) if @status.spoiler_text.blank?
+    LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id)
   end
 
   def broadcast_updates!

spec/lib/activitypub/activity/update_spec.rb

@@ -4,43 +4,97 @@ RSpec.describe ActivityPub::Activity::Update do
   let!(:sender) { Fabricate(:account) }
 
   before do
-    stub_request(:get, actor_json[:outbox]).to_return(status: 404)
-    stub_request(:get, actor_json[:followers]).to_return(status: 404)
-    stub_request(:get, actor_json[:following]).to_return(status: 404)
-    stub_request(:get, actor_json[:featured]).to_return(status: 404)
-
     sender.update!(uri: ActivityPub::TagManager.instance.uri_for(sender))
   end
 
-  let(:modified_sender) do
+  subject { described_class.new(json, sender) }
-    sender.tap do |modified_sender|
-      modified_sender.display_name = 'Totally modified now'
-    end
-  end
-
-  let(:actor_json) do
-    ActiveModelSerializers::SerializableResource.new(modified_sender, serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter).as_json
-  end
-
-  let(:json) do
-    {
-      '@context': 'https://www.w3.org/ns/activitystreams',
-      id: 'foo',
-      type: 'Update',
-      actor: ActivityPub::TagManager.instance.uri_for(sender),
-      object: actor_json,
-    }.with_indifferent_access
-  end
 
   describe '#perform' do
-    subject { described_class.new(json, sender) }
+    context 'with an Actor object' do
+      let(:modified_sender) do
+        sender.tap do |modified_sender|
+          modified_sender.display_name = 'Totally modified now'
+        end
+      end
 
-    before do
+      let(:actor_json) do
-      subject.perform
+        ActiveModelSerializers::SerializableResource.new(modified_sender, serializer: ActivityPub::ActorSerializer, adapter: ActivityPub::Adapter).as_json
+      end
+
+      let(:json) do
+        {
+          '@context': 'https://www.w3.org/ns/activitystreams',
+          id: 'foo',
+          type: 'Update',
+          actor: ActivityPub::TagManager.instance.uri_for(sender),
+          object: actor_json,
+        }.with_indifferent_access
+      end
+
+      before do
+        stub_request(:get, actor_json[:outbox]).to_return(status: 404)
+        stub_request(:get, actor_json[:followers]).to_return(status: 404)
+        stub_request(:get, actor_json[:following]).to_return(status: 404)
+        stub_request(:get, actor_json[:featured]).to_return(status: 404)
+
+        subject.perform
+      end
+
+      it 'updates profile' do
+        expect(sender.reload.display_name).to eq 'Totally modified now'
+      end
     end
 
-    it 'updates profile' do
+    context 'with a Question object' do
-      expect(sender.reload.display_name).to eq 'Totally modified now'
+      let!(:at_time) { Time.now.utc }
+      let!(:status) { Fabricate(:status, account: sender, poll: Poll.new(account: sender, options: %w(Bar Baz), cached_tallies: [0, 0], expires_at: at_time + 5.days)) }
+
+      let(:json) do
+        {
+          '@context': 'https://www.w3.org/ns/activitystreams',
+          id: 'foo',
+          type: 'Update',
+          actor: ActivityPub::TagManager.instance.uri_for(sender),
+          object: {
+            type: 'Question',
+            id: ActivityPub::TagManager.instance.uri_for(status),
+            content: 'Foo',
+            endTime: (at_time + 5.days).iso8601,
+            oneOf: [
+              {
+                type: 'Note',
+                name: 'Bar',
+                replies: {
+                  type: 'Collection',
+                  totalItems: 0,
+                },
+              },
+
+              {
+                type: 'Note',
+                name: 'Baz',
+                replies: {
+                  type: 'Collection',
+                  totalItems: 12,
+                },
+              },
+            ],
+          },
+        }.with_indifferent_access
+      end
+
+      before do
+        status.update!(uri: ActivityPub::TagManager.instance.uri_for(status))
+        subject.perform
+      end
+
+      it 'updates poll numbers' do
+        expect(status.preloadable_poll.cached_tallies).to eq [0, 12]
+      end
+
+      it 'does not set status as edited' do
+        expect(status.edited_at).to be_nil
+      end
     end
   end
 end