Fixed issue #16708: php error in statistics view (#1616)

### Diagnosis

According to the description of the issue we believe to be caused by a combination of errors:

1) When changing the type of a question from a type with subquestions to a type without subquestions (ex: from Array to List), the subquestions are not removed from the DB. The subquestions remain in the table with the old type.

2) Check Integrity processes these "orphan" subquestions making sure they match the parent question's type, without checking if it's actually correct for that type to have subquestions. So, it updates the subquestions while it should remove them.

I think both actions should check the question type definition (QuestionType attributes) and, if the 'subquestions' attribute is false, remove the subquestions from the DB.

### Solution

1) Override 'update' method in Question model to delete existing subquestions if the question's type doesn't allow them.

2) Updated saveQuestionData action to avoid saving subquestions if the question's type doesn't allow them (because the subquestions are part the POST if they exist in the original question).

3) Updated common_helper's 'fixSubquestions' (used by check integrity) to do the same as mentioned in point 1.

Note that 'fixSubquestions' only processes subquestions where the type is not the same as the parent question. This wasn't modified because of the potential impact in performance, but it means it won't fix subquestions of a wrong type if the parent question has the same type.

application/controllers/QuestionEditorController.php

@@ -314,8 +314,8 @@ function ($value) {
                 SettingsUser::deleteUserSetting('question_default_values_' . $questionData['question']['type']);
             }
 
-            // If set, store subquestions
-            if (isset($questionData['scaledSubquestions'])) {
+            // If set, and the question type allows it, store subquestions
+            if (isset($questionData['scaledSubquestions']) && $oQuestion->getQuestionType()->subquestions) {
                 if (!($questionCopy === true && $questionCopySettings['copySubquestions'] == false)) {
                     $setApplied['scaledSubquestions'] = $this->storeSubquestions(
                         $oQuestion,

application/helpers/common_helper.php

@@ -4333,7 +4333,9 @@ function getSurveyUserGroupList($outputformat = 'htmloptions', $surveyid)
 
 
 /**
-* This function fixes the group ID and type on all subquestions
+* This function fixes the group ID and type on all subquestions,
+* or removes the subquestions if the parent question's type doesn't
+* allow them.
 * Optimized for minimum memory usage even on huge databases
 */
 function fixSubquestions()
@@ -4346,9 +4348,21 @@ function fixSubquestions()
     ->limit(10000)
     ->query();
     $aRecords = $surveyidresult->readAll();
+
+    $aQuestionTypes = QuestionType::modelsAttributes();
     while (count($aRecords) > 0) {
         foreach ($aRecords as $sv) {
-            Yii::app()->db->createCommand("update {{questions}} set type='{$sv['type']}', gid={$sv['gid']} where qid={$sv['qid']}")->execute();
+            if ($aQuestionTypes[$sv['type']]['subquestions']) {
+                // If the question type allows subquestions, set the type in each subquestion
+                Yii::app()->db->createCommand("update {{questions}} set type='{$sv['type']}', gid={$sv['gid']} where qid={$sv['qid']}")->execute();
+            } else {
+                // If the question type doesn't allow subquestions, delete each subquestion
+                // Model is used because more tables are involved.
+                $oSubquestion = Question::model()->find("qid=:qid", array("qid"=>$sv['qid']));
+                if (!empty($oSubquestion)) {
+                    $oSubquestion->delete();
+                }
+            }
         }
         $surveyidresult = Yii::app()->db->createCommand()
         ->select('sq.qid, q.gid , q.type ')

application/models/Question.php

@@ -1236,4 +1236,35 @@ public function getHasSubquestions(){
     public function getHasAnsweroptions(){
 
     }
+
+    /**
+     * Override update() method to "clean" subquestions after saving a parent question
+     */
+    public function update($attributes=null)
+	{
+        if(parent::update($attributes)) {
+            $this->removeInvalidSubquestions();
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    protected function removeInvalidSubquestions()
+    {
+        // No need to remove anything if this is a subquestion
+        if ($this->parent_qid) {
+            return;
+        }
+
+        // Remove subquestions if the question's type doesn't allow subquestions
+        if (!$this->getQuestionType()->subquestions) {
+            $aSubquestions = Question::model()->findAll("parent_qid=:parent_qid", array("parent_qid"=>$this->qid));
+            if (!empty($aSubquestions)) {
+                foreach ($aSubquestions as $oSubquestion) {
+                    $oSubquestion->delete();
+                }
+            }
+        }
+    }
 }