From 24ccf23ed87da0348070fc02b8433c5f5664513c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Thu, 6 Aug 2020 21:44:09 +0200
Subject: [PATCH] Don't save the answer if the voter is not recorded

Up until now, we were assuming the voter was valid, but were not raising
an exception if it wasn't. And in the user interface everything seemed
to be working properly.

We were having this issue when skipping verification, when there could
be voters without a document number, which would be considered invalid.

Raising an exception when failing to save the voter and making sure the
answer and the voter are saved inside a transaction solves the problem.
---
 app/controllers/polls/questions_controller.rb |  4 +---
 app/models/poll/answer.rb                     |  8 ++++++--
 spec/models/poll/answer_spec.rb               | 19 +++++++++++++++----
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/app/controllers/polls/questions_controller.rb b/app/controllers/polls/questions_controller.rb
index 4236f7f52..3f2809b75 100644
--- a/app/controllers/polls/questions_controller.rb
+++ b/app/controllers/polls/questions_controller.rb
@@ -9,9 +9,7 @@ class Polls::QuestionsController < ApplicationController
     token = params[:token]
 
     answer.answer = params[:answer]
-    answer.touch if answer.persisted?
-    answer.save!
-    answer.record_voter_participation(token)
+    answer.save_and_record_voter_participation(token)
 
     @answers_by_question_id = { @question.id => params[:answer] }
   end
diff --git a/app/models/poll/answer.rb b/app/models/poll/answer.rb
index 4beb3b73a..8742b7d64 100644
--- a/app/models/poll/answer.rb
+++ b/app/models/poll/answer.rb
@@ -14,7 +14,11 @@ class Poll::Answer < ApplicationRecord
   scope :by_author, ->(author_id) { where(author_id: author_id) }
   scope :by_question, ->(question_id) { where(question_id: question_id) }
 
-  def record_voter_participation(token)
-    Poll::Voter.find_or_create_by(user: author, poll: poll, origin: "web", token: token)
+  def save_and_record_voter_participation(token)
+    transaction do
+      touch if persisted?
+      save!
+      Poll::Voter.find_or_create_by!(user: author, poll: poll, origin: "web", token: token)
+    end
   end
 end
diff --git a/spec/models/poll/answer_spec.rb b/spec/models/poll/answer_spec.rb
index 6ffb5e3d1..8c0598734 100644
--- a/spec/models/poll/answer_spec.rb
+++ b/spec/models/poll/answer_spec.rb
@@ -37,7 +37,7 @@ describe Poll::Answer do
     end
   end
 
-  describe "#record_voter_participation" do
+  describe "#save_and_record_voter_participation" do
     let(:author) { create(:user, :level_two) }
     let(:poll) { create(:poll) }
     let(:question) { create(:poll_question, :yes_no, poll: poll) }
@@ -46,7 +46,7 @@ describe Poll::Answer do
       answer = create(:poll_answer, question: question, author: author, answer: "Yes")
       expect(answer.poll.voters).to be_blank
 
-      answer.record_voter_participation("token")
+      answer.save_and_record_voter_participation("token")
       expect(poll.reload.voters.size).to eq(1)
       voter = poll.voters.first
 
@@ -57,12 +57,12 @@ describe Poll::Answer do
 
     it "updates a poll_voter with user and poll data" do
       answer = create(:poll_answer, question: question, author: author, answer: "Yes")
-      answer.record_voter_participation("token")
+      answer.save_and_record_voter_participation("token")
 
       expect(poll.reload.voters.size).to eq(1)
 
       answer = create(:poll_answer, question: question, author: author, answer: "No")
-      answer.record_voter_participation("token")
+      answer.save_and_record_voter_participation("token")
 
       expect(poll.reload.voters.size).to eq(1)
 
@@ -70,5 +70,16 @@ describe Poll::Answer do
       expect(voter.document_number).to eq(answer.author.document_number)
       expect(voter.poll_id).to eq(answer.poll.id)
     end
+
+    it "does not save the answer if the voter is invalid" do
+      allow_any_instance_of(Poll::Voter).to receive(:valid?).and_return(false)
+      answer = build(:poll_answer)
+
+      expect do
+        answer.save_and_record_voter_participation("token")
+      end.to raise_error(ActiveRecord::RecordInvalid)
+
+      expect(answer).not_to be_persisted
+    end
   end
 end