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
This commit is contained in:
		
							parent
							
								
									6dfda3670f
								
							
						
					
					
						commit
						2e7bc0fbf5
					
				
					 4 changed files with 25 additions and 13 deletions
				
			
		| 
						 | 
				
			
			@ -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
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue