about summary refs log tree commit diff
diff options
context:
space:
mode:
authorClaire <claire.github-309c@sitedethib.com>2022-02-10 14:26:54 +0100
committerGitHub <noreply@github.com>2022-02-10 14:26:54 +0100
commit63854bee6c387fc82b41f1a8eea968790541cf29 (patch)
treec24b6737c55e47cdc8b64d1f36be45bf8f051138
parent1bfcb75105baae556101f44957d0fa5b28ef013b (diff)
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
-rw-r--r--app/models/poll.rb6
-rw-r--r--app/services/activitypub/process_status_update_service.rb6
-rw-r--r--app/services/update_status_service.rb6
-rw-r--r--spec/services/update_status_service_spec.rb20
4 files changed, 25 insertions, 13 deletions
diff --git a/app/models/poll.rb b/app/models/poll.rb
index 71b5e191f..ba08309a1 100644
--- 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 5e38852fc..7438a7c53 100644
--- 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 68e73d3b4..238ef0755 100644
--- 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 1d4b0a6be..fe1b60d24 100644
--- 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