Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checkbox question validation fail when "other" answer value is numeric #282

Open
stephanepoinsart opened this issue Mar 24, 2020 · 1 comment

Comments

@stephanepoinsart
Copy link

stephanepoinsart commented Mar 24, 2020

Hello,

When a participant put a numeric value in an answer for a checkbox question with "other" fields and the number of checked box allowed is just at the limit, the validation will fail with the default message "There is something wrong with your answer to question (...)".

This is because in the function response_data, the array $responsedata contains a list of both field name and field index, so for an example with 2 other fields it stores something like :

{
1=>"1",
"o1" => "my first answer",
2 => "2",
"o2" => "my second answer"
}

The validation function count the number of value that are numeric in this array to know if the participant checked more box than allowed. This is wrong, because if you put a numeric value in a text field, the field is counted 2 times, so the validation fail because the participant checked "4 boxes out of the 2 boxes that exists". It should check the key (starts with the letter "o") and not the value (user input).

While the bug is unfixed, it's quit hard to understand because it usually need the combination "a lot of box checked" + "one or more numeric value in other fields". If you test for just one of the condition or the other you will not trigger the bug and will fail miserably to understand it for hours while problem reports from users flood your inbox, and be as sad than a penguin facing global warming on his melting iceberg.

The provided patch should fix the problem, but i did not have the opportunity to test it in production.

Best regards,

Stephane

diff --git a/classes/question/check.php b/classes/question/check.php
index 67aa12c..3a4f8e1 100644
--- a/classes/question/check.php
+++ b/classes/question/check.php
@@ -212,7 +212,7 @@ class check extends question {
                 }
             }
         } else if (isset($responsedata->{'q'.$this->id})) {
-            foreach ($responsedata->{'q'.$this->id} as $answer) {
+            foreach ($responsedata->{'q'.$this->id} as $answerid => $answer) {
                 if (strpos($answer, 'other_') !== false) {
                     // ..."other" choice is checked but text box is empty.
                     $othercontent = "q".$this->id.substr($answer, 5);
@@ -221,7 +221,7 @@ class check extends question {
                         break;
                     }
                     $nbrespchoices++;
-                } else if (is_numeric($answer)) {
+                } else if (is_numeric($answerid)) {
                     $nbrespchoices++;
                 }
             }
@@ -337,4 +337,4 @@ class check extends question {
         }
         return $resultdata;
     }
-}
\ No newline at end of file
+}

@mchurchward
Copy link
Contributor

Thanks for this report. Could you provide a specific question example, and specific response for testing? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants