Fix single-record invalid condition on PollVote (#23810)
parent
d3eefead30
commit
4bb39ac3c3
|
@ -23,6 +23,7 @@ class PollVote < ApplicationRecord
|
||||||
after_create_commit :increment_counter_cache
|
after_create_commit :increment_counter_cache
|
||||||
|
|
||||||
delegate :local?, to: :account
|
delegate :local?, to: :account
|
||||||
|
delegate :multiple?, :expired?, to: :poll, prefix: true
|
||||||
|
|
||||||
def object_type
|
def object_type
|
||||||
:vote
|
:vote
|
||||||
|
|
|
@ -2,13 +2,13 @@
|
||||||
|
|
||||||
class VoteValidator < ActiveModel::Validator
|
class VoteValidator < ActiveModel::Validator
|
||||||
def validate(vote)
|
def validate(vote)
|
||||||
vote.errors.add(:base, I18n.t('polls.errors.expired')) if vote.poll.expired?
|
vote.errors.add(:base, I18n.t('polls.errors.expired')) if vote.poll_expired?
|
||||||
|
|
||||||
vote.errors.add(:base, I18n.t('polls.errors.invalid_choice')) if invalid_choice?(vote)
|
vote.errors.add(:base, I18n.t('polls.errors.invalid_choice')) if invalid_choice?(vote)
|
||||||
|
|
||||||
if vote.poll.multiple? && vote.poll.votes.where(account: vote.account, choice: vote.choice).exists?
|
if vote.poll_multiple? && already_voted_for_same_choice_on_multiple_poll?(vote)
|
||||||
vote.errors.add(:base, I18n.t('polls.errors.already_voted'))
|
vote.errors.add(:base, I18n.t('polls.errors.already_voted'))
|
||||||
elsif !vote.poll.multiple? && vote.poll.votes.where(account: vote.account).exists?
|
elsif !vote.poll_multiple? && already_voted_on_non_multiple_poll?(vote)
|
||||||
vote.errors.add(:base, I18n.t('polls.errors.already_voted'))
|
vote.errors.add(:base, I18n.t('polls.errors.already_voted'))
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -18,4 +18,24 @@ class VoteValidator < ActiveModel::Validator
|
||||||
def invalid_choice?(vote)
|
def invalid_choice?(vote)
|
||||||
vote.choice.negative? || vote.choice >= vote.poll.options.size
|
vote.choice.negative? || vote.choice >= vote.poll.options.size
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def already_voted_for_same_choice_on_multiple_poll?(vote)
|
||||||
|
if vote.persisted?
|
||||||
|
account_votes_on_same_poll(vote).where(choice: vote.choice).where.not(poll_votes: { id: vote }).exists?
|
||||||
|
else
|
||||||
|
account_votes_on_same_poll(vote).where(choice: vote.choice).exists?
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def already_voted_on_non_multiple_poll?(vote)
|
||||||
|
if vote.persisted?
|
||||||
|
account_votes_on_same_poll(vote).where.not(poll_votes: { id: vote }).exists?
|
||||||
|
else
|
||||||
|
account_votes_on_same_poll(vote).exists?
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def account_votes_on_same_poll(vote)
|
||||||
|
vote.poll.votes.where(account: vote.account)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -10,4 +10,53 @@ RSpec.describe PollVote, type: :model do
|
||||||
expect(poll_vote.object_type).to eq :vote
|
expect(poll_vote.object_type).to eq :vote
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'validations' do
|
||||||
|
context 'with a vote on an expired poll' do
|
||||||
|
it 'marks the vote invalid' do
|
||||||
|
poll = Fabricate.build(:poll, expires_at: 30.days.ago)
|
||||||
|
|
||||||
|
vote = Fabricate.build(:poll_vote, poll: poll)
|
||||||
|
expect(vote).to_not be_valid
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with invalid choices' do
|
||||||
|
it 'marks vote invalid with negative choice' do
|
||||||
|
poll = Fabricate.build(:poll)
|
||||||
|
|
||||||
|
vote = Fabricate.build(:poll_vote, poll: poll, choice: -100)
|
||||||
|
expect(vote).to_not be_valid
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'marks vote invalid with choice in excess of options' do
|
||||||
|
poll = Fabricate.build(:poll, options: %w(a b c))
|
||||||
|
|
||||||
|
vote = Fabricate.build(:poll_vote, poll: poll, choice: 10)
|
||||||
|
expect(vote).to_not be_valid
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with a poll where multiple is true' do
|
||||||
|
it 'does not allow a second vote on same choice from same account' do
|
||||||
|
poll = Fabricate(:poll, multiple: true, options: %w(a b c))
|
||||||
|
first_vote = Fabricate(:poll_vote, poll: poll, choice: 1)
|
||||||
|
expect(first_vote).to be_valid
|
||||||
|
|
||||||
|
second_vote = Fabricate.build(:poll_vote, account: first_vote.account, poll: poll, choice: 1)
|
||||||
|
expect(second_vote).to_not be_valid
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'with a poll where multiple is false' do
|
||||||
|
it 'does not allow a second vote from same account' do
|
||||||
|
poll = Fabricate(:poll, multiple: false, options: %w(a b c))
|
||||||
|
first_vote = Fabricate(:poll_vote, poll: poll)
|
||||||
|
expect(first_vote).to be_valid
|
||||||
|
|
||||||
|
second_vote = Fabricate.build(:poll_vote, account: first_vote.account, poll: poll)
|
||||||
|
expect(second_vote).to_not be_valid
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue