From 708a1cec00c7c6176748987e48c4bbd013fb41d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 4 Sep 2013 23:42:35 +0200 Subject: [PATCH 1/3] MDL-29378 Fix regression in the Survey/COLLES P+A module This basically reverts the commit 5196df589b0fbcead4a0943c8e7b227f8a98c897 that I believe was a result of misunderstanding of how question type field is (ab)used in the Survey module. As it took significant time to get familiar with the overall logic of questions and their processing in the module, I left my findings in added inline comments. The point is that it is $question->type that matters. Types of questions listed as subquestions in the multi field is irrelevant in that case (and all have it set to 1 IIRC). This patch re-enables the "COLLES (Preferred and Actual)" survey type that did not work at all due to regression. --- mod/survey/lib.php | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/mod/survey/lib.php b/mod/survey/lib.php index cebafac633e..3444da0e94b 100644 --- a/mod/survey/lib.php +++ b/mod/survey/lib.php @@ -509,8 +509,22 @@ function survey_print_multi($question) { $options = explode( ",", $question->options); $numoptions = count($options); + // COLLES Actual (which is having questions of type 1) and COLLES Preferred (type 2) + // expect just one answer per question. COLLES Actual and Preferred (type 3) expects + // two answers per question. ATTLS (having a single question of type 1) expects one + // answer per question. CIQ is not using multiquestions (i.e. a question with subquestions). + // Note that the type of subquestions does not really matter, it's the type of the + // question itself that determines everything. $oneanswer = ($question->type == 1 || $question->type == 2) ? true : false; + // COLLES Preferred (having questions of type 2) will use the radio elements with the name + // like qP1, qP2 etc. COLLES Actual and ATTLS have radios like q1, q2 etc. + if ($question->type == 2) { + $P = "P"; + } else { + $P = ""; + } + echo "$strresponses"; echo "". get_string('notyetanswered', 'survey'). ""; while (list ($key, $val) = each ($options)) { @@ -533,13 +547,6 @@ function survey_print_multi($question) { $q->text = get_string($q->text, "survey"); } - $oneanswer = ($q->type == 1 || $q->type == 2) ? true : false; - if ($q->type == 2) { - $P = "P"; - } else { - $P = ""; - } - echo ""; if ($oneanswer) { echo ""; From 2e5d19ddfc6b21af17b22fc3eee3c49baa2f74f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Wed, 4 Sep 2013 23:52:06 +0200 Subject: [PATCH 2/3] MDL-29378 Fix the form display in the Survey/COLLES P+A module In MDL-7501, we stopped using rowspanning cells in the form table due to accessibility. That had introduced a regression so in the COLLES P+A survey, all the rows were displayed with the same background colour. This patch returns the previous behaviour that each couple of items can be distinguished by the background colour. Also, there is no need to display "I prefer that" and "I found that" as a small text any more. It had made sense in rowspanning layout but not after MDL-7501 was fixed. And finally, as all items are enumerated now sequentially, there are actually 48 lines, each couple covering one question in two variants. I think it's correct to reflect this in the description of the form so the text was slightly amended. --- mod/survey/lang/en/survey.php | 2 +- mod/survey/lib.php | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/mod/survey/lang/en/survey.php b/mod/survey/lang/en/survey.php index a664a9cb32e..893d3ad77ed 100644 --- a/mod/survey/lang/en/survey.php +++ b/mod/survey/lang/en/survey.php @@ -98,7 +98,7 @@ $string['clicktocontinue'] = 'Click here to continue'; $string['clicktocontinuecheck'] = 'Click here to check and continue'; $string['collesaintro'] = 'The purpose of this survey is to help us understand how well the online delivery of this unit enabled you to learn. -Each one of the 24 statements below asks about your experience in this unit. +Each couple of the 24 statements below asks about your experience in this unit. There are no \'right\' or \'wrong\' answers; we are interested only in your opinion. Please be assured that your responses will be treated with a high degree of confidentiality, and will not affect your assessment. diff --git a/mod/survey/lib.php b/mod/survey/lib.php index 3444da0e94b..168f070a4ca 100644 --- a/mod/survey/lib.php +++ b/mod/survey/lib.php @@ -542,7 +542,11 @@ function survey_print_multi($question) { foreach ($subquestions as $q) { $qnum++; - $rowclass = survey_question_rowclass($qnum); + if ($oneanswer) { + $rowclass = survey_question_rowclass($qnum); + } else { + $rowclass = survey_question_rowclass(round($qnum / 2)); + } if ($q->text) { $q->text = get_string($q->text, "survey"); } @@ -564,11 +568,10 @@ function survey_print_multi($question) { $checklist["q$P$q->id"] = 0; } else { - // yu : fix for MDL-7501, possibly need to use user flag as this is quite ugly. echo ""; echo "$qnum   "; $qnum++; - echo "$stripreferthat   "; + echo "$stripreferthat   "; echo "$q->text\n"; $default = get_accesshide($strdefault); @@ -585,7 +588,7 @@ function survey_print_multi($question) { echo ""; echo ""; echo "$qnum   "; - echo "$strifoundthat   "; + echo "$strifoundthat   "; echo "$q->text\n"; $default = get_accesshide($strdefault); From f2578055730250018b0f2fadf739500e83b4b743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Thu, 5 Sep 2013 00:01:42 +0200 Subject: [PATCH 3/3] MDL-29378 Code cleanup in the Survey module While working on the issue, I spotted these two places that were worth of fixing. The first one is a trivial reminiscence of a previous refactoring, after which both branches of the if() statement became equal. The second one is actually a typo as in theory it could generate unexpected input fields with the name like qPP1. Luckily this never happened due to the way how survey questions are hardcoded (there are no questions with the type 2 that would require two answers to their subquestions). --- mod/survey/lib.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/mod/survey/lib.php b/mod/survey/lib.php index 168f070a4ca..07f12aa420e 100644 --- a/mod/survey/lib.php +++ b/mod/survey/lib.php @@ -532,11 +532,7 @@ function survey_print_multi($question) { } echo "\n"; - if ($oneanswer) { - echo "$question->intro\n"; - } else { - echo "$question->intro\n"; - } + echo "$question->intro\n"; $subquestions = $DB->get_records_list("survey_questions", "id", explode(',', $question->multi)); @@ -575,7 +571,7 @@ function survey_print_multi($question) { echo "$q->text\n"; $default = get_accesshide($strdefault); - echo ''; + echo ''; for ($i=1;$i<=$numoptions;$i++) {