Fix poll votes not being properly reset on poll change (#17498)

* Fix poll votes not being properly reset on poll change

* Fix and add tests

* Fix poll update handling when the number of options changes

diff --git a/app/models/poll.rb b/app/models/poll.rb
index 71b5e191f84bdf349208f20c8c110195a492cac4..ba08309a15c9bddd804552db65f7bcd9f0a8d4a9 100644 (file)
--- a/app/models/poll.rb
+++ b/app/models/poll.rb
@@ -83,6 +83,12 @@ class Poll < ApplicationRecord
     end
   end
 
+  def reset_votes!
+    self.cached_tallies = options.map { 0 }
+    self.votes_count = 0
+    votes.delete_all unless new_record?
+  end
+
   private
 
   def prepare_cached_tallies

diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb
index 5e38852fcbb8c98302f87a625528a971082518f5..7438a7c5360b22ef7ae0a5b633b92477b3fc3d95 100644 (file)
--- a/app/services/activitypub/process_status_update_service.rb
+++ b/app/services/activitypub/process_status_update_service.rb
@@ -95,10 +95,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)
-        @poll_changed = true
-        poll.votes.delete_all unless poll.new_record?
-      end
+      @poll_changed = true if poll_parser.significantly_changes?(poll)
 
       poll.last_fetched_at = Time.now.utc
       poll.options         = poll_parser.options
@@ -106,6 +103,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
       poll.expires_at      = poll_parser.expires_at
       poll.voters_count    = poll_parser.voters_count
       poll.cached_tallies  = poll_parser.cached_tallies
+      poll.reset_votes! if @poll_changed
       poll.save!
 
       @status.poll_id = poll.id

diff --git a/app/services/update_status_service.rb b/app/services/update_status_service.rb
index 68e73d3b46836c6609a5f29571e814f8a43ea2c1..238ef0755691b3a082fbdf04084ad2fe6b9b461f 100644 (file)
--- a/app/services/update_status_service.rb
+++ b/app/services/update_status_service.rb
@@ -73,15 +73,13 @@ class UpdateStatusService < BaseService
 
       # If for some reasons the options were changed, it invalidates all previous
       # votes, so we need to remove them
-      if @options[:poll][:options] != poll.options || ActiveModel::Type::Boolean.new.cast(@options[:poll][:multiple]) != poll.multiple
-        @poll_changed = true
-        poll.votes.delete_all unless poll.new_record?
-      end
+      @poll_changed = true if @options[:poll][:options] != poll.options || ActiveModel::Type::Boolean.new.cast(@options[:poll][:multiple]) != poll.multiple
 
       poll.options     = @options[:poll][:options]
       poll.hide_totals = @options[:poll][:hide_totals] || false
       poll.multiple    = @options[:poll][:multiple] || false
       poll.expires_in  = @options[:poll][:expires_in]
+      poll.reset_votes! if @poll_changed
       poll.save!
 
       @status.poll_id = poll.id

diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb
index 1d4b0a6be2e08da7b80d8d21550beff32ef17f12..fe1b60d24db175e5f3a3b77810a6061ffa8eae9c 100644 (file)
--- a/spec/services/update_status_service_spec.rb
+++ b/spec/services/update_status_service_spec.rb
@@ -71,17 +71,27 @@ RSpec.describe UpdateStatusService, type: :service do
   end
 
   context 'when poll changes' do
-    let!(:status) { Fabricate(:status, text: 'Foo') }
-    let!(:poll) { Fabricate(:poll, options: %w(Foo Bar)) }
+    let(:account) { Fabricate(:account) }
+    let!(:status) { Fabricate(:status, text: 'Foo', account: account, poll_attributes: {options: %w(Foo Bar), account: account, multiple: false, hide_totals: false, expires_at: 7.days.from_now }) }
+    let!(:poll)   { status.poll }
+    let!(:voter) { Fabricate(:account) }
 
     before do
       status.update(poll: poll)
-      subject.call(status, status.account_id, text: 'Foo', poll: { options: %w(Bar Baz), expires_in: 5.days.to_i })
+      VoteService.new.call(voter, poll, [0])
+      subject.call(status, status.account_id, text: 'Foo', poll: { options: %w(Bar Baz Foo), expires_in: 5.days.to_i })
     end
 
     it 'updates poll' do
-      expect(status.poll).to_not eq poll
-      expect(status.poll.options).to eq %w(Bar Baz)
+      poll = status.poll.reload
+      expect(poll.options).to eq %w(Bar Baz Foo)
+    end
+
+    it 'resets votes' do
+      poll = status.poll.reload
+      expect(poll.votes_count).to eq 0
+      expect(poll.votes.count).to eq 0
+      expect(poll.cached_tallies).to eq [0, 0, 0]
     end
 
     it 'saves edit history' do