06079: sub-question filtering broken if "other" or "no answer" option is given

This bug affects 1 person(s).

ID	Project	Category	View Status	Date Submitted	Last Update

06079	Bug reports	Expression Manager	public	2012-05-10 11:07	2012-06-03 07:51

Reporter	~~user19811~~	Assigned To	TMSWhite
Priority	normal	Severity	partial_block
Status	closed	Resolution	fixed
Product Version	1.92+

Summary	06079: sub-question filtering broken if "other" or "no answer" option is given
Description	You have two questions: A "Multiple Choice" with "other" and mandatory An "Array" mandatory Each line of the array corresponds with one answer from the first question. If check one line of the first question, the corresponding line of the second question shoul be shown. For the answer options "no answer" and "other", there is no line in the second question. So we only have conditions for the other answer options of the first question. If you show the question, that the browser will not do anything, as there is an error in the JavaScript code produced by the LimeExpressionManager.php class. In line 5510, the if part is build with this line: $relParts[] = " if ( " . $sq['relevancejs'] . " ) {\n"; But the index 'relevancejs' is empty and so the output is: if ( ) { which is not valid JavaScript. A quick fix, I could find, was replacing the line with the following: $relParts[] = " if ( " . ( empty( $sq['relevancejs'] )? 'false' : $sq['relevancejs'] ) . " ) {\n"; which will than produce the following for such a case: if ( false ) { This is valid JavaScript code and the else part will be executed, which is the desired behaviour in that case. I don't know if it's a conceptional bug or something else, but I just wanted to share my bugfix with you.
Tags	No tags attached.
Attached Files	limesurvey_survey_98464.lss (26,264 bytes) diff6079.diff (3,000 bytes) diff --git a/classes/expressions/LimeExpressionManager.php b/classes/expressions/LimeExpressionManager.php index 5a3f2db..8f7ee35 100644 --- a/classes/expressions/LimeExpressionManager.php +++ b/classes/expressions/LimeExpressionManager.php @@ -3260,26 +3260,31 @@ if (!is_null($questionNum)) { $jsVars = $this->em->GetJSVarsUsed(); - $relevanceVars = implode('\|',$this->em->GetJSVarsUsed()); - $relevanceJS = $this->em->GetJavaScriptEquivalentOfExpression(); - - if (!isset($this->subQrelInfo[$questionNum])) { - $this->subQrelInfo[$questionNum] = array(); + if(count ($jsVars)){ + $relevanceVars = implode('\|',$this->em->GetJSVarsUsed()); + $relevanceJS = $this->em->GetJavaScriptEquivalentOfExpression(); + if (!isset($this->subQrelInfo[$questionNum])) { + $this->subQrelInfo[$questionNum] = array(); + } + $this->subQrelInfo[$questionNum][$rowdivid] = array( + 'qid' => $questionNum, + 'eqn' => $eqn, + 'prettyPrintEqn' => $prettyPrint, + 'result' => $result, + 'numJsVars' => count($jsVars), + 'relevancejs' => $relevanceJS, + 'relevanceVars' => $relevanceVars, + 'rowdivid' => $rowdivid, + 'type'=>$type, + 'qtype'=>$qtype, + 'sgqa'=>$sgqa, + 'hasErrors'=>$hasErrors, + ); + } + else + { + $this->subQrelInfo[$questionNum][$rowdivid] = false; } - $this->subQrelInfo[$questionNum][$rowdivid] = array( - 'qid' => $questionNum, - 'eqn' => $eqn, - 'prettyPrintEqn' => $prettyPrint, - 'result' => $result, - 'numJsVars' => count($jsVars), - 'relevancejs' => $relevanceJS, - 'relevanceVars' => $relevanceVars, - 'rowdivid' => $rowdivid, - 'type'=>$type, - 'qtype'=>$qtype, - 'sgqa'=>$sgqa, - 'hasErrors'=>$hasErrors, - ); } return $result; } @@ -5494,10 +5499,11 @@ { foreach ($LEM->subQrelInfo[$arg['qid']] as $subq) { - $subqParts[$subq['rowdivid']] = $subq; + if($subq){ + $subqParts[$subq['rowdivid']] = $subq; + } } } - $qidList[$arg['qid']] = $arg['qid']; if (!isset($gseq_qidList[$arg['gseq']])) { diff6079.diff (3,000 bytes) ProcessSubQRelevance.php (2,434 bytes)

Bug heat	6
Complete LimeSurvey version number (& build)	120501
I will donate to the project if issue is resolved	No
Browser
Database type & version	5.1.61-0+squeeze1-log - (Debian)
Server OS (if known)	Debian 2.6.32-41
Webserver software & version (if known)	Apache/2.2.16 (Debian)
PHP Version	PHP Version 5.3.3-7+squeeze8

User List	There are no users monitoring this issue.

DenisChenu 2012-05-10 12:32 developer ~18654	Thank you kauboy. And think it make a lot of helping for "bad expression manager manipualtion" ;) Carsten: "which is the desired behaviour in that case." ? I don't know the exact desired behaviour

DenisChenu 2012-05-10 14:30 developer ~18663	kauboy : i can't reproduce on 120501 ? I upload a survey sample, maybe i don't understand. Can you upload a survey sample too ?

DenisChenu 2012-05-11 11:49 developer ~18695	kauboy : need an example survey, i can't reproduce with my test survey. Thank you.

~~user19811~~ 2012-05-11 14:16 ~18700	The newly attached survey has the two questions with the conditions set, that will cause the error.

DenisChenu 2012-05-11 15:02 developer ~18701	OK, Don't put the "no answer" at the good question. It's not really a bug, more a new feature. I see with "Expression Manager Great Guru"

DenisChenu 2012-05-11 15:15 developer ~18704	Hello Tom ! I knew you have some time for us and LimeSurvey now ;) . I put a patch diff6079.diff for the "new feature". But this patch change function _ProcessSubQRelevance , i put too the new function. This patch work for kauboy and for other problem like 5977 . It don't change the "showlogicfile", i think it can be a good idea to have permit logical error but don't throw error to user. If OK, return the bug to me after while. Thank's a lot.

TMSWhite 2012-05-11 15:26 reporter ~18706	kauboy- That is an error in the survey. According to the documentation, if you want to use array_filter, then the sub-questions in each must match. Your question 2 has SQ003, which is not present in question 1. Can you provide a real example of why you think extra sub-questions in array_filter-ed questions should be allowed?

DenisChenu 2012-05-11 15:41 developer ~18709	Hello Tom, It's why i say it's a "new feature" :). Maybe we can put : // Now check whether there is sub-question relevance to perform for this question $subqParts = array(); if (isset($LEM->subQrelInfo[$arg['qid']])) { foreach ($LEM->subQrelInfo[$arg['qid']] as $subq) { if(isset($subq['relevancejs']){ $subqParts[$subq['rowdivid']] = $subq; } } } Less modification, but still work if SubQ aren't present or if user made an error ;). Denis

TMSWhite 2012-05-11 15:44 reporter ~18711	Denis- Glad to see you're starting to master EM :-) Yes, that is an option. Another is to show the error on the logic file, but surround the generated javascript in try { } catch { } so that it doesn't block the survey. Personally, I'd vote for the 2nd option, since in a long survey, users might not notice that they didn't keep their set of answer options consistent.

~~user19811~~ 2012-05-11 16:12 ~18712	That's absolutely correct! I just asked my colleague, why she wanted to have it the way. Her answer was: If you don't make the question mandatory, than the user can not choose "no answer". So she added "no answer" but it would have make any sense to add something to the array. But I explained to her, that yuo don't need a "no answer" if a question is not mandatory. So that's OK. You can close the ticket!

DenisChenu 2012-05-11 16:14 developer ~18713	<quote>but surround the generated javascript in try { } catch { }</quote> Great ! Give it to me and go to work ! ;)

TMSWhite 2012-05-11 16:17 reporter ~18714	Per reporter, no change necessary, so closing ticket

TMSWhite 2012-06-03 07:50 reporter ~19042	Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=8633

TMSWhite 2012-06-03 07:51 reporter ~19044	Fix committed to Yii branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=8635

LimeSurvey: master 8b549dca 2012-06-02 22:48:05 TMSWhite Details Diff	Fixed issue 06165: unable to implement affirmative exclusive non-answer in array_filter_exclude Fixed issue 06079: sub-question filtering broken if "other" or "no answer" option is given Dev Array_filter and array_filter_exclude now handle mismatched lists of sub-questions gracefully. Values in list B that are not in list A will not be filtered, and will not throw errors. Dev This lets authors support exclusive options like "none of the above" in array filtered questions	Affected Issues 06079, 06165
	mod - classes/expressions/LimeExpressionManager.php	Diff File
LimeSurvey: Yii a2b96ed8 2012-06-02 22:51:04 TMSWhite Details Diff	Fixed issue 06165: unable to implement affirmative exclusive non-answer in array_filter_exclude Fixed issue 06079: sub-question filtering broken if "other" or "no answer" option is given Dev Array_filter and array_filter_exclude now handle mismatched lists of sub-questions gracefully. Values in list B that are not in list A will not be filtered, and will not throw errors. Dev This lets authors support exclusive options like "none of the above" in array filtered questions	Affected Issues 06079, 06165
	mod - application/helpers/expressions/em_manager_helper.php	Diff File

Date Modified	Username	Field	Change
2012-05-10 11:07	~~user19811~~	New Issue
2012-05-10 12:24	c_schmitz	Assigned To	=> DenisChenu
2012-05-10 12:24	c_schmitz	Status	new => assigned
2012-05-10 12:32	DenisChenu	Note Added: 18654
2012-05-10 14:30	DenisChenu	Note Added: 18663
2012-05-10 14:31	DenisChenu	File Added: limesurvey_survey_27314.lss
2012-05-11 11:49	DenisChenu	Note Added: 18695
2012-05-11 11:49	DenisChenu	Status	assigned => feedback
2012-05-11 14:15	~~user19811~~	File Added: limesurvey_survey_98464.lss
2012-05-11 14:16	~~user19811~~	Note Added: 18700
2012-05-11 14:16	~~user19811~~	Status	feedback => assigned
2012-05-11 15:02	DenisChenu	Note Added: 18701
2012-05-11 15:08	DenisChenu	Relationship added	related to 05977
2012-05-11 15:10	DenisChenu	File Added: diff6079.diff
2012-05-11 15:10	DenisChenu	File Added: ProcessSubQRelevance.php
2012-05-11 15:10	DenisChenu	File Deleted: limesurvey_survey_27314.lss
2012-05-11 15:11	DenisChenu	Assigned To	DenisChenu => TMSWhite
2012-05-11 15:15	DenisChenu	Note Added: 18704
2012-05-11 15:26	TMSWhite	Note Added: 18706
2012-05-11 15:26	TMSWhite	Status	assigned => feedback
2012-05-11 15:41	DenisChenu	Note Added: 18709
2012-05-11 15:44	TMSWhite	Note Added: 18711
2012-05-11 16:12	~~user19811~~	Note Added: 18712
2012-05-11 16:12	~~user19811~~	Status	feedback => assigned
2012-05-11 16:14	DenisChenu	Note Added: 18713
2012-05-11 16:17	TMSWhite	Note Added: 18714
2012-05-11 16:17	TMSWhite	Status	assigned => resolved
2012-05-11 16:17	TMSWhite	Resolution	open => no change required
2012-05-17 10:28	c_schmitz	Status	resolved => closed
2012-06-03 07:50	TMSWhite	Changeset attached	=> LimeSurvey master 8b549dca
2012-06-03 07:50	TMSWhite	Note Added: 19042
2012-06-03 07:50	TMSWhite	Resolution	no change required => fixed
2012-06-03 07:51	TMSWhite	Changeset attached	=> LimeSurvey Yii a2b96ed8
2012-06-03 07:51	TMSWhite	Note Added: 19044
2012-06-03 07:51	TMSWhite	Relationship added	related to 06165

View Issue Details

Relationships

Users monitoring this issue

Activities

Related Changesets

Issue History

related to	05977	closed	DenisChenu	Unable to use array_filter : javascript error
related to	06165	closed	TMSWhite	unable to implement affirmative exclusive non-answer in array_filter_exclude