From 56e62a41b6380a552f71c6be60c5b69364653ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 10 Oct 2019 00:23:20 +0200 Subject: [PATCH] Fix duplicate given order creating answers It's possible to have a given order greater than the number of answers; we don't have any validation rules for that. So the check for the number of answers isn't enough. Checking the maximum given order in the answers is safer. Another option would be to reorder the answers every time we add a new one, but I'm not sure whether that's the expected behaviour. Note even after this change the action is not thread-safe, as it is possible to create two questions with the same given order with two simultaneous requests. --- app/controllers/polls/answers_controller.rb | 2 +- spec/features/polls/votation_type_spec.rb | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/controllers/polls/answers_controller.rb b/app/controllers/polls/answers_controller.rb index c8b0081f2..13b1b3412 100644 --- a/app/controllers/polls/answers_controller.rb +++ b/app/controllers/polls/answers_controller.rb @@ -9,7 +9,7 @@ class Polls::AnswersController < ApplicationController if @question.votation_type.open? && !check_question_answer_exist @question.question_answers.create( title: params[:answer], - given_order: @question.question_answers.count + 1, + given_order: @question.question_answers.maximum(:given_order).to_i + 1, hidden: false ) flash.now[:notice] = t("dashboard.polls.index.succesfull") diff --git a/spec/features/polls/votation_type_spec.rb b/spec/features/polls/votation_type_spec.rb index b313361f3..cf5363d2c 100644 --- a/spec/features/polls/votation_type_spec.rb +++ b/spec/features/polls/votation_type_spec.rb @@ -320,6 +320,17 @@ describe "Poll Votation Type" do expect(page).to have_link "Added answer", class: "answered" end end + + scenario "existing given order is bigger than the number of answers", :js do + answer1.update(given_order: question.question_answers.count + 1) + + visit poll_path(poll_current) + + fill_in "answer", with: "Added answer" + click_button "Add answer" + + expect(page).to have_link "Added answer" + end end context "Answers set" do