diff --git a/app/models/poll.rb b/app/models/poll.rb index 6f5b5488a..9aa63a7c6 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -18,6 +18,22 @@ class Poll < ApplicationRecord before_save :evaluate_markdown after_create :create_poll_options + # We only want a user to be able to vote (or abstain) once per poll. + # This query helps validate that constraint. + # + # + # @param user_id [Integer] + # + # @return [TrueClass] if the given user has a registered vote or skip + # @return [FalseClass] if the given user does not have a poll vote + # nor poll skip. + def vote_previously_recorded_for?(user_id:) + return true if poll_votes.where(user_id: user_id).any? + return true if poll_skips.where(user_id: user_id).any? + + false + end + def voting_data { votes_count: poll_votes_count, votes_distribution: poll_options.pluck(:id, :poll_votes_count) } end diff --git a/app/models/poll_skip.rb b/app/models/poll_skip.rb index 8e2b357d1..0068c7355 100644 --- a/app/models/poll_skip.rb +++ b/app/models/poll_skip.rb @@ -14,7 +14,9 @@ class PollSkip < ApplicationRecord private def one_vote_per_poll_per_user - already_voted = poll.poll_votes.where(user_id: user_id).any? || poll.poll_skips.where(user_id: user_id).any? - errors.add(:base, "cannot vote more than once in one poll") if already_voted + return false unless poll + return false unless poll.vote_previously_recorded_for?(user_id: user_id) + + errors.add(:base, "cannot vote more than once in one poll") end end diff --git a/app/models/poll_vote.rb b/app/models/poll_vote.rb index 11dd3fcfa..5d381795f 100644 --- a/app/models/poll_vote.rb +++ b/app/models/poll_vote.rb @@ -28,10 +28,9 @@ class PollVote < ApplicationRecord def one_vote_per_poll_per_user return false unless poll + return false unless poll.vote_previously_recorded_for?(user_id: user_id) - has_votes = ( - poll.poll_votes.where(user_id: user_id).any? || poll.poll_skips.where(user_id: user_id).any?) - errors.add(:base, "cannot vote more than once in one poll") if has_votes + errors.add(:base, "cannot vote more than once in one poll") end def touch_poll_votes_count diff --git a/spec/models/poll_skip_spec.rb b/spec/models/poll_skip_spec.rb index 394515d3a..01dce6a56 100644 --- a/spec/models/poll_skip_spec.rb +++ b/spec/models/poll_skip_spec.rb @@ -5,25 +5,23 @@ RSpec.describe PollSkip, type: :model do let(:user) { create(:user) } let(:poll) { create(:poll, article: article) } - describe "validations" do - context "when checking against poll" do - before do - create(:poll_skip, user: user, poll: poll) - end + context "when user has already voted (or skipped) on the poll" do + it "is invalid" do + vote = build(:poll_skip, user: user, poll: poll) + allow(vote.poll).to receive(:vote_previously_recorded_for?).with(user_id: user.id).and_return(true) - it "is unique across poll and user" do - expect(build(:poll_skip, user: user, poll: poll)).not_to be_valid - end - - it "is valid if it belongs to the same user but to a different poll" do - second_poll = create(:poll, article: article) - expect(build(:poll_skip, user: user, poll: second_poll)).to be_valid - end + expect(vote).not_to be_valid + expect(vote.errors[:base]).to include("cannot vote more than once in one poll") end + end - it "is unique across user and poll votes for the poll" do - create(:poll_vote, user: user, poll: poll, poll_option: poll.poll_options.last) - expect(build(:poll_skip, user: user, poll: poll)).not_to be_valid + context "when user has not voted nor skipped the poll" do + it "is valid" do + vote = build(:poll_skip, user: user, poll: poll) + allow(vote.poll).to receive(:vote_previously_recorded_for?).with(user_id: user.id).and_return(false) + + expect(vote).to be_valid + expect(vote.errors[:base]).to be_empty end end end diff --git a/spec/models/poll_spec.rb b/spec/models/poll_spec.rb index 316dc8b62..dfb3aa472 100644 --- a/spec/models/poll_spec.rb +++ b/spec/models/poll_spec.rb @@ -35,6 +35,33 @@ RSpec.describe Poll, type: :model do end end + describe "#vote_previously_recorded_for?" do + subject(:registered) { poll.vote_previously_recorded_for?(user_id: user.id) } + + let(:poll) { create(:poll) } + let(:user) { create(:user) } + + context "when user has existing poll_votes" do + before do + create(:poll_vote, poll: poll, user: user) + end + + it { is_expected.to be_truthy } + end + + context "when user has existing poll_skips" do + before do + create(:poll_skip, poll: poll, user: user) + end + + it { is_expected.to be_truthy } + end + + context "when user has no poll_skips nor poll votes" do + it { is_expected.to be_falsey } + end + end + context "when callbacks are triggered after create" do it "creates options from input" do options = %w[hello goodbye heyheyhey] diff --git a/spec/models/poll_vote_spec.rb b/spec/models/poll_vote_spec.rb index 69671f4c0..6cc7c83a2 100644 --- a/spec/models/poll_vote_spec.rb +++ b/spec/models/poll_vote_spec.rb @@ -9,50 +9,23 @@ RSpec.describe PollVote, type: :model do expect(described_class.new.valid?).to be(false) end - it "limits one vote per user per poll" do - create(:poll_vote, poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - described_class.create(poll_option_id: poll.poll_options.first.id, user_id: user.id, poll_id: poll.id) - described_class.create(poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - expect(user.poll_votes.size).to eq(1) + context "when user has already voted (or skipped) on the poll" do + it "is invalid" do + vote = build(:poll_vote, poll_option: poll.poll_options.last, user: user, poll: poll) + allow(vote.poll).to receive(:vote_previously_recorded_for?).with(user_id: user.id).and_return(true) + + expect(vote).not_to be_valid + expect(vote.errors[:base]).to include("cannot vote more than once in one poll") + end end - it "allows one vote per user across multiple polls" do - second_poll = create(:poll, article_id: article.id) - create(:poll_vote, poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - create(:poll_vote, poll_option_id: second_poll.poll_options.last.id, user_id: user.id, poll_id: second_poll.id) - expect(user.reload.poll_votes.size).to eq(2) - end + context "when user has not voted nor skipped the poll" do + it "is valid" do + vote = build(:poll_vote, poll_option: poll.poll_options.last, user: user, poll: poll) + allow(vote.poll).to receive(:vote_previously_recorded_for?).with(user_id: user.id).and_return(false) - it "allows multiple people to vote in one poll" do - second_user = create(:user) - create(:poll_vote, poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - create(:poll_vote, poll_option_id: poll.poll_options.last.id, user_id: second_user.id, poll_id: poll.id) - expect(user.poll_votes.size).to eq(1) - expect(second_user.poll_votes.size).to eq(1) - end - - it "disallows a user to skip poll after voting" do - create(:poll_vote, poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - PollSkip.create(poll_id: poll.id, user_id: user.id) - expect(user.poll_skips.size).to eq(0) - end - - it "disallows a vote after skipping" do - PollSkip.create(poll_id: poll.id, user_id: user.id) - described_class.create(poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - expect(user.poll_votes.size).to eq(0) - end - - it "updates poll voting count after create" do - create(:poll_vote, poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - expect(poll.reload.poll_votes_count).to eq(1) - expect(poll.reload.poll_options.last.poll_votes_count).to eq(1) - end - - it "updates poll voting count after create with accuracy alongside skips" do - create(:poll_vote, poll_option_id: poll.poll_options.last.id, user_id: user.id, poll_id: poll.id) - PollSkip.create(poll_id: poll.id, user_id: user.id) - expect(poll.reload.poll_votes_count).to eq(1) - expect(poll.reload.poll_options.last.poll_votes_count).to eq(1) + expect(vote).to be_valid + expect(vote.errors[:base]).to be_empty + end end end