View Issue Details

Jump to NotesJump to History
This issue affects 1 person(s).
 8
IDProjectCategoryView StatusDate SubmittedLast Update
05403Bug reportsPrint Viewpublic2011-08-17 18:012011-08-24 15:32
Reporterronvdburg Assigned Toc_schmitz  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version1.91+ 
Summary05403: Actually hide questions in Print Answers if they have attribute 'hidden'
Description

The 'print answers' at the end of the survey prints also hidden questions.

The attached patch file hides these questions from printing answers.

Steps To Reproduce

Give a question attribute 'hidden'.
Finish & submit the survey and then press 'print answers'.

Additional Information

If you wish to configure this behaviour with a new variable in config.php, please give me a proper name for the new variable and I will upload a new patch file that has configurable behaviour for hiding hidden questions from 'print answers'.

TagsNo tags attached.
Attached Files
common_functions.php.diff-u (622 bytes)
common_functions.php.diff-u.v2.txt (598 bytes)    
--- common_functions.php.org	2011-08-11 20:48:32.000000000 +0200
+++ common_functions.php	2011-08-18 11:37:49.000000000 +0200
@@ -7553,6 +7553,14 @@
     $oldqid = 0;
     foreach ($aFieldMap as $sKey=>$fname)
     {
+        if (!empty($fname['qid']))
+        {
+            $attributes = getQuestionAttributes($fname['qid']);
+            if (getQuestionAttributeValue($attributes, 'hidden') == 1)
+            {
+                continue;
+            }
+        }
         $question = $fname['question'];
         $subquestion='';
         if (isset($fname['gid']) && !empty($fname['gid'])) {
common_functions.php.diff-u.v2.txt (598 bytes)   
common_functions.php.svn-diff.txt (650 bytes)    
Index: common_functions.php
===================================================================
--- common_functions.php	(revision 10834)
+++ common_functions.php	(working copy)
@@ -7553,6 +7553,14 @@
     $oldqid = 0;
     foreach ($aFieldMap as $sKey=>$fname)
     {
+        if (!empty($fname['qid']))
+        {
+            $attributes = getQuestionAttributes($fname['qid']);
+            if (getQuestionAttributeValue($attributes, 'hidden') == 1)
+            {
+                continue;
+            }
+        }
         $question = $fname['question'];
         $subquestion='';
         if (isset($fname['gid']) && !empty($fname['gid'])) {
common_functions.php.svn-diff.txt (650 bytes)   
Bug heat8
Complete LimeSurvey version number (& build)10746
I will donate to the project if issue is resolvedNo
Browser
Database type & versionmysql 14.12
Server OS (if known)Centos 2.2.3 (Linux version 2.6.18-194.3.1.el5)
Webserver software & version (if known)Apache/2.2.3 (CentOS)
PHP Version5.1.6
Git branch / Pull Request URL

Users monitoring this issue

ronvdburg

Activities

mot

mot

2011-08-17 18:40

reporter   ~16085

For this line:

if (isset($fname['qid']) && !empty($fname['qid']))

a short note: empty() check's already if a variable is set. So could be better written as:

if (!empty($fname['qid']))
ronvdburg

ronvdburg

2011-08-18 12:26

reporter   ~16090

Ok mot, thanks for your explanation of empty().
See attached version 2 of the patch file containing your suggested rewrite.

By the way, shouldn't this also be done in other places?
For example, I copied the isset()+empty() construct from a few lines below.
Would you suggest to also remove the isset() before an empty() in the following lines (according to build 9671):
4424, 4476, 4569, 7558, 7566?

If you wish, I can upload another patch with those 5 changes.
I could then also include the same changes for:
group.php (1x)
index.php (2x)
admin/browse.php (1x)
admin/export_data_functions.php (1x)
admin/export_structure_quexml.php (1x)

admin/export_structure_xml.php (3x)
These 3 occurrences are a bit strange though:
------- quote -----
if ((!isset($exclude) && $exclude['answers'] !== true) || empty($exclude))
if ((!isset($exclude) && $exclude['conditions'] !== true) || empty($exclude))
if ((!isset($exclude) && $exclude['quotas'] !== true) || empty($exclude))
------- unquote -----
these could well read:
------- quote -----
if (empty($exclude))
if (empty($exclude))
if (empty($exclude))
------- unquote -----
because
!isset($x)
and
$x['bla'] !== true
can not be both true at the same time. Or am I wrong here?

admin/quota.php (2x)

Regards,

Ron
mot

mot

2011-08-18 12:54

reporter   ~16091

Last edited: 2011-08-18 12:57

Hi Ron,

yes I have seen there are other places as well. I think it's important for new code we don't use the more complicated expressions, but as long as old code is concerned, I suggest to not touch it (sounds a bit unfair, right :) ).

I can understand the itch in the fingers ;), the problem with changing/improving unrelated code is that it can introduce new bugs (as we put a hand on it, mistakes happen) and that it's much work that is better to do in a dedicated code review with a whole team etc. . Currently the codebase gets actually ported as well, so there is even less use in doing that now.

For the other indeed complicated looking expressions:

 (!isset($exclude) && $exclude['answers'] !== true)

The useless part is, that an unset variable is NULL and never an Array, so there is no Array Value that can be true or false. So this expression never is TRUE. The ORed part only is in action, which leaves

empty($exclude)

So logically your suggestion is correct. Question is, why did the original author write it that way? Maybe the following was meant?

(isset($exclude['answers']) && $exclude['answers'] !== true)

We can only assume, but we should not make assumptions in debugging. As long as we don't know and there is no known issue about that part, the code should not be changed. Contacting the original author could help here or testing for flaws that are related to these lines of code.

Thank you for taking care.
c_schmitz

c_schmitz

2011-08-23 18:34

administrator   ~16122

ronvdburg, is it possible to provide a patch created with subversion?
The provided diff format is very cumbersome.
ronvdburg

ronvdburg

2011-08-24 15:07

reporter   ~16129

Hi Carsten,

Sorry, forgot to use svn diff in stead of diff -u.
Is this better now?

Regards,

Ron
c_schmitz

c_schmitz

2011-08-24 15:32

administrator   ~16134

Thank you! Will be fixed in next 1.91+ release!

Issue History

Date Modified Username Field Change
2011-08-17 18:01 ronvdburg New Issue
2011-08-17 18:02 ronvdburg File Added: common_functions.php.diff-u
2011-08-17 18:03 ronvdburg Issue Monitored: ronvdburg
2011-08-17 18:40 mot Note Added: 16085
2011-08-18 11:39 ronvdburg File Added: common_functions.php.diff-u.v2.txt
2011-08-18 12:26 ronvdburg Note Added: 16090
2011-08-18 12:54 mot Note Added: 16091
2011-08-18 12:56 mot Note Edited: 16091
2011-08-18 12:57 mot Note Edited: 16091
2011-08-18 13:03 c_schmitz Assigned To => c_schmitz
2011-08-18 13:03 c_schmitz Status new => assigned
2011-08-23 18:34 c_schmitz Note Added: 16122
2011-08-24 15:01 ronvdburg File Added: common_functions.php.svn-diff.txt
2011-08-24 15:07 ronvdburg Note Added: 16129
2011-08-24 15:32 c_schmitz Note Added: 16134
2011-08-24 15:32 c_schmitz Status assigned => closed
2021-08-05 13:51 guest Bug heat 6 => 8
2026-10-25 16:41 c_schmitz Resolution open => fixed